Fixed in daf0514345f03189187606ba2323794588c79dc9 .

For web requests, the lock timeout should be 5 min:

That said, from mc1019, I see:

In T194403#4278157, @jcrespo wrote:

Please excuse my ignorance, but you are talking redis for sessions, not for the jobqueue (which is, or is close to be, deprecated, right?).

I suggest that cookie set/receive round-tripping should be tested for encoding/truncation issues with @ or # for these apps, as well as letter case changes or such. The above patch simply discards cookie headers for cpPosIndex that are botched.

There are now only ~10/min of these now. I still see no 'redis' channel errors, but I wonder if the random eviction model of redis is at play. redis 3.0 is a bit better at LRU per https://redis.io/topics/lru-cache than our 2.8.

Does this still happen after https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/440031/ ?

Seems to gotten better after https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/440031/ was deployed and likewise for logstash (+"ChronologyProtector::initPositions").

In T95799#4274424, @tstarling wrote:

I don't understand how we can implement the task as described. It's intentional that write-heavy maintenance scripts go at the speed of the slowest slave. If you only wait for a majority then you could have 50% of slaves permanently lagged, potentially by days or weeks.

In T91820#4259902, @tstarling wrote:

Special:Userlogin starts a session on a GET request so that it can implement CSRF protection on the login form. And that's a localised page name so it's not easy to filter in VCL unless we change the URL in MediaWiki to something more predictable. Not sure if there are other cases, we'd need some sort of audit. If session access is very rare in the secondary DC then could we just tunnel session access to the primary DC, instead of replicating? GET requests causing session creation would be slightly delayed, then the user would get their session cookie and be directed to the primary for subsequent requests.

In T196303#4257638, @ArielGlenn wrote:

From T196125: get* and getMulti* commands now take the Memcached::GET_EXTENDED flag to retrieve user flags and cas tokens: I guess this might be it. There is indeed no cas token returned for the get; thus, no cas token is supplied with the cas command itself; twemproxy appears to expect (require) such a token, though I am not certain of that; see https://github.com/twitter/twemproxy/blob/master/src/proto/nc_memcache.c#L474

Maybe we can get @aaron to verify this.

I suspect that some RESTBase service forwards a user's cookies (for permissions) but uses a local IP, judging from the logs. Since the CP position redis key is based on the client IP/agent hash, then it will not be found and will timeout. I don't know if the agent is passed through or not.

In T192771#4223990, @Joe wrote:

The reason of the hybrid proxy approach is that mcrouter is known to use a non-insignificant amount of memory when under write pressure, so I wanted to avoid sharing the same machines as memcached itself.

We can for sure think of having global proxies in both datacenters in the future, but we can reassess at a later time.

If SET/DELETE go to all mc* servers in the wancache-(eqiad/codfw) pools (as mediawiki_wancache is configured to do in puppet), then Option B would still work since the consistent hashing wouldn't matter. Having broadcasted operations go to all mc* servers rather than just 1-per-DC (based on hash) is not required for WANCache though. Keeping it this way wouldn't scale well if the rate of those (purge) operations increased hugely for some reason. I do like the conceptual simplicity though.

In T190082#4206075, @Dbrant wrote:

Per above patch, I've also uncovered a condition in our app where it can potentially send cookies that are expired. Since the lifetime of the cpPosIndex cookie seems to be quite short, this issue is especially applicable to this cookie, and may in fact be the root cause here...

The logging level went from INFO to WARNING. I suppose this has been happening for a very long time then.

In T193008#4177829, @daniel wrote:

LBFactory does not implement DestructibleService, though it has a destroy() method.

LBFactory used to implement DestructibleService, see https://gerrit.wikimedia.org/r/c/286314/. Was that changed in order to move LBFactory to /libs? Supporting fork was indeed a driving factor behind introducing service resets, and resetting LBFactory was the primary need.

LBFactory could again implement DestructibleService, if we moved includes/services to libs as well. I don't see anything there that depends on MediaWiki. All it would take would be to change the namespace from MediaWiki\Services to Wikimedia\Services (or perhaps more appropriately, Wikimedia\ServiceContainer).

Correct usage of ForkController (which has logic that ttmserver-export is mostly doing) works fine.

Why doesn't that script use ForkController btw?

LBFactory does not implement DestructibleService, though it has a destroy() method. This is due to it being in /libs. It relies on reference counting, where the old service container instance falls out of scope in resetGlobalInstance() with $oldInstance dying, then LBFactory following suite and triggering __destruct()=>destroy(), and so on. If something has a ServiceContainer instance (with LBFactory loaded) pre-fork and tries to use it later it will get ContainerDisabledException.

This has now been running for a while (since Apr 17) with the new packages (both debian versions, though the stretch server isn't there anymore afaik).

refreshLinks2 is not used anymore. Since it is not in $wgJobClasses anymore, they probably won't get cleared in recycleAndDeleteStaleJobs().

I assume daf0514345f03 exposed this bug.

The warnings are pointless, the patch above adds an isset() check.

This is related to T149847 in that we would *have* to stop moving file content around in Special:MovePage just to rename files.

I suspect the transactions are just empty ones with SELECT statements, which don't need to give errors here.

The message index code could do for a large amount of rework. In the meantime, I can't tell why the MessageIndexRebuildJob::newJob() instance must run immediately in isValid()...it's not like the method recheck's what it did before after the rebuild. If nothing else depends on it being immediate, then it should use a DeferredUpdate. If it has to be immediate...then CONN_TRX_AUTO can be considered (as long as it doesn't deadlock by having to transactions updating the same rows).

In T190960#4100608, @jcrespo wrote:

@aaron - if you don't like the current model, should we think an alternative, simpler one based on the heartbeat table- or is this still ok for you?