Previously, the refresh probability ramp-up logic assumed that
keys were always assigned logical TTLs >= "lowTTL". Thus, if a
key was saved with a TTL of 3 seconds and "lowTTL" was 10 seconds,
then the probability of a refresh at t=0 seconds would incorrectly
equal the value it should have at t=7.
Also:
* Use class constants for key metadata array keys. This makes it
easy to track the usage of options and is less prone to typos.
* Improve some documentation slightly.
Bug: T264787
Change-Id: I7267e8639617fb8dc0850530465ff9d1b899b592
This makes it easier to rollout one keyspace/project at a time even
if some keys are shared and receive purges. The shared keys can all
be done as the last step.
Also, simplify getMulti() to no longer need extractBaseKey().
Make the "warm up cache" logic a bit easier to follow and less
likely to copy values around.
Change-Id: I8b602ddf5dd1feaada45fb0af202c5603836a8dd
If a key always takes a very long time to regenerate, is popular,
and does not use lockTSE, it still needs to be cacheable. Since a
value cannot be more up-to-date than the time it takes to regenerate
it, take the "lower the TTL" approach for these cases. Use "walltime"
to narrow down the "reject the set()" case based on regeneration time.
This is already provided by getWithSetCallback() automatically.
Bug: T244877
Change-Id: Id43fb02738b28dad3bc922057efb7eee0272d0e1
This is useful for grouping related keys on the same servers to reduce
the need for cache server connections and availability. A cache key that
uses "lockTSE" can already involve accessing several keys during the
read/write cache-aside paths:
a) The value key itself
b) The check key (named after the main key, a common pattern)
c) The mutex key (used if the value looks stale)
d) The cool-off key (used if regeneration took a while)
Any problems accessing the first two could cause extra value regenerations.
Problems with the mutex key could lead to stampedes due to threads assuming
another thread was regerating a soon-to-expire value when, in fact, none was.
A similar problem could happen with cool-off keys, with threads assuming
that another saved the newly regenerated value when, in fact, none did.
The use of hash stops puts the tiny related keys on the same server as the
main cache key that they serve. This is only for hash-based routing, and not
route prefix routing (e.g. All*Route still sends the key to multiple child
routes, but the PoolRoute/HashRoute function will hash differently).
The option is not enabled by default yet.
Change-Id: I37e92a88f356ef1e2a2b7728263040e2f6f09a13
We're observing WANObjectCacheTest fail sometimes, possibly due to a
1/825 chance of a cache key being counted as "popular" combined with an
mt_srand() call making this more likely. I tweaked the options to
getWithSetCallback() to avoid the affected mt_rand() call altogether.
I also added a lowTTL option to another getWithSetCallback() call,
eliminating a condition with a 3% chance. It didn't appear to matter
which branch the test took, but I'd rather avoid non-deterministic
behaviour.
There are a few remaining coin tosses being logged, but they all occur
with a chance of less than 10^-5.
Bug: T238197
Change-Id: Iadd3dc00d44b8490210fec97fe1f49236326300f
To help isolate the referenced bug, which is a sporadic test failure
in testGetWithSetCallback().
Bug: T238197
Change-Id: If35d60340c804b6bfe1e9ddfcf53c76373c794b1
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff
Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
Use the stricter comparisons from assertSame instead of
assertEquals.
In some cases this revealed a test that was asserting a value
different from the one actually returned:
* testSetAndGet:
The test failed with the test case (null, 3), where it tries
to store the value `null`, but gets back `false`, which is wrong.
This appears to be a genuine bug, filed as T234583.
Change-Id: Ic3dcc7fa2e8749b0f2d68917a8ac728dda26b6ca
assertEquals( false, … ) still succeeds when the actual value is 0, null,
an empty string, even an empty array. All these should be reported as a
failure, I would argue.
Note this patch previously also touched assertSame( false ). I reverted
these. The only benefit would have been consistency within this codebase,
but there is no strict reason to prefer one over the other. assertFalse()
and assertSame( false ) are functionally identical.
Change-Id: Ic5f1c7d504e7249002d3184520012e03313137b4
The benefit of using count() is that the test would still succeed if
the return vfalue is not an array, but an iterable object. It seems
this is not needed.
Change-Id: I23529f6990aebe0cce86e236a21820fe74993204
assertEquals( null, … ) still succeeds when the actual value is 0, false,
an empty string, even an empty array. All these should be reported as a
failure, I would argue.
Note this patch previously also touched assertSame( null ). I reverted
these. The only benefit would have been consistency within this codebase,
but there is no strict reason to prefer one over the other. assertNull()
and assertSame( null ) are functionally identical.
Change-Id: I92102e833a8bc6af90b9516826abf111e2b79aac
assertSame() is guaranteed to not do any type conversion. This can be
critical when acciden tially comparing, for example, 0 to 0.0.
Change-Id: Iffcc9bda69573623ba14af655dcd697d0fcce525
This should help fix the following issues that various repos
are getting from Phan as of late:
> Call with 5 arg(s) to \BagOStuff::makeKey() which only takes 2 arg(s)
> defined at ../../includes/libs/objectcache/BagOStuff.php:456
> <source="PhanParamTooMany"/>
Bug: T228563
Depends-On: I5cfba063821101325a5a7359e6b8ad71a0fb1b2f
Depends-On: Ifa5b96735376f2fbe3680799f960616ba8d357ff
Change-Id: Ic9df7f3ad7f356c7cbdfe1edfbe35821b931dda6
As long as get()s are disallowed from the process cache, the sets() should
at least still be up-to-date, so there is little reason to prevent them.
Change-Id: Ic62c8380801130de7f8412cddcf85b246e33b3cd
Add WANObjectCache::multiRemap() as an array_combine() wrapper for
easily working with IDs after getMultiWith(Union)SetCallback() calls.
Make the enforcement of uniqueness in makeMultiKeys() stricter and
discourage poor key design in comments. Add WANObjectCache::hash256()
method for getting good key component hashes.
Also avoid pointless use of ArrayIterator::getArrayCopy().
Change-Id: I61ffdbf4af4374864bac180df590b4dddc8da56b
Add FLD_VALUE_VERSION key to the value wrapper array to hold the version
number used in getWithSetCallback(). Remove the VFLD_* wrapper array from
FLD_VALUE for versioned values.
Keys stored with the old VFLD_VERSION and VFLD_DATA fields will be seen as
having the wrong version. The previous WAN cache code will see the new keys
that use FLD_VALUE_VERSION as having the wrong version too. In either case,
the usual variant key logic applies, so there should not be any issues.
This means that moving from a non-versioned to a versioned cache key is no
longer a breaking change when, for the same key, some code passes a version
number to getWithSetCallback() while other code does not.
Also:
* Make "pcTTL" respect the version number for sanity
* Make sure set() respects TTL_UNCACHEABLE for completeness
* Track slow regeneration callback runtime in FLD_GENERATION_TIME
* Remove is_callable() check overhead and rely on PHP Error instances
* Refactor unwrap() to return a more immediately useful value
* Simplify getNonProcessCachedKeys() signature by using $opts
* Split out PURGE_* constants for purge entries since those keys are
never stored in any serialize value but are only in PHP arrays
* Rename doGetWithSetCallback() to be more succinct
* Rename and reorganize some variables for clarity
Change-Id: I4060b19583cdfd9fa36c91d7014441eeef4b3609
This changeset implements T89432 and related tickets and is based on exploration
done at the Prague Hackathon. The goal is to identify tests in MediaWiki core
that can be run without having to install & configure MediaWiki and its dependencies,
and provide a way to execute these tests via the standard phpunit entry point,
allowing for faster development and integration with existing tooling like IDEs.
The initial set of tests that met these criteria were identified using the work Amir did in
I88822667693d9e00ac3d4639c87bc24e5083e5e8. These tests were then moved into a new subdirectory
under phpunit/ and organized into a separate test suite. The environment for this suite
is set up via a PHPUnit bootstrap file without a custom entry point.
You can execute these tests by running:
$ vendor/bin/phpunit -d memory_limit=512M -c tests/phpunit/unit-tests.xml
Bug: T89432
Bug: T87781
Bug: T84948
Change-Id: Iad01033a0548afd4d2a6f2c1ef6fcc9debf72c0d
Avoid the ADD operation spam from all threads trying to access
a tombstoned key by checking the interim value cache timestamp.
This also avoids the GET/CAS spam from threads that manage to
get the mutex. If a single thread repeatedly accesses the same
tombstoned value in rapid succession, there will significantly
less cache operation spam.
Do the same for cache updates to keys in the holdoff state
due to "check keys" or the "touchedCallback" function.
Relatedly, fix getWithSetCallback() to disregard interim values
set prior to or at the same time as the latest delete() call.
This can slightly reduce the chance of the cache being behind
replica DBs for a second. It also avoids unit test failures
were a series of deletes and cache access happen at the same
timestamp (via time injection or regular system time calls).
In addition:
* Add PASS_BY_REF flag with backwards compatibility to avoid
bloating the signature of get()/getMulti() with the new
tombstone information needed for the above changes.
* Avoid confusing pass-by-reference in getInterimValue() and
fix use of incorrect $asOf parameter.
* Move some more logic into setInterimValue().
* Update some comments regarding broadcasted operations that
were not true for the currently assumed mcrouter setup.
* Rename $cValue => $curValue and $versioned => $needsVersion
for better readability.
Bug: T203786
Change-Id: I0eb3f9b697193d39a70dd3c0967311ad7e194f20
Instead, make the "high read lag with lockTSE" case just lower FLD_TTL.
This also avoids constant regenerations by threads getting the mutex.
Added logging for TTL adjustments in set() for the lockTSE case.
Also remove some delete() calls from tests that were not needed.
Change-Id: Id7695f0377235e4a2f6e0efc88e870c8a990c3b0
* Remove 'channels' field references from config/setup
* Remove 'relayer'/'channels' field reference in unit tests
* Remove unused DEFAULT_PURGE_CHANNEL class constant
* Also remove long-since bogus 'pool' field references
Follow-up to 4753b0a4ed
Change-Id: If6670ff4e1dccc8ae253a08b46d205601da10024
* Fix the timestamps to static hard-coded values
* Force the timestamps before various get/set tests so they do not
use the microtime() value either.
* Remove the direct and duplicated (causing further risk) microtime()
calls from testGetWithSeveralCheckKeys().
Bug: T207247
Change-Id: Id30a8127f11501dbe54e075b6e9d18490353f4a5
This is useful when the timestamps to be checked depend on the value or are stored
in the database rather than as check keys.
Change-Id: I81ab08a943ee7d2f96a132d371965501941ed37f
This can be used to discard all values before a certain point in
time, such as periods of severe network problems or misconfiguration
that resulted in missed purges.
Change-Id: I612db8f91a5960b912e9f35645a3d3872df47460
