[aur-dev] [PATCH 1/1] Make cache type selectable based on config value
Lukas Fleischer
archlinux at cryptocrack.de
Tue Jun 21 17:06:55 EDT 2011
On Tue, Jun 21, 2011 at 01:31:45PM -0700, elij wrote:
> On Tue, Jun 21, 2011 at 6:33 AM, Lukas Fleischer
> <archlinux at cryptocrack.de> wrote:
> > 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.
>
> It has been quite a while since I submitted the patch. I believe I
> tested it (why wouldn't I?), but I don't remember to the extent that I
> did so.
I just wanted to be sure. We had some patches that went into the master
branch and broke stuff due to insufficient testing. As I don't have
enough time to test your patches myself, I just wanted to be sure
someone tested them, at least. If you say you did so, that's perfectly
fine for me.
>
> >> ---
> >> 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.
>
> Really? Just to be pedantic, I see one opther sentence in this file
> start with a lowercase letter. But.. Really? This is your criticism
> here?
Dude, I'm just trying to keep our codebase as consistent and clean as
possible. That wasn't supposed to be any criticism but a proposal for a
minor tweak. Normally, I would just have amended your patch to match our
commenting style myself but there were some complains about me changing
patches without waiting for the original contributors, so I just
commented on that, waiting for further comments. I'll just change the
comments myself and push that patch now.
>
> > 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.
>
> Wow.
>
> I think this officially marks my last contribution to the aur under
> the current project leadership.
> As for now, I am done.
>
> Feel free to disregard or refactor any of my outstanding patches.
It's a pity you tend to take things personally. Reading my last mail
again, I also noticed I probably didn't choose a happy wording here.
Please note that English is not my native language and I sometimes fail
to phrase things adequately trying to be clear. Sorry for that.
Anyway, it's your decision. Thanks a lot for your patches and the time
you spent on contributing to the AUR!
I'll have a look at your outstanding patches as soon as possible.
>
> >> +#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
> >
More information about the aur-dev
mailing list