Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

That's a known issue, I believe it may not be possible to fix it without some kind of variation on #891600: Page and block caches are cleared the same way on cron as on content change in core. In Drupal 6 cache_clear_all() was replacable by cache backends, in D7 it's hard coded in cache.inc and the cache backends themselves can't do much about it.

Might be possible to do a debug_backtrace() from ->clear(), then detect it was called via cache_clear_all() with no arguments vs. the other cases. I didn't have any non-nasty ideas for fixing that in memcache itself.

pillarsdotnet’s picture

I've read the referenced issue as well as #1134242: Review cache_lifetime behaviour. and a few others. I still don't understand why memcache's built-in tests must fail, even on a test site which does not have cron running at all. I'm not even sure what the failures mean, or how they may affect functionality if I use memcache.

If the test failures mean that memcache is currently broken on Drupal 7, then how, exactly, is it broken? How have your large Drupal 7 sites worked around this brokenness?

If the test failures don't mean anything, then why are they not deleted?

How can anyone work on the patch (to be ported) issues if there is no reliable way of determining whether memcache is working, either before or after patching?

If Memcache needs to distinguish between cache_clear_all() and other invocations in order to pass its own tests, where is the patch against includes/cache.inc so that I may review and test it?

catch’s picture

The test failure means that when content is posted, content won't be flushed from the cache - this mainly affects the page cache.

The large D7 site I worked on (not currently) that uses memcache actively avoided cache_clear_all() from node and comment submits because it clears the cache too often on busy sites. Other sites may use varnish for the page cache, and would then not notice this either.

We could comment the tests out until the core bug is fixed, it'd be nicer to get expectedFail() into core to document this stuff better.

http://drupal.org/node/891600#comment-4502492 still describes what needs to happen here, I may work on a patch for that, however I've been hoping that #81461: Clean up the cache API (stop overloading function arguments, remove procedural wrappers) would go in soon (which contains a garbage collection method) then we could backport the relevant bits. Reviews of that patch would be great.

pillarsdotnet’s picture

The test failure means that when content is posted, content won't be flushed from the cache - this mainly affects the page cache.

Does this mean that stale content will be shown to anonymous users? Or does it mean that the cache will contain stale entries that will not be shown?

then we could backport the relevant bits

So after applying the patch from #81461, what changes must be made to memcache.inc in order to get tests to pass?

catch’s picture

It won't be a straight backport. But we'd add a ->garbageCollection() method to memcache.inc, that just returns FALSE, then modify ->clear() to clear the cache when it gets NULL, $bin as arguments (currently that particular combination is ignored).

pillarsdotnet’s picture

then modify ->clear() to clear the cache when it gets NULL, $bin as arguments (currently that particular combination is ignored).

Does the current code allows stale data to be shown to anonymous users, and would the changes you propose correct this error?

catch’s picture

Yes and yes.

However if you were just to make that change now, without the core change, then every single cron run will flush the page cache, which is arguably a worse bug.

pillarsdotnet’s picture

Status: Active » Needs review
FileSize
1.57 KB

So I applied #81461-13 and made the following changes to memcache.inc. Am I on the right track?

EDIT: nope. tests still fail.

pillarsdotnet’s picture

Patch and Test results:

pillarsdotnet’s picture

FileSize
1.77 KB
1.77 KB
1.55 KB

Only three failures with patch from #81461-21: Clean up the cache API (stop overloading function arguments, remove procedural wrappers).

Unfortunately, they're the same three failures as in the unpatched version.

pillarsdotnet’s picture

catch’s picture

#81461 can't be backported wholesale from D8 to D7 because it's too large of an API change. We might be able to backport to http://drupal.org/project/cache_backport but that's a different thing.

I'd like to backport just the garbageCollection() method (and maybe one or two other bits) in core so that it's possible to differentiate between garbage collection and flushing of temporary items, but that depends on the patch getting into first before it can be applied to memcache.

With #1015946: Eliminate $user->cache and {session}.cache in favor of $_SESSION['cache_expiration'][$bin] would be happy just moving to $_SESSION['cache_something'] directly instead of $user->cache regardless of what core does there, but we should fix it for all bins as per #1079518: When one cache table is cleared, cached items from other tables invalidated.

pillarsdotnet’s picture

Status: Needs review » Needs work

@catch -- If you would be kind enough to outline a strategy that would enable memcache to pass its own tests, I'd be happy to continue rolling patches toward that end. Otherwise, I suppose this issue should be closed (won't fix).

pillarsdotnet’s picture

Status: Needs work » Closed (duplicate)