On Tue, Jun 21, 2011 at 01:31:45PM -0700, elij wrote:

On Tue, Jun 21, 2011 at 6:33 AM, Lukas Fleischer 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