Self-diagnosed and fixed W3 Total Cache bug in faulty object caching

  • Evening folks and Frederick,

    As some of you may know, there has been an issue with object caching not returning values and various side effects like WP-Cron not running lingering in W3TC code for, I believe, years (I may be wrong, but some of the posts I found go back that far).

    I spent several days tracking down the bug inside Memcached caching for object caching not working, and finally with the help of a live remote debugger, I’ve figured out where the bug is.

    The bug and diagnosis

    Relevant files in W3TC (0.9.4 as of today):

    • lib/W3/Cache/Base.php
    • lib/W3/Cache/Memcached.php
    • lib/W3/Cache/ObjectCache.php

    To recap, a large arbitrary portion of the cached objects aren’t returned correctly. As I found out, they are, in fact, in memcached, but there is a bug with the so called “key_version” which W3 uses to split the values into different buckets that has erroneous logic that ends up setting value V into bucket with key_version A but when getting it, it ends up using a bucket with key_version B.

    This in turn means that values that are continuously getting set aren’t retrieved and are then set again, over and over, many times a second in some cases. Outside of the fact that caching plain doesn’t work, that also means evictions are much more frequent and end up in expiring values that do happen to work much-much faster.

    In actuality, it all comes down to the default values used for the variable $group in various getters and setters in lib/W3/Cache/Memcached.php (by default $group = 0) and its parent lib/W3/Cache/Base.php (by default $group = ”).

    Both lib/W3/Cache/Memcached.php and lib/W3/Cache/Base.php contain set() but only lib/W3/Cache/Memcached.php contains get(). Because of this, the $group getting passed to the function that calculates the right bucket and key_version is “0” for set() and ” for get(). At this point the get() function throws away the correctly returned from memcached value and returns null because key_version doesn’t match.

    The fix

    The quick and dirty fix is to add the missing get() function into lib/W3/Cache/Memcached.php, like so:

    function get($key, $group = '0') {
  return parent::get($key, $group);
}

    This will realign the $group defaults to be the same for get() and set(). I’ve verified that this fix works.

    This quick fix above, however, isn’t 100% complete, as I believe the grouping functionality remains broken altogether since it basically forces only two groups with keys ‘0’ and ” (empty), but at least it fixes this major bug because the data is set and gotten from the same groups. I’m going to post the other part of the answer in a follow-up reply to split things up a bit.

    https://www.ads-software.com/plugins/w3-total-cache/

Viewing 8 replies - 1 through 8 (of 8 total)
  • Thread Starter archon810

    (@archon810)

    Now for part 2.

    Relevant files:

    • lib/W3/Cache/Base.php
    • lib/W3/Cache/Memcached.php
    • lib/W3/Cache/ObjectCache.php

    The solution above basically means that only 2 buckets with keys ” (blank) and 0 will exist. The cached data will be split between these 2 buckets:

    [20-Apr-2014 00:42:20 UTC] APDEBUG setting key version for group  to 44
[20-Apr-2014 00:42:20 UTC] APDEBUG setting key version for group 0 to 14
[20-Apr-2014 00:42:20 UTC] APDEBUG setting key version for group  to 44
[20-Apr-2014 00:42:20 UTC] APDEBUG setting key version for group 0 to 14

    That seems to work, but the intention of the code that deals with object caching is to make the buckets correspond to groups (like ‘options’, ‘transient’, etc), at least that’s what I gather from reading the code.

    The main problem is that the $group value isn’t passed from ObjectCache.php to the respective plugins, such as Apc.php, Memcached.php, etc. This means that the groups for all objects will be set to the defaults, hence 0 and 1 (see part 1 above for why we have the discrepancy in the first place).

    I think the right fix is to also pass the $group value along as well. I’m not sure my patch is 100% complete – it should still be checked by a core developer, but the idea is to do this:

    Index: lib/W3/ObjectCache.php
===================================================================
--- lib/W3/ObjectCache.php      (revision 891523)
+++ lib/W3/ObjectCache.php      (working copy)
@@ -164,7 +164,7 @@
                   !in_array($group, $this->nonpersistent_groups) &&
                     $this->_check_can_cache_runtime($group)) {
             $cache = $this->_get_cache(null, $group);
-            $v = $cache->get($key);
+            $v = $cache->get($key, $group);
             if (is_array($v) && $v['content'] != null)
                 $value = $v['content'];
             else
@@ -234,7 +234,7 @@

         $this->cache[$key] = $data;

-        if ($this->_caching &&
+        if ($this->_caching &&
                 !in_array($group, $this->nonpersistent_groups) &&
                 $this->_check_can_cache_runtime($group)) {
             $cache = $this->_get_cache(null, $group);
@@ -251,7 +251,7 @@

             $v = array('content' => $data);
             return $cache->set($key, $v,
-                ($expire ? $expire : $this->_lifetime));
+                ($expire ? $expire : $this->_lifetime), $group);
         }

         return true;
@@ -277,7 +277,7 @@
         if ($this->_caching && !in_array($group, $this->nonpersistent_groups)) {
             $cache = $this->_get_cache(null, $group);

-            return $cache->delete($key);
+            return $cache->delete($key, $group);
         }

         return true;

    At this point, going back to the log produces something like:

    [20-Apr-2014 01:00:39 UTC] APDEBUG setting key version for group post_tag to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group posts to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group category to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group post_tag_relationships to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group category_relationships to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group post_format_relationships to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group post_meta to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group users to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group userlogins to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group useremail to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group userslugs to
[20-Apr-2014 01:00:40 UTC] APDEBUG setting key version for group user_meta to
[20-Apr-2014 01:07:45 UTC] APDEBUG setting key version for group default to 15
[20-Apr-2014 01:07:45 UTC] APDEBUG setting key version for group options to 15
[20-Apr-2014 01:07:46 UTC] APDEBUG setting key version for group transient to 15

    I’m too tired to figure out if it’s a good or a bad thing that the key versions are the same for different groups, but at least they seem to be consistent across get() and set().

    I’m hoping Frederick, who I’ve been trying to reach to work on this bug, or someone from the W3TC dev team will see this, respond, and evaluate.

    Thank you and good luck, everyone.

    Thread Starter archon810

    (@archon810)

    Just for fun, guess when the object caching bug was fixed in our setup over at androidpolice.com.

    db server: https://i.imgur.com/yJ0wwhk.png
    web server 1: https://i.imgur.com/RFQR4tL.png https://i.imgur.com/EjH2xcT.png

    Thread Starter archon810

    (@archon810)

    Frederick, I think you meant to respond here instead of https://www.ads-software.com/support/topic/suggested-changes-to-object-caching-and-transients?replies=3? That thread could still use a proper reply as well if you have time.

    @archon810

    fwiw, i put your quick and dirty fix into Memcached.php and it fixed an issue that i was having running woocommerce with w3TC, letting me turn on object caching for the first time. thanks!

    are you running the patch for ObjectCache.php you listed above in production? If so, how is it working for you?

    Thread Starter archon810

    (@archon810)

    Correct, I’ve been running it ever since. It’s working relatively well, and definitely resolves this major bug. But object caching itself is still wonky sometimes, like when stickying posts, for example. It’s like the cache doesn’t get updated/dumped for a longer period of time. I usually run without object caching because of that, saves me a lot of headaches. Only turn it back on when things get really heated up with traffic.

    @archon810, thanks for your work on this.

    I’m curious if anyone has tested the “more complete” fixes, or just the get() patch?

    I look forward to seeing this included in a proper W3TC release.

    Any new on this? It does not look like it’s been adressed even though Frederick Townes’ first reply on https://www.ads-software.com/support/topic/suggested-changes-to-object-caching-and-transients suggests it would be…

    Nothing that I’ve seen. I reached out to Frederick via the w3-edge contact form twice.

    I’d at least like to know if this was being worked on for a future release or if development has stopped in favor of more profitable ventures.

Viewing 8 replies - 1 through 8 (of 8 total)
  • The topic ‘Self-diagnosed and fixed W3 Total Cache bug in faulty object caching’ is closed to new replies.