The RedisConnectionPool patch idea seems reasonable to me.

In T238493#5693277, @Gilles wrote:

@aaron have you started looking into this? Any leads?

In T230813#5648838, @Krinkle wrote:

In T230813#5599191, @Mholloway wrote:

Whoever picks this up, please ping me for access to the required private repo.

I thought maybe the ticket was missing some updates that were communicated elsewhere, but from what I understand our team was unable to install the extension. The review thus-far covered the backend and was based on static review of the code. Next steps is to figure out a minimal way to install (part of) it for frontend review.

Are there any cache busting user preferences at play here?

Aside from the things mentioned in the above patch, the overall code looks OK to me.

In T231086#5601608, @fgiunchedi wrote:

swiftrepl is puppetized now to run an eqiad -> codfw sync once a week on Monday (without deletes).

I just want it on the work board (I had a meeting with Erik/Bill) for tracking object cache review and work (we have the goal of getting CPT more involved in maintenance rather than just myself and Timo).

In T229686#5454294, @CDanis wrote:

In db-eqiad/codfw.php we currently provide IP addresses as the values of the keys in externalLoads instead of using hostsByName to translate hostnames to IP, e.g.:

'externalLoads' => [
	# es2
	'cluster24' => [
		'10.64.32.184' => 0, # es1015, C2 11TB 128GB, master
		'10.64.0.6'    => 1, # es1011, A2 11TB 128GB
		'10.64.16.186' => 1, # es1013, B1 11TB 128GB
	],
]

However looking at the code path, I don't think there's any reason why this has to be so. I see in LBFactoryMulti.php that newExternalLB calls newLoadBalancer which calls makeServerArray which implements the translation: $serverInfo['host'] = $this->hostsByName[$serverName] ?? $serverName;
So I'm planning on emitting hostnames when I implement externalLoads output in dbctl, but wanted to verify with @Krinkle and @aaron that I understood the situation correctly. @Krinkle also suggested that if we are going to rely on this behavior, then it should be explicitly tested in mediawiki-core.

In T224422#5563013, @jcrespo wrote:

So this is the latest news:
On T234948 we found that special page updates can lead to a constant 1-2 seconds of lag for the confluence of large long running query with heavy load from the many s8 edits + ongoing wikibase refactoring. Because the timeout is only 1 second, this means chronology protector will fail frequently. However, because we allow pooling for I think a limit of 5 seconds, the host is not depooled. Also, some jobs probably use the "slow" replica and try to use chronology protector, which leads to lots of errors. Obviously, increasing the timeout cannot be done lightly, as it could cause worse problems.
The good news is that I believe this was deployed successfully and works as intended. The bad news is that there is a weakness on our infrastructure, but I am not sure how to move forward, need suggestions. We can open discussion on another ticket, as this scope seems solved.

Indeed the logging is based on the *whole* raw unfiltered position...I should add a logstash key for the filtered one too.

@jcrespo @Marostegui What do think of the idea of having another cluster of mysql servers set up just like the parser cache ones? That would be nice from an HA perspective and to avoid adding extra load to any existing DB cluster (e.g. objectcache table of metawiki or extension1)? Traffic would be modest given that it would start out for use for WikimediaEvents, LoginNotify, perhaps AbuseFilter stats too (see https://docs.google.com/document/d/1tX8ekiYb3xYgpNJsmA1SiKqzkWc0F-_E4SGx6BI72vA/edit#heading=h.bdt9mhl3o7k5).

Not seeing this in the logs anymore.

In T157651#5535315, @kchapman wrote:

@aaron are you requesting code review from Core Platform or do you need something else?

Seems like some kind of merge conflict.

Odd, the constant seems to be there.

In T218207#5157583, @Krinkle wrote:

Interesting. In the specific case of SQLite, "cache in database" and "cache on disk" are effectively both use the disk. Some quick comparisons using Quick MediaWiki to install MediaWiki with SQLite (macOS, on-disk /private/tmp/quickmw, PHP 7.1.26 from Homebrew).

SQLite (default installation)

LocalSettings.php (generated)

$wgLocalisationCacheConf['storeServer'] = [
	'type' => 'sqlite',
	'dbname' => "{$wgDBname}_l10n_cache",
	'tablePrefix' => '',
	'variables' => [ 'synchronous' => 'NORMAL' ],
	'dbDirectory' => $wgSQLiteDataDir,
	'trxMode' => 'IMMEDIATE',
	'flags' => 0
];

Test: Deploy one language

time php maintenance/rebuildLocalisationCache.php --lang de --force
real	0m0.404s, 0m0.416s, 0m0.407s

! StaticArray

LocalSettings.php (appendix)

$wgCacheDirectory = $wgSQLiteDataDir;
$wgLocalisationCacheConf['store'] = 'array';

Test: Deploy one language

real	0m0.140s, 0m0.156s, 0m0.148s

Looks like Static Array beats SQLite as well. We've shown in all previous benchmarks that the "All languages" and "Page load time" use cases always align with the "One language" use case, so I won't bother re-running those. Besides, I don't think this would inform our decision here, as I don't think we should optimise the stock MW default for SQLite against MySQL and other RDBMS'es.
In the unlikely event someone finds that sqlite3-based writing or reading outperforms opcache-backed arrays, it will still work by default, and can be optimised by setting wgLocalisationCacheConf directly.

So, getting this test merged depends on redoing the wikibase schema hook application order for update.php. In CI, there seems to be a problem when it interacts with Flow hooks trying to make pages.

Should be fixed now.

It looks like WebStart.php sets ignore_user_abort() for POSTS and the major entry points have wfTransactionalTimeLimit() set for POSTS. In the case of module_deps updates for load.php, that's on GET.

Client disconnects (HTTP 499) are interesting...before the ignore_user_abort() in doPostOutputShutdown(), I suppose it's possible to end up with stuff like this (and long has been). https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/519741/ would help this particular case by avoiding DB writes.

In T231086#5434295, @CDanis wrote:

I wonder if some entry point lacks proper shutdown.

What is the value of apc.enable_cli ? I don't seem to have that problem.

I do worry about the risk of data loss if swiftrepl is also deleting files based on container list differences.

I'd love to have a simplified version of WebRequest as a service. One that would be useful for dealing with the issue that https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/532367/ is about. Optimization hacks like https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/526801/ could be avoided too. It could be injected with pathinfo/cookie settings, but would not deal with complex encoding stuff that uses $wgContLang and so on.

Still, a file was only uploaded, and no other operations done...I'm not sure why the DB would commit if the file store failed in one of the FileBackendMultiwrite backends and 'replication' is 'sync'...

Isn't there a swiftrepl background process to fix this?

Note that CdnCacheUpdate queues a purge to happen X seconds later to help deal with lag (mediawiki-config has $wgCdnReboundPurgeDelay at 11). If lag gets near that amount, then $wgCdnMaxageLagged will kick in.

In T229456#5417884, @Tgr wrote:

This is significantly less useful than the old behavior. Affected_rows is typically used to either skip expensive cache purges when nothing actually changed, or signal to the user whether they actually managed to change something. (Why would a caller care about rows matched but not changed?)

Seems to be resolved, likely by vary-revision refactoring from T226785.

I don't think so.

Does this still occur?

Per my comment above, this is the expected behavior.

It's an optional table, not installed by update.php.

They were obsoleted by flaggedrevs_statistics.

The remaining vary-revision instances are basic self-transclusions (https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/526157/ should handle those).

Is https://phabricator.wikimedia.org/T212881#5195101 the error that still happens or is it the read-only one too?

Jobs are fine...though this case is complicated since people want their "latest views" to be immediately reflected...so it would have to do something like WatchedItemStore.

How much of this is unique from T205936 ?

I wonder if this is fixed in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/519565/

The logs for doSelectDomain() look quite for the last 7 days.

959daa2ca44c039e72c8a9a5199d4c74dd05caba added the << $status->value = [ 'warnings' => $upload->checkWarnings() ]; >> line. It seems like checkWarnings() has all kinds of File objects inside of it potentially. Some callback could easily slip in given that.

ObjectCache always mentioned getMainStashInstance() as "Ephemeral global storage". It was just supposed to *try harder* to be persistent than memcached (rdb snapshots, expectation that stuff can *probably* still be there a week later or so). The existence of redis evictions and consistent re-hashing on host failure making data disappear or go stale was well known at the time it was picked as the original "stash".

In T228465#5352272, @tstarling wrote:

Fixed and confirmed

[0450][www-data@labweb1001:/]$ time /usr/local/bin/mwscript extensions/TorBlock/maintenance/loadExitNodes.php --wiki=labswiki --force
Successfully loaded 1206 exit nodes.
real	0m1.958s
user	0m0.572s
sys	0m0.212s

I guess the cron jobs should be removed now, is there a bug for that?

JobQueueException should be thrown from push(), with nothing catching it other than MWExceptionHandler or site-specific callers. Things like RenameUser *depend* on knowing whether something enqueued or not in order to function correctly. Typically, push() should be used pre-send, before preOutputCommit, so everything would just rollback anyway. Jobs pushed after than are enqueued during DeferrableUpdates (directly or indirectly via lazyPush()); in that case, DeferredUpdates should (already) catch any exceptions (not just job queue ones) and rollback on an update-by-update bases. The exceptions are logged in the DeferredUpdates channel (previously the Exception channel).

Also, the timeout exceptions themselves where redis, not LBFactory. The later seemed to just have errors related to the improper shutdown.

In T225103#5348415, @Jdforrester-WMF wrote:

Is this still a train blocker?

Dropping the field doesn't make sense, but dropping the whole table does. We do not use that class in production (and it is optional within MW core).