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];
$res[$value] = $row[$value];
}
setCache($key,$cValue);
}
return $res;
}
The idea is to use the function as follows:
_getData($userid,"name_table","first_name,last_name",$dbConnection);
The above will return the data requested from memcahced first and if not present, then query it from mysql and store it in memcached.
The above works well, but how much does it hurt the eye?
1 Answer 1
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
andmemcache_get
functions, you don't need to instantiate an object withnew 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 callmemcache_connect
even though the rest of your code is object-oriented; be consistent and use$memcache_obj->connect(...)
memcache_get
returnsFALSE
when the object is not in the cache. So there is no need in yourgetCache
function to doif(empty($res)){$res=false}
because$res
will already befalse
.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
andsetCache
are called. I would callMemcache::close
before returning from either function. Alternatively, if these functions are called often within one request-response cycle, you could have aCache
class that you instantiate; theMemcache
instance would be a property of the class. Then in the class constructor, you can callnew Memcache
andMemcache::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
-
\$\begingroup\$ Great info, thank you very much. In terms of performance, is there any disadvantage to combining OOP with procedural, as you mention in "1" ? \$\endgroup\$JPJens– JPJens2016年06月12日 18:41:10 +00:00Commented Jun 12, 2016 at 18:41
-
\$\begingroup\$ I don't know and don't want to speculate. I encouraged you to be consistent because complexity breeds bugs. You want your code to be simple to follow in order to reduce errors and improve maintainability. Out of curiosity, how did you come up with 18 seconds as the default storage lifespan in
setCache
? \$\endgroup\$BeetleJuice– BeetleJuice2016年06月13日 08:39:38 +00:00Commented Jun 13, 2016 at 8:39 -
\$\begingroup\$ Great info. Thanks again. The 18 seconds are for testing purpose. Enough to see it being stored and Enough to get fresh data while developing. (could just as well have been 20) \$\endgroup\$JPJens– JPJens2016年06月13日 08:53:53 +00:00Commented Jun 13, 2016 at 8:53
-
\$\begingroup\$ Ok. Give that a lot of thought before you deploy, especially when it comes to storing DB data in the cache indiscriminately (your
_getData
always stores/fetches to/from the cache regardless of what info is sought). I ran into trouble overusing the cache to improve performance and my operations would return stale data (that no longer matched what was in the DB). So I'm always trying to find that equilibrium; I don't treat all DB data the same vis a vis caching. \$\endgroup\$BeetleJuice– BeetleJuice2016年06月13日 08:59:02 +00:00Commented Jun 13, 2016 at 8:59