wiki.techinc.nl/includes/objectcache
Timo Tijhof 75aec3686a objectcache: Reduce boilerplate and indirection around makeKey()
== Background

Most of this was introduced in commit 5c335f9d77 (I1eb897c2cea3f5b7).
The original motivation was:

* Ensure wrappers like MultiWriteBagOStuff naturally do the right
  thing. In practice, makeKey() results are interchangeable, with
  the most contrained one (Memcached) also generally used as the first
  tier. However, this is not intuitive and may change in the future.
  To make it more intuitive, the default implemention became known
  as "generic", with proxyCall() responsible for decoding these,
  and then re-encoding them with makeKey() from the respective
  underlying BagOStuff. This meant that MultiWriteBag would no longer
  use the result of the Memcached-formatted cache key and pass it
  to SqlBagOStuff.

* Allow extraction of the key group from a given key cache,
  for use in statistics.

Both motivations remains valid and addressed after this refactor.

== Change

* Remove boilerplate and indirection around makeKey from a dozen
  classes. E.g. copy-paste stubs for makeKey, makeKeyInternal, and
  convertGenericKey.

  Instead, let BagOStuff::makeKey and ::makeKeyInternal hold the
  defaults. I believe this makes the logic easier to find, understand,
  and refer to.

  The three non-default implementations (Memcached, WinCache, Sql)
  now naturally reflect what they are in terms of business logic,
  they are a method override.

  Introduce a single boolean requireConvertGenericKey() to let the
  three non-default implementations signal their need to convert
  keys before use.

* Further improve internal consistently of BagOStuff::makeKeyInternal.

  The logic of genericKeyFromComponents() was moved up into
  BagOStuff::makeKeyInternal. As a result of caling this directly
  from BagOStuff::makeKey(), this code now sees $keyspace and $components
  as separate arguments. To keep the behaviour the same, we would
  have to either unshift $keyspace into $components, or duplicate
  the strtr() call to escape it.

  Instead, excempt keyspace from escaping. This matches how the most
  commonly used BagOStuff implementations (MemcachedBag, and SqlBag)
  already worked for 10+ years, thus this does not introduce any new
  responsibility on callers. In particular, keyspace (not key group)
  is set by MediaWiki core in service wiring to the wiki ID, and so
  is not the concern of individual callers anyway.

* Docs: Explain in proxyCall() why this indirection and complexity
  exists. It lets wrapping classes decode and re-encode keys.

* Docs: Explain the cross-wiki and local-wiki semantics of makeKey
  and makeKeyGlobal, and centralise this and other important docs
  about this method in the place with the most eye balls where it is
  most likely seen and discovered, namely BagOStuff::makeKey.
  Remove partial docs from other places in favour of references to this one.

  Previously, there was no particular reason to follow `@see IStoreKeyEncoder`
  much less to know that it holds critical that communicate the
  responsibility to limit the key group to 48 chars.

* Docs: Consistently refer to the first component as the "key group",
  thus unifying what was known as "key class", "collection",
  "key collection name", or "collection name".

  The term "key group" seems to be what is used by developers in
  conversations for this concept, matching WMF on-boarding docs and
  WMF's Grafana dashboard for WANObjectCache.

Change-Id: I6b3167cac824d8bd8773bc66c386f41e4d380021
2023-08-03 10:42:56 +02:00
..
ObjectCache.php Fix infinite recursion in DBLoadBalancerFactoryConfigBuilder service 2023-05-03 13:39:44 +10:00
SqlBagOStuff.php objectcache: Reduce boilerplate and indirection around makeKey() 2023-08-03 10:42:56 +02:00