In the current implementation, garbageCollection() performs expiration based on variable cache_flush_$bin being non zero, expecting it to represent a timestamp. However, immediately after noticing a flush is in order, it resets that variable to zero to avoid a flush stampede.

This variable now being 0, the next garbage collections will no longer flush anything until that variable is reset to non-zero, which only happens when clear(NULL, $bin) is invoked (see MongoDBCache::clear()), typically via cache_clear_all(NULL, $bin), or cache_clear_all() on its own for core bins.

However, drupal_flush_all_caches() invokes cache_clear_all('*', $table, TRUE), meaning even a full cache clear will not trigger a garbage collection, but just truncate the core predefined collections and those defined by hook_flush_caches() implementations, and not re-enable the expiration cycle.

Workaround: manually invoke these clears, like drush ev 'cache_clear_all(NULL, "some_bin");' for each bin.

Suggested fix:

  • also reset cache_flush_$bin, in the $cid = '*', $wildcard = TRUE case.
  • implement mongodb_cache_flush_caches() to declare the bins, but this raises the question of identifying them: just relying on the collection name may not be enough, as some developers may name cache collections with names not matching cache_*, or may name non-cache collections with names matching this pattern. A solution could be to add a collection prefix so that the collection name could be relied upon to find cache collections.
  • implement #1802114: Take advantage of TTL collections which would let the server perform the cleanup itself, and would save us having to query the DB in garbageCollection()
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fgm created an issue. See original summary.

fgm’s picture

Issue summary: View changes
fgm’s picture

Status: Active » Needs review
FileSize
21.3 KB

Until #1802114: Take advantage of TTL collections is solved, the suggested patch should fix it.

Since the patch combines some general cleanup (formatting, phpdocs...) with the fix, it is easier to review the actual fix on github : https://github.com/FGM/mongodb/commit/454663f607357d2f41b3fcca1b43491d02...

fgm’s picture

Now improved to also support ill-behaved modules using undeclared bins, even not named cache_*. (met several in the wild yesterday).

Fix without the coding standards changes is at https://github.com/FGM/mongodb/commit/9af8311e831dcd9121d6293d2b5b7ca260...

fgm’s picture

And the complet patch is attached.

fgm’s picture

FileSize
30.37 KB

Now with all core-equivalent tests: all passing except one assertion.

fgm’s picture

Title: Cache collections may never be flushed » Match core flushing logic and ensure expiration always happens

Digging into why one core test didn't pass, I found core change #1015946: Eliminate $user->cache and {session}.cache in favor of $_SESSION['cache_expiration'][$bin] had never been ported so the cache plugin had been failing on cache_lifetime > 0 issues since then (2012, Drupal 7.13). The new patch ports that change, removes some useless code again, and has all core-equivalent tests passing.

At this point, I think this should be RTBC: even if it is not perfect, it fixes the plugin behaviour with nonzero cache lifetime on CACHE_TEMPORARY data.

fgm’s picture

FileSize
31.1 KB
5.8 KB

Patch is easier to review on Github, since this commit contains most of the logic, separated from the miscellaneous phpdoc and coding standards fixes : https://github.com/FGM/mongodb/commit/4affba5198d4b759aa7ea810aff05f3749...

fgm’s picture

Patch is easier to review on Github, since this commit contains most of the logic, separated from the miscellaneous phpdoc and coding standards fixes : https://github.com/FGM/mongodb/commit/4affba5198d4b759aa7ea810aff05f3749...

fgm’s picture

Status: Needs review » Fixed

Committed to 7.x-1.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.