Commit graph

44 commits

Author SHA1 Message Date
Matthew Flaschen
c4e698dc27 Follow-up cdc93a62bf: add serialize/unserialize tests for RedisBagOStuff
Bug: T134923
Change-Id: I0a7bec40c3fbf301720a248fbe21d46f5e64154c
2016-05-12 22:41:16 +00:00
Kunal Mehta
6e9b4f0e9c Convert all array() syntax to []
Per wikitech-l consensus:
 https://lists.wikimedia.org/pipermail/wikitech-l/2016-February/084821.html

Notes:
* Disabled CallTimePassByReference due to false positives (T127163)

Change-Id: I2c8ce713ce6600a0bb7bf67537c87044c7a45c4b
2016-02-17 01:33:00 -08:00
Aaron Schulz
d2d935483f Make makeKeyInternal() limit more conservative
This should avoid error log entries for long WAN cache keys

Change-Id: I401482d25dd5bf47052a3c6729c5f8bc9fd68770
2015-11-01 19:37:21 -08:00
Thiemo Mättig
e6cf3aa0b3 Add tests for MemcachedBagOStuff::validateKeyEncoding
If3e20c6 and the following patches introduced a breaking change and
cause a regression in Wikibase because we are using the version number
constant as part of a cache key prefix. Currently the Wikibase version
is set to "0.5 alpha".

Space characters were allowed before and encoded as "%20". This does
not happen any more.

Change-Id: Ia2fd4ed6738a10e02050bced947ef5d4e8b98980
2015-10-28 18:12:35 +01:00
Aaron Schulz
cce813a922 Move MultiWriteBagOStuff to /libs
Also moved related tests files to /libs.

Change-Id: I806eeaa30205733d497adde933baf0c4157f7aae
2015-10-24 12:15:26 -07:00
Aaron Schulz
0877963f95 Fixes to MemcachedBagOStuff::makeKeyInternal()
* Follow-up to 0c9fb12265
* Make sure colons actually get escaped
* Added more unit tests
* Also fixed the test actual/expected order

Change-Id: Ie04ea6059ee1eb6d1da8f30fefdec52fa49d38fb
2015-10-24 04:53:20 +00:00
Ori Livneh
0c9fb12265 Escape colons in BagOStuff key segments
For the sake of safety and correctness, the following BagOStuff::makeKey()
invocations should return distinct keys:

   $cache->makeKey( 'ab:', 'cd' );
   $cache->makeKey( 'ab', ':cd' );

That is not currently the case, because while we use ':' as a key path
separator, we don't escape ':' in the input supplied to makeKey(). So, make
BagOStuff::makeKeyInternal() URL-encode colons.

To prevent this from messing up the max. key length calculations, reproduce
this logic in MemcachedBagOStuff::makeKeyInternal(), in lieu of having the
method call its parent.

Change-Id: I83ea7e7336a1c9e64aa42284c2517089a736efe5
2015-10-23 20:26:49 -07:00
Ori Livneh
cdb5432728 Ensure all key transformations are applied by BagOStuff::makeKeyInternal()
Currently we have to undo any transformations we apply to keys in getMulti() by
iterating over the response array keys reversing any changes. This is hairy and
complicated. So --

* Replace calls to MemcachedBagOStuff::encodeKey() with calls to a new method,
  MemcachedBagOStuff::validateKeyEncoding(). The new method only validates that
  the key is compatible with memcached. If it is not, it throws an exception.
  If it is, it returns the key unmodified.

* Remove MemcachedBagOStuff::{encode,decode}Key(), since they no longer serve a
  purpose, and have no callers.

Change-Id: If3e20c6a1a1b42fc1f2823aa660328e37c26eccb
2015-10-23 18:46:04 -07:00
Ori Livneh
bf11cee6c5 Improve normalization and sanitization of memcached keys
The motivation for this patch came from seeing the following error in the
memcached log:

  memcached ERROR: Memcached error for key "flowdb:flow_ref:wiki:by-source:\
   v3:0:tawikinews:பூமிக்கு_அச்சுறுத்தலான_சிறுகோள்களைக்_கண்டுபிடிக்கும்_முயற்சியில்_தனியார்_நிறுவன0jம்:4.7" \
   on server ":": A BAD KEY WAS PROVIDED/CHARACTERS OUT OF RANGE

I submitted a fix to Flow in I26e531f6, but I noticed that AbuseFilter had a
similar issue (fixed by Aaron in I27b51a4b), so I started thinking about how
to solve this more generally:

* The regular expression we current use to sanitize keys does not cover
  characters outside the ASCII range, but such characters can be illegal
  if one of their constituent bytes (when taken by itself) is an ASCII
  control character. So change the regular expression to cover any and all
  characters that fall outside the range \x22-\x7e (and '#' -- see below).

* Enforce a key length limit of 255 bytes, which is the maximum length
  permitted by the memcached protocol. The Tamil segment in the key above is 84
  characters, but 233 bytes in UTF-8, which become 684 characters when
  URL-encoded. To fix this, try to shrink any segment that would push the total
  key length over the limit by md5()ing it. If the end result is *still* over
  the limit (this would happen if, for example, $args consists of many short
  strings), then concatenate all args together and MD5 them.

* MD5'd arguments are prefixed with '#'. Any "organic" '#'s in the key segments
  are URL-encoded.

Change-Id: Ia46987d3b0a09bb6b1952abd936d4c72ea7c56a0
2015-10-23 16:21:04 -07:00
Aaron Schulz
5c8ef13306 Add WRITE_SYNC flag to BagOStuff::set()/merge()
* This blocks on writing to all replicas
  and returns false if any failed.
* This is useful if ChronologyProtector is to work across
  domains by having the writes go everywhere so that later
  reads will see them (and be local at the same time).
* Redundant doc comments were also removed.

Change-Id: I9ed098d563c64dba605e7809bc96731da3b3e79d
2015-10-22 01:44:09 +00:00
Aaron Schulz
1171cc00cd Inject MultiWriteBagOStuff addCallableUpdate() dependency
Inject the DeferredUpdates::addCallableUpdate method via the
ObjectCache. This brings it closer to being able to move to /libs.

Change-Id: Ifa0d893002c3d709a4dc7346c263a92162274bd7
2015-10-20 10:31:36 -07:00
Aaron Schulz
f84b32af37 objectcache: Remove getWithSetCallback() signature backwards-compatability
All callers have been migrated

Change-Id: I7b6b87594dd724434ba24d8206fe07d66c1d5d25
2015-10-19 23:47:04 +00:00
Ori Livneh
6916548490 Add makeKey and makeGlobalKey to BagOStuff
* Add a string `keyspace` member to BagOStuff instances. The default
  implementation, meant for simple key/value stores, treats the key space
  as a string prefix to prepend to keys. By default, its value is `local`,
  but any instance created via ObjectCache::newFromParams() (or or one of
  its callers) will have that default to $wgCachePrefix / wfWikiID().
* Add `makeKey` and `makeGlobalKey` methods to the base BagOStuff class.
  These methods are not static to allow for BagOStuff types which require
  a configured instance to know the underlying storage engine's key semantics.
* Make wfMemcKey() and wfGlobalCacheKey() delegate to these methods on the main
  ObjectCache instance.

Change-Id: Ib7fc2f939be3decfa97f66af8c2431c51039905f
2015-10-13 09:32:48 -04:00
Aaron Schulz
968e28f894 Add process cache support to WANObjectCache
* Make getWithSetCallback() accept a TTL field for this
* Make ChangeTag callers use this flag to avoid hundreds of
  duplicate queries at Special:Tags

Change-Id: Ic1ed28294f5d557e02f39a7f20d36766244b9ded
2015-10-10 06:14:50 +00:00
Timo Tijhof
09cbebb224 WANObjectCache: Change getWithSetCallback() signature to key/ttl/callback/opts
* Put 'checkKeys' param in opts array instead of as a separate parameter.
  It's neat to not have to skip unnamed/positional arguments that are optional.
* Move TTL to be before callback instead of after. This avoids dangling integers
  toward the bottom of a large code block that have no obvious meaning.
  Matches BagOStuff::getWithSetCallback.

Add unit test for lockTSE to confirm

Change-Id: I60d6b64fc5dff14108741bafe801fd1fde16d1df
2015-10-07 18:03:15 -07:00
Timo Tijhof
14788f9aab objectcache: Fix flaky unit test
Change-Id: Id7d3b75248a42e2e82a51d70a3ee1479f0507685
2015-10-07 17:23:46 -07:00
Aaron Schulz
355ba8530e objectcache: Add BagOStuff::getWithSetCallback() convenience method
Change-Id: I9cc162ff1cc48c1c500f2999327bd18ba235bfd0
2015-10-05 17:26:32 -07:00
Amir E. Aharoni
b59ad5811b Make lines short to pass phpcs in BagOStuffTest.php
Bug: T102614
Change-Id: I125cf7ec294818ae4c7741acf24a40980448daa9
2015-09-30 16:01:54 +00:00
Aaron Schulz
aaa7df96a4 Added testStaleSet() WAN cache test
Change-Id: Ib8565c48af09af0df1809e877fde9d4d6c7e7038
2015-09-29 18:14:48 -07:00
Matthias Mullie
8ca796ea99 Fix Memcached key decode
Flow had a key: flowdb:flow_ref:wiki:by-source:v3:Parser\'s_"broken"_+_(page)_&_grill:testwiki:1:4.7
the '+' in there was not being encoded (it only does /[\x00-\x20\x25\x7f]+/)
but coming back, it was decoded into ' '.
getMulti() shows a key=>value array or results. Since key was different,
we couldn't find what we had requested.

Bug: T110326
Change-Id: Ia92edd73d0eb7fe0d35e38e7e7af8174fb85cbcc
2015-09-28 19:37:53 +02:00
Aaron Schulz
fd8e85fc2d Support async writes to secondary MultiWriteBagOStuff stores
* This is useful for ParserCache, as it tries to focus on memcached
  and use other caches (e.g. mariadb) for the long-tail of less used
  content, as setup on WMF. The class uses BagOStuff in a way that is
  compatible with this approach.

Bug: T109751
Change-Id: Ia64eb44a9b52a988fde27b468d604d9163bed4b4
2015-09-18 21:35:43 -07:00
Matthias Mullie
58c81f5b86 Decode Memcached keys before returning getMulti result
Keys are sent to Memcached encoded. However, getMulti()
will respond in [key => value] format. The keys it
responds with should not be the encoded versions, or
callers won't be able to map them to the results.

Bug: T111138
Change-Id: I0d821b1219a492be8e93453f0249c78f18e24533
2015-09-07 05:43:49 +00:00
Aaron Schulz
c9b1fe1896 Added reentrant lock support to BagOStuff
* Also fixed HashBagOStuff::delete() return value for non-keys

Change-Id: I9a977750c6fc6b8406bc1c9366a6b1b34bf48b6b
2015-08-25 01:17:45 +00:00
Aaron Schulz
97c7a897c8 Avoid some possible deadlocks on account creation
* This uses a non-blocking $wgMemc lock to reserve the user
  name in question. This should prevent two threads from
  reaching LOCK IN SHARE MODE and getting stuck on INSERT.
  The lock is global to better cover auth plugins.
* This adds a BagOStuff::getScopedLock() convenience method.
  It uses less queries than LockManager by being EX only.
* Avoid extra lock attempt in lock() in non-blocking mode.
* Removed (un)lock() HashBagOStuff overrides that made it
  behave differently than other caches (wrt to re-entrance).

Bug: T106850
Change-Id: Iecf95206d712367f5d202f76ab0eaa9d7bdabf2b
2015-08-17 21:33:50 -07:00
Aaron Schulz
7d5f3efa2f objectcache: Add WANObjectCache::resetCheckKey() method
Change-Id: I6f8b97c1f4511534e1ab2656f472adee491f9d9f
2015-07-13 21:25:20 +00:00
Aaron Schulz
84758e6f10 Made WANObjectCache::getCheckKey() automatically init the key
* This still allows If-Modified-Since logic but does not
  need to broadcast initialization values just because a
  key fell out of cache. The value can differ between DCs
  anyway via skew, this just lets them drift more. Actual
  purge events are still broadcasted, which is what matters.
* The User class has now been simplified given this change.
* Added more general comments to getCheckKeyTime().

Change-Id: Ic1f4bbb1947e0d1dd47499c9e9dc86991c30580c
2015-07-02 07:13:22 +00:00
Timo Tijhof
0d98e9f2fb objectcache: Add tests for ReplicatedBagOStuff
Fixed PHP runtime warnings:
> Declaration of ReplicatedBagOStuff::getMulti() should be compatible with BagOStuff.
> Declaration of ReplicatedBagOStuff::decr() should be compatible with BagOStuff.

Change-Id: Icf1a0bf2c30408c4a5bef2de0b69ae2162b234d5
2015-06-24 20:17:18 +01:00
umherirrender
d8821f2b0b Fixed spacing
- Removed space after casts
- Removed spaces in array index
- Added spaces around string concat
- Added space after words: switch, foreach
- else if -> elseif
- Removed parentheses around require_once, because it is not a function
- Added newline at end of file
- Removed double spaces
- Added spaces around operations
- Removed repeated newlines

Bug: T102609
Change-Id: Ib860222b24f8ad8e9062cd4dc42ec88dc63fb49e
2015-06-17 20:22:32 +00:00
Vivek Ghaisas
9f5b6f5aeb Fix whitespace issues around parentheses
Fix issues found by MediaWiki.WhiteSpace.SpaceyParenthesis sniff.

Bug: T102617
Change-Id: Iec7f71e64081659fba373ec20d9d2006306a98f4
2015-06-16 22:14:02 +03:00
Vivek Ghaisas
665f1a033d Ensure that files end with a newline
Bug: T102619
Change-Id: Iae6e722151581d15c9421d41c4d14b100bb6e437
2015-06-16 16:21:57 +03:00
Aaron Schulz
4db819011a Allow for dynamic TTLs in getWithSetCallback()
* This gives it better parity with BagOStuff::set()
* Also updated some doc comments

Change-Id: Ib94b97715fae901bac1750656e3dc7501919d6d2
2015-05-12 13:04:53 -07:00
Aaron Schulz
3a1f8b1111 Added WANObjectCache class
This class handles caching across distance sites where purges
must reach both. It also aims to make purging more reliable in
the face of network glitches and node consistent hash ejection.

bug: T88492
Change-Id: I686811b3075bf22e2f4de45127e8461e54648ead
2015-04-23 21:36:42 +00:00
Timo Tijhof
5754e1a9f9 objectcache: Add @covers for BagOStuffTest
Change-Id: I93a8074ba79b5ea66a984edabe009cc828e6fc4d
2015-04-05 16:02:53 +01:00
Marius Hoch
3cbaac08cd MediaWikiTestCase: Enforce children call parent::tearDown
Can't be implemented as a test case because that's not being
run before MediaWikiTestCase::tearDown.
Also there's no parent destructor, so no need to call one.

Also fixed BagOStuffTest.

Change-Id: Ifd8659b6c8a748c6716099f8822e2c50ab51bda7
2014-09-14 21:21:40 +02:00
addshore
892a992ab8 Remove MediaWikiPHPUnitCommand
All functionality has been moved to other places

Change-Id: I6b6b0ef846bc63108c4dff9e17098432fd9d1697
2014-08-02 21:35:34 +00:00
Siebrand Mazeland
5a16395d8e Pass phpcs-strict on some test files (9/11)
Change-Id: I69f17bf2af45f03274fdb38853184880e46c3514
2014-04-24 13:50:47 -07:00
addshore
f86db05d8d Remove unused stuff from tests
Change-Id: Iddabfbc80378b02fa4a2c58f80d50241be8105d3
2014-03-07 21:16:32 +00:00
Matthew Flaschen
613c451286 Fix doc error in new incr test
It covers all implementations of the abstract class method BagOStuff::incr.

Change-Id: I763985477b8564857a0905019c4485dd36f84539
2013-10-23 16:56:05 +00:00
Matthew Flaschen
7cff82a118 Test BagOStuff->incr method
Change-Id: I3b72f19df82ee302dee47dcf22b69ed9bb6ff8e0
2013-10-22 15:48:50 -04:00
Siebrand Mazeland
791d0b2a98 Update code formatting
Change-Id: I16a9b42651f1cfb1a70dffbb67b7b83dfeb90d03
2013-04-26 14:21:20 +00:00
Matthias Mullie
a8f00e5c97 Read full memcached response before manipulating data
Memcached response when fetching data typically looks like this:
VALUE <the stored value for whatever key you requested>
END

What the code used to do is read the first line (the VALUE) and re-
assemble the data being fetched there (like unserializing serialized
data). After that, it will read the next line (END).

The value could be a serialized object, which could have a __wakeup.
This __wakeup could have code which in turn executes Memcached-
related stuff. The problem is that, while that object is being
unserialized already, it's wakeup code is attempting to read new
stuff from Memcached, but we have yet to read the END of the data
we're attempting to unserialize (when we'll read a new value from
Memcached, the first thing we'd get is the END we have not yet read..)

The correct way to go about this would be to first read the full
Memcached response, and only unserialize the read data after that.
This is exactly what this patchset does.

Change-Id: I902809c6dde657091c8161a09df823170bd41f7a
2013-03-07 15:59:10 +01:00
Siebrand Mazeland
49dfbc59d0 Update formatting
6 of n.

Change-Id: I0ca3f1f72349623631ce1d7f3a4e2ed5edbdbdf4
2013-02-15 12:44:42 +00:00
Antoine Musso
0fd05285d7 pass codesniffer on tests/
Fix almost all occurences of the following sniffs:

Generic.CodeAnalysis.UselessOverridingMethod.Found
Generic.Formatting.NoSpaceAfterCast.SpaceFound
Generic.Functions.FunctionCallArgumentSpacing.SpaceBeforeComma
Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine
Generic.PHP.LowerCaseConstant.Found
PSR2.Classes.PropertyDeclaration.ScopeMissing
PSR2.Files.EndFileNewline.TooMany
PSR2.Methods.MethodDeclaration.StaticBeforeVisibility

Change-Id: I96aacef5bafe5a2bca659744fba1380999cfc37d
2013-01-28 12:14:26 +01:00
Matthias Mullie
74f581e348 Added merge() function to BagOStuff for CAS-like functionality.
* merge() will use CAS if supported or use locking otherwise
* The lock()/unlock() methods now have a default implementation
* added unit tests for merge

Change-Id: Ic27088488f8532f149cb4b36e156516f22880134
2013-01-10 09:03:09 +01:00