Page MenuHomePhabricator

Standardise lock management in MediaWiki
Open, LowestPublic

Description

Status quo

The status quo is that we tend to acquire locks in whatever we have handy in the same component. This has some short-term benefits in terms of learning curve. For example, if a component X is primary reading-writing things from a database with Rdbms, then it can use that same database to perform any locks it might need to coordinate work between requests. Likewise, if a component Y is primarily computing data and caching it in memcached with BagOStuff, it might as well use memcached locks to coordinate writes between requests when needed.

There are however limitations to this, which leads to cases where we currently reach for other solutions. Let's start with a list of components we have at the moment inside MediaWiki that are capable of providing locks:

  1. Rdbms (MySQL, Postgres, Sqlite): Our database interface is primarly used for reading and writing rows in our various database tables. Our supported database types all support named locks, and so IDatabase::lock in Rdbms provide access to this capability in a way that naturally translates to this native database capability.
    • Failsafe: Deny all locks, throws exception. This is appropriate as "low tier" solution, with relatively low demand/traffic, where you can afford to enforce uniqueness by lock as a hard requirement (i.e. hundreds per minute, not thousands per second).
    • Capacity: Low.
    • Availability: High.
  2. BagOStuff (Null, Memcached, Redis, APCu): Our cache interface for storing ephemeral key-value data. These typically don't have an explicit "lock" feature, but, it is easy to emulate this by storing a temporary key like foobar-my-lock with a value of 1 and checking whether that exists. And so BagOStuff::lock provides this behind an easy to use interface.
    • Failsafe: Grant all locks. This is a "high tier" solution (can handle large loads, with best-effort guruantees). The failure mode, including the default when no cache backend is provided, is to grant all locks (i.e. no keys appear to exist).
    • Capacity: High.
    • Availability: Medium.
  3. PoolCounter (Null, Redis, poolcounterd): This is a dedicated MediaWiki component specifically for locks and locks only. It was developed in 2009 by @tstarling following the Michael Jackson effect. The locks are kept in memory in one of several independent central poolcounterd processes. It is designed for the highest uptime and load capacity. Locks are hashed/mapped to 1 of 3 hosts, with fallback to the other two in order in case of downtime. It protects the servers against an overload from demands for specific unavailable content, by ensuring server capacity is never allocated all to the same task, thus making sure capacity remains available for other page views. Despite being in the critical path for page views, we have to trust it enough that we can failsafe to rendering an error (or stale content) instead of best-effort rendering something anyway. This is made possible through the service being implemented in a few hundred lines of stable C code, with no other responsibilities. The caveat here is htat PoolCounter is an optional dependency and so by default we use a Null placeholder where by all locks are granted. This makes sense for the purpose of PoolCounter, since it is something you won't need until you're pretty large in scale (e.g. Wikipedia). However, lack of a default means we shy away from this as our go-to solution. We only use as extra layer for things important to Wikipedia.
    • Failsafe: Deny. Render stale content or display an error.
    • Capacity: Highest.
    • Availability: Highest.
  4. LockManager (Files, Redis, Memcached): This is a dedicated MediaWiki component specifically for locks and locks only. It was developed specifically for MediaWiki FileBackend and is used as part of the multimedia upload process (e.g. when uploading photos to Wikimedia Commons). It was created in 2011 by @aaron when MediaWiki evolved to store files beyond a local or NFS-mounted hard drive for photos in favour of a dedicated Swift cluster (wikitech: Media storage § History). Before that point, FileBackend would lock file paths using file operations directly on the local/NFS disk. This is coded in a way that achieved both high availability and high guarantees. That it has a sensible default (based on local files in the hard drive), but has an interchangable Redis implementation as well. The Redis implementation uses quorums so that it isn't merely best-effort based on the responsiveness and uptime of a single Redis server. Instead of sharding keys across the cluster (like you would for caching), it uses quorums to increase uptime and offer higher guruantees. The another notable aspect of LockManager is that it allows acquiring multiple locks at once in a way that reliably grants you all or nothing.
    • Failsafe: Deny all locks, if a quorum can't be reached (e.g. less than 2/3 redis-lock hosts are up and responding in time)
    • Capacity: Medium.
    • Availability: Medium.

Examples when we can't use "whatever we have at hand":

  • Reducing load on a service by using locks in the same service, is ineffective. For example, you can't reduce connections to a primary database host through a lock on that same database (whereby only the first request to get the lock would connect and do the work). If you use that same primary database to store the locks, then you still end up establishing too many primary DB connections. In this situation we tend to grab something else semi-random. For example, you might reach for memcached/BagOStuff to create a best-effort lock instead.
    • Example: ResourceLoader/DependencyStore. Before ResourceLoader writes data, it uses the "main cache" (usually memcached via BagOStuff) to acquire a lock and thus ignore and debounce any redundant attempts from other requests around the same time.
    • Downside: The "main cache" is optional in MediaWiki (default: CACHE_NONE) and so by default this wil not actually de-duplicate the work for third party setups. In practice there is generally always a reasonable place to put locks, even in a plain install. But knowing which one to pick at runtime is non-trivial.

Problem statement

Currently, if high-performance blocking locks are desired for tasks (such as cache regeneration de-duplication), the only option is PoolCounter.

In the sub-usecase of mutexes (vs semaphores), a broader set of backends could be used than those that could implement the *full* set of PoolCounter features.

For example, MySQL named locks could handle the exclusive regeneration (PC's acquireForMe case) case of PoolCounter, but not the case of multiple threads blocking on the same key (e.g. X threads can be "doing Y" at the same time).

The case of de-duplicating parsing work on edit fits into the former case (e.g. waiting on any prior edit stash request to finish for that edit) but not the later.

Proposed solution

For simplicity and flexibility, it would be nice if a new interface could be introduced for interacting with these locks. This would allow for the application to select any appropriate backend.

This would let the codebase fall back to things likes mysql named locks (when possible) but use faster, multi-DC appropriate things PoolCounter, if installed/available.

Currently, such logic would require a config variable and some if/else logic for the named lock vs PoolCounter case. A common interface for all native blocking locks would simplify this kind of scenario. Demanding PoolCounter is unnecessarily constraining and falling back to polling loops is suboptimal.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

We've talked about this briefly today in TechCom. We agreed we don't need an RFC for this, however the current commit is lacking a bit of design documentation. It would be good to lay out there (or on Phabricator first) a problem statement and evaluation of existing interfaces we have for locking in filebackend, bagostuff, poolcounter etc., their issues and what this would address, the guarantees it is meant to provide etc.

In addition to the interface itself, it would also be good to have one or two existing callers changed in a draft commit to better show how this would improve certain aspects.

Per last weeks' ArchCom meeting, we'll be tracking this as an RFC after all. My apologies for the confusion.

We'll try to keep the process lightweight. Mainly this means the problem statement and proposed approach should (also) be described in this task's description instead of only in Gerrit.

aaron triaged this task as Lowest priority.May 26 2017, 5:07 PM

This RFC has been scheduled for public discussion on IRC next Wednesday, September 20.

As always, the discussion will take place in the IRC channel #wikimedia-office on Wednesday 21:00 UTC (2pm PDT, 23:00 CEST).

I asked @aaron about an example use for this interface, in preparation for the IRC meeting tonight..

<Krinkle> what would be a good example in core or an extension that would make use of this interface (e.g. something that currently does something less nice)
<AaronSchulz> I think the edit stash locks that use always GET_LOCK atm. This is less flexible and is slower for postgres and sqlite since that just does a looping check.
<AaronSchulz> the getScopedLock calls in AuthManager/MessageCache too
<AaronSchulz> ChronologyProtector too

Existing abstract lock managers in MediaWiki core:

InterfaceImplementationsConfigurationObtainBatchingShared Write (Semaphore)Exclusive WriteShared ReadComment
PoolCounterStub, Redis, PoolCounter_Client$wgPoolCounterConfPoolCounter::factory?Obtained and configured by purpose (e.g. "ArticleView"), not by backend, and lacks default or fallback.
LockManagerDB, FileSystem, Memcached, Postgres, Redis, Scoped$wgLockManagersLockManagerGroup
IDatabase::lockMySQL (GET_LOCK), MSSQL (memory), Postgres (ADVISORY_LOCK), Oracle, SQLite (FSLockManager)(N/A)MediaWikiServices?Not for generic use.
BagOStuff::lock(WaitConditionLoop)(N/A)MediaWikiServices?Not for generic use.

I'm still very confused about the name. "InterruptMutexManager" sounds like a manager of mutexes of interrupts. If I understand correctly, "Interrupt" in the name refers to the guarantee that the lock will be released automatically when the current request terminates, no matter how it terminates. So perhaps "AutoReleaseMutexManager" would be more descriptive. But then, will we have MutexManagers that do not guarantee this? Why not just call the interface "MutexManager"?

This RFC was discussed on IRC on September 20th. Log: https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-09-20-21.01.log.html

During that meeting, it was agreed to let this RFC enter the Last Call period. Should no pertinent concerns remain unaddressed by October 5, this RFC will be approved for implementation.

ksmith subscribed.

This was approved in the 2017-10-04 TechCom private meeting.

Change 332950 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] [WIP] Add InterruptMutexManager interface

https://gerrit.wikimedia.org/r/332950

Code is written, needs code review -- the gerrit patches are in the task description. @tstarling Is someone from your team able to take that on?

The use case for this is to allow Edit Stashing to use PoolCounter, instead of database locks (which means that we could do stashing in the secondary data center). We're open to solving this in another way instead.

Change 332951 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/extensions/PoolCounter@master] [WIP] Implement InterruptMutexManager methods in PoolCounter_Client

https://gerrit.wikimedia.org/r/332951

aaron removed aaron as the assignee of this task.Jul 3 2018, 3:44 PM

Change 532869 had a related patch set uploaded (by Krinkle; owner: Aaron Schulz):
[mediawiki/core@master] lockmanager: rewrite MySqlLockManager to use GET_LOCK() and friends

https://gerrit.wikimedia.org/r/532869

Just flying by here to say that having this abstracted and standardised interface for a dc-local non-blocking mutex lock manager, would also help with reducing use of the internal getLocalClusterInstance object. Apart from one or two core use cases, use of that are mostly for wanting a cheap lock that works both by default in stock MW and at WMF. By having this service class, we'd be able to transition more use away from the internal getLocalClusterInstance and to something with a proper service interface. (The alternative to using getLocalClusterInstance today, are basically to either hardcode against DB master locks for stock MW and for WMF; or to use WMF-only poolcounter and otherwise be unprotected by default; hence getLocalClusterInstance is currently a common thing to reach for).

See also T243233.

Random observation because this poped up on my radar again: I'd like to reiterate that the word "Interrupt" in the name sends me down the wrong path of thought every time. Why not just "MutexManager" or "MutexLockManager"?

Closing due to lack of interest.

Change 332950 abandoned by Aaron Schulz:

[mediawiki/core@master] [WIP] Add InterruptMutexManager interface

Reason:

https://gerrit.wikimedia.org/r/332950

Change 332951 abandoned by Aaron Schulz:

[mediawiki/extensions/PoolCounter@master] [WIP] Implement InterruptMutexManager methods in PoolCounter_Client

Reason:

https://gerrit.wikimedia.org/r/332951

Change 532869 abandoned by Aaron Schulz:

[mediawiki/core@master] lockmanager: rewrite MySqlLockManager to use GET_LOCK() and friends

Reason:

https://gerrit.wikimedia.org/r/532869

Change 532869 restored by Aaron Schulz:

[mediawiki/core@master] lockmanager: rewrite MySqlLockManager to use GET_LOCK() and friends

https://gerrit.wikimedia.org/r/532869

Change 532869 abandoned by Aaron Schulz:

[mediawiki/core@master] lockmanager: rewrite MySqlLockManager to use GET_LOCK() and friends

Reason:

PoC; going with removal

https://gerrit.wikimedia.org/r/532869

Krinkle renamed this task from Introduce InterruptMutexManager to Standardise lock management in MediaWiki.Sep 7 2024, 9:14 PM
Krinkle reopened this task as Open.
Krinkle updated the task description. (Show Details)