[aur-dev] [PATCH 1/1] Make cache type selectable based on config value

[Lukas Fleischer]

On Sun, May 29, 2011 at 04:27:55PM -0700, elij wrote:
> Provie a mechanism to specify cache type from NONE, APC, or MEMCACHE
> based on a config variable.
> 
> If MEMCACHE type is selected, a list of servers can be specified to
> provide multiserver support. Note that php-memcaced is required for
> MEMCACHE support.

Sorry for the late response, I was very busy recently. Did you actually
test MEMCACHE support? I'm still too busy to setup and test less
important stuff thoroughly but I trust your judgement, especially since
this will probably be rarely used.

> ---
>  web/lib/aur.inc.php          |   49 +-----------------------
>  web/lib/cachefuncs.inc.php   |   85 ++++++++++++++++++++++++++++++++++++++++++
>  web/lib/config.inc.php.proto |   10 +++++
>  3 files changed, 96 insertions(+), 48 deletions(-)
>  create mode 100644 web/lib/cachefuncs.inc.php
> 
> diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php
> index 7cf43e6..e65677d 100644
> --- a/web/lib/aur.inc.php
> +++ b/web/lib/aur.inc.php
> @@ -13,12 +13,7 @@ set_lang();
>  include_once("config.inc.php");
>  include_once("version.inc.php");
>  include_once("acctfuncs.inc.php");
> -
> -# Check if APC extension is loaded, and set cache prefix if it is
> -if (!defined('EXTENSION_LOADED_APC')) {
> -	define('EXTENSION_LOADED_APC', extension_loaded('apc'));
> -	define('APC_PREFIX', 'aur:');
> -}
> +include_once("cachefuncs.inc.php");
>  
>  # see if the visitor is already logged in
>  #
> @@ -263,48 +258,6 @@ function db_query($query="", $db_handle="") {
>  	return $result;
>  }
>  
> -# set a value in the cache (currently apc) if cache
> -# is available for use. if not available, this becomes
> -# effectively a no-op (return value is false)
> -# accepts an optional TTL (defaults to 600 seconds)
> -function set_cache_value($key, $value, $ttl=600) {
> -	$status = false;
> -	if (EXTENSION_LOADED_APC) {
> -		$status = apc_store(APC_PREFIX.$key, $value, $ttl);
> -	}
> -	return $status;
> -}
> -
> -# get a value from the cache (currently apc) if cache
> -# is available for use. if not available, this 
> -# returns false (optionally sets passed in variable $status
> -# to false, much like apc_fetch behaves). this allows
> -# for testing the fetch result appropriately even in the event 
> -# that a 'false' value was the value in the cache.
> -function get_cache_value($key, &$status=false) {
> -	if(EXTENSION_LOADED_APC) {
> -		$ret = apc_fetch(APC_PREFIX.$key, $status);
> -		if ($status) {
> -			return $ret;
> -		}
> -	}
> -	return $status;
> -}
> -
> -# run a simple db query, retrieving and/or caching the value if APC
> -# is available for use
> -# accepts an optioanal TTL value (defaults to 600 seconds)
> -function db_cache_value($dbq, $dbh, $key, $ttl=600) {
> -	$status = false;
> -	$value = get_cache_value($key, $status);
> -	if (!$status) {
> -		$result = db_query($dbq, $dbh);
> -		$row = mysql_fetch_row($result);
> -		$value = $row[0];
> -		set_cache_value($key, $value, $ttl);
> -	}
> -	return $value;
> -}
>  
>  # set up the visitor's language
>  #
> diff --git a/web/lib/cachefuncs.inc.php b/web/lib/cachefuncs.inc.php
> new file mode 100644
> index 0000000..8e32370
> --- /dev/null
> +++ b/web/lib/cachefuncs.inc.php
> @@ -0,0 +1,85 @@
> +<?php
> +
> +if (!defined('CACHE_TYPE')) {
> +	define('CACHE_TYPE', 'NONE');
> +}
> +
> +# Check if APC extension is loaded, and set cache prefix if it is
> +if (CACHE_TYPE == 'APC' && !defined('EXTENSION_LOADED_APC')) {
> +	define('EXTENSION_LOADED_APC', extension_loaded('apc'));
> +	define('CACHE_PREFIX', 'aur:');
> +}
> +
> +# Check if memcache extension is loaded, and set cache prefix if it is
> +if (CACHE_TYPE == 'MEMCACHE' && !defined('EXTENSION_LOADED_MEMCACHE')) {
> +	define('EXTENSION_LOADED_MEMCACHE', extension_loaded('memcached'));
> +	define('CACHE_PREFIX', 'aur:');
> +	global $memcache;
> +	$memcache = new Memcached();
> +	$mcs = defined('MEMCACHE_SERVERS') ? MEMCACHE_SERVERS : '127.0.0.1:11211';
> +	foreach (explode(',', $mcs) as $elem) {
> +		$telem = trim($elem);
> +		$mcserver = explode(':', $telem);
> +		$memcache->addServer($mcserver[0], intval($mcserver[1]));
> +	}
> +}
> +
> +# set a value in the cache (currently apc) if cache
> +# is available for use. if not available, this becomes
> +# effectively a no-op (return value is false)
> +# accepts an optional TTL (defaults to 600 seconds)
> +function set_cache_value($key, $value, $ttl=600) {
> +	$status = false;
> +	if (defined('EXTENSION_LOADED_APC')) {
> +		$status = apc_store(CACHE_PREFIX.$key, $value, $ttl);
> +	}
> +	if (defined('EXTENSION_LOADED_MEMCACHE')) {
> +		global $memcache;
> +		$status = $memcache->set(CACHE_PREFIX.$key, $value, $ttl);
> +	}
> +	return $status;
> +}
> +
> +# get a value from the cache (currently apc) if cache
> +# is available for use. if not available, this 
> +# returns false (optionally sets passed in variable $status
> +# to false, much like apc_fetch behaves). this allows
> +# for testing the fetch result appropriately even in the event 
> +# that a 'false' value was the value in the cache.
> +function get_cache_value($key, &$status=false) {
> +	if(defined('EXTENSION_LOADED_APC')) {
> +		$ret = apc_fetch(CACHE_PREFIX.$key, $status);
> +		if ($status) {
> +			return $ret;
> +		}
> +	}
> +	if (defined('EXTENSION_LOADED_MEMCACHE')) {
> +		global $memcache;
> +		$ret = $memcache->get(CACHE_PREFIX.$key);
> +		if (!$ret) {
> +			$status = false;
> +		}
> +		else {
> +			$status = true;
> +		}
> +		return $ret;
> +	}
> +	return $status;
> +}
> +
> +# run a simple db query, retrieving and/or caching the value if APC
> +# is available for use
> +# accepts an optioanal TTL value (defaults to 600 seconds)
> +function db_cache_value($dbq, $dbh, $key, $ttl=600) {
> +	$status = false;
> +	$value = get_cache_value($key, $status);
> +	if (!$status) {
> +		$result = db_query($dbq, $dbh);
> +		$row = mysql_fetch_row($result);
> +		$value = $row[0];
> +		set_cache_value($key, $value, $ttl);
> +	}
> +	return $value;
> +}
> +
> +?>
> diff --git a/web/lib/config.inc.php.proto b/web/lib/config.inc.php.proto
> index 43c64d2..5351a4a 100644
> --- a/web/lib/config.inc.php.proto
> +++ b/web/lib/config.inc.php.proto
> @@ -24,6 +24,16 @@ define("DEFAULT_LANG", "en");
>  # development. Should not be enabled in production. Default to 0 (off).
>  define("SQL_DEBUG", 0);
>  
> +# set cache type. Either "APC", "MEMCACHE", or "NONE"
> +# defaults to NONE
> +#

Please try to match our commenting style. We are case sensitive, use
punctuation and use uppercase when starting a new sentence. Check the
other comments in this file.

It would be cool if you could pay attention to our style and commenting
guidelines everywhere (even tho there's a lot of legacy code that
doesn't match them, too) but this is particularly striking here.

> +#define("CACHE_TYPE", "APC");
> +#define("CACHE_TYPE", "MEMCACHE");
> +# if using memcache cache_type, list servers. you can separate multiple
> +# servers with a comma, ex: '127.0.0.1:11211,127.0.0.1:11212'
> +# if undefined, defaults to '127.0.0.1:11211'
> +#define("MEMCACHE_SERVERS", '127.0.0.1:11211');
> +
>  # Languages we have translations for
>  $SUPPORTED_LANGS = array(
>  	"ca" => "Català",
> -- 
> 1.7.2.5