HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Storing MySQL data in Memcache

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mysqlstoringdatamemcache

Problem

I have written a script that checks if data is known in memcached, and if not it queries it from the mysql db and stores it.

I would appreciate any inputs if i have done this the "correct" way, since i'm trying to optimize the code as much as possible.


_query() & _rows() are simple mysqli functions

Save in memcache

// Store memcache data
function setCache($key,$value,$storage=18) {
    $memcache_obj = new Memcache;
    $memcache_obj = memcache_connect(memcache_host, 11211);

    if(is_array($key)) {
        $i = 0;
        foreach($key as $id => $storeName) {
            $memcache_obj->set($storeName, $value[$id], 0, $storage);   
        }
    }else{
        $memcache_obj->set($key, $value, 0, $storage);
    }
}


Get from memcache

// Get memcache data
function getCache($key) {
    $memcache_obj = new Memcache;
    $memcache_obj = memcache_connect(memcache_host, 11211);
    $res = $memcache_obj->get($key);
    if(empty($res)) {
        $res = false;   // Expired
    }
    return $res;
}


Get data from memcache (or store in memcache if not present)

```
// Get data from either memcache or mysql
function _getData($userid,$table,$fields,$server) {

$toSelect = explode(",",$fields);
$toPush = array();
foreach($toSelect AS &$value) {
// Check if data is available from cache
$key = $userid."_".$table."_".$value;
$res[$value] = getCache($key);
if(empty($res[$value])) {
// Not cached, so must be pushed
$toPush[] = $value;
}
}

if($toPush) {
// Some or all data missing from cache, so we fetch and cache it
$fieldsToSelect = implode(",",$toPush);
$q = _query("SELECT ".$fieldsToSelect." FROM ".$table." WHERE id = '".$userid."'",$server);
$row = _rows($q);
$key = array();
$cValue = array();
foreach($toPush AS &$value) {
$key[] = $userid."_".$table."_".$value;
$cValue[] = $row[$value];

Solution

Four things come to mind:

-
You're mixing object oriented programing with the more 'traditional' way PHP was written. You don't need to use both. If you will use the older memcache_connect, memcache_get and memcache_get functions, you don't need to instantiate an object with new Memcache. Checkout the bottom of the connect page for the two side by side. Instantiating gives you more flexibility though (see point 3 below). You tend to call memcache_connect even though the rest of your code is object-oriented; be consistent and use $memcache_obj->connect(...)

-
memcache_get returns FALSE when the object is not in the cache. So there is no need in your getCache function to do if(empty($res)){$res=false} because $res will already be false.

-
Consider closing the connection to the cache when you're done with it. Otherwise you may have a large number of active connections as you create a new instance and connect every time getCache and setCache are called. I would call Memcache::close before returning from either function. Alternatively, if these functions are called often within one request-response cycle, you could have a Cache class that you instantiate; the Memcache instance would be a property of the class. Then in the class constructor, you can call new Memcache and Memcache::connect, and use the same connection for all operations. Then in the class destructor (the __destruct function, you can close the connection.

-
Your _getData indiscriminately uses caching. No matter what data is sought, it gets cached. You may have to be a bit more surgical because if you have fast changing DB data, _getData will soon start returning obsolete information (for instance, one would not seriously consider serving current stock prices or currency conversion rates from a cache). You may want to apply caching only to a subset of your DB, depending on the nature of your data

Context

StackExchange Code Review Q#131597, answer score: 2

Revisions (0)

No revisions yet.