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()
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff-2547411-6-8.diff | 5.8 KB | fgm |
#8 | 2547411-flush-8.patch | 31.1 KB | fgm |
#6 | 2547411-flush-6.patch | 30.37 KB | fgm |
Comments
Comment #2
fgmComment #3
fgmUntil #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...
Comment #4
fgmNow 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...
Comment #5
fgmAnd the complet patch is attached.
Comment #6
fgmNow with all core-equivalent tests: all passing except one assertion.
Comment #7
fgmDigging 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.Comment #8
fgmPatch 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...
Comment #9
fgmPatch 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...
Comment #10
fgmCommitted to 7.x-1.x.