In the getMasterDatabase() method posted above, I noticed that the database domain (e.g. DB/schema/prefix) is missing from getConnection(). Instead that should be:

In T205369#4670193, @daniel wrote:

For reference: I see three extension methods that access the ParserOutput via WikiPage::prepareContentFor edit:

• TemplateDataHooks::onPageContentSave
• SimpleCaptcha::shouldCheck
• SpamBlacklistHooks::filterMergedContent
  
  All of these should be hitting a cached instance, but perhaps they are not for some reason. The caching logic in WikiPage is not nice. Perhaps it would be better to have an in-process cache in the RevisionRenderer service. That would be straight forward, but would not cache PST content for pre-PST content.

Fixed in master.

In T193271#4663790, @Mainframe98 wrote:

I'm posting here instead of at a separate task, because I can't decipher whether this is a regression, side effect, bug or anything else. As a result of a3d6c1411dad, a lot interface messages (as in, messages actually used in the interface) are no longer cached. This results in a whopping amount of 172 database queries for interface messages on Special:Version on MediaWiki-Vagrant using MW master a3d6c1411dad or newer. Compared to the 10 there were before, this is a 1620% increase. As every query ends up in the debug log, both the Query overview and debug log tab of the debug toolbar have become rather difficult to use.

I still see 100-200 per 3 hour interval.

In T205369#4660371, @aaron wrote:

Looking at https://performance.wikimedia.org/xhgui/run/view?id=5bbfdc7c3f3dfaea44b5847c after a null edit on https://en.wikipedia.org/wiki/1857_in_Sweden I see MediaWiki\Revision\RenderedRevision::getSlotParserOutputUncached being hit 4 times even though normal pages have only 1 slot...

Looking at https://performance.wikimedia.org/xhgui/run/view?id=5bbfdc7c3f3dfaea44b5847c after a null edit on https://en.wikipedia.org/wiki/1857_in_Sweden I see MediaWiki\Revision\RenderedRevision::getSlotParserOutputUncached being hit 4 times even though normal pages have only 1 slot...

In T205369#4621225, @Peter wrote:

The alerts fired but we didn't act on them and then 7 days later it got back to the new "normal":

I think we can change or add another alert where we alert on a hard limit instead (as we done on some other dashboards). In the coming Grafana 5.3.0 the alerts will have reminders.

In T206580#4655824, @kaldari wrote:

@aaron - Any thoughts about the course of action suggested above in T206580#4655234?

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/465300/ also mentions the ID in the message.

Does this occur in master? I more so wonder if https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/452878/ happens to help.

In T203786#4629141, @elukey wrote:=

The throttling mechanism that memcache offers is good of course, but maybe -R 20 is not optimal?

Are these jobs that try to also move user subpages?

CAS errors on user might also help pinpoint some causes.

Yes and yes. I think if COMMIT takes a few seconds, then even with this UPDATE near the transaction end, multiple writes can still pile up if enough tabs are opened or other things locking user rows are going on.

If you go that route, then something like having the getFileSha1() and the sha1 field being null for certain containers plus having doOperations and friends pass a flag to getFileStat/Sha1 to have the current behavior of lazy-loading and not using null, it might work.

Is it that much space? If you add an option, you have to have getFileStat return some dummy value for the SHA1 and also not have that mess up the logic in doOperations(), which is why it seemed easier to just include the header.

In T204346#4631071, @Anomie wrote:

Why does Database::close() ever try to commit anyway?

I don't see many of these in the logs for the last 7 days. This is likely caused by editing in parallel (multiple rollback tabs at once).

It looks like there is no way to say "Level 2 (reviewed for quality) is not allowed as a tag on pages outside namespace 0". Right, now, I suppose it is just convention that reviewers only mark template revisions as at level 1 (basic review). If $wgFlaggedRevsTags included a 'namespaces' field with (NS => level) as the value (defaulting to all of $wgFlaggedRevsNamespaces at the highest level, the status quo), then this could be configured.

It's used for originals. I don't think it matters much for thumbnails, but it's hard to cleanly tell that to SwiftFileBackend. It seems like it might be easiest to have thumbor hash the local file and save the metadata in the PUT request to avoid these errors (and slowness of triggering a GET to POST the missing data).

I think any callers should use '', not '*', which doesn't make much sense to me. That said, we already started the pattern, so it may as well work here too.

Closing per comments on patch about MCR refactor.

Sometimes callers might want I/O (swift/elastics/blazegraph) near DB I/O transactions, so even if we use setTransactionListener() (like Maintenance) and listen for points where no trx is active anywhere (kind of like DeferredUpdates), we'd want to be careful about waiting for lag too long or erroring out. Then again, mixed source-IO code should generally follow guidelines (https://www.mediawiki.org/wiki/Database_transactions#Updating_secondary_non-RDBMS_stores) and use patterns like doing the key/value writes first and committing or using commit hooks/deferred updates. So...maybe a callback could listen to setTransactionListener(), it could be given the affected row count, and a deferred MergeableUpdate could be added to DeferredUpdates when the count is high for among DBs recently (using pass-by-ref listener callback vars for last-time and running-count or something). The update could wait for replication, and would do so after any related I/O updates that relate to the DB writes.

In T196378#4254123, @jcrespo wrote:

The main blocker right now is to decide on a tunneling technology, as most seem to have issues.

perf-roots seems appropriate. If anything extra is needed, that can always be discussed in the future (probably by adding to perf-roots).

All calls to incEditCountImmediate currently move it to the end of the transaction. According to logstash << +channel:DBPerformance +"user_editcount=user_editcount+N" +"sub-optimal" >> it seems to usually be very fast. Though I see occasional entries a little over 1 second. I suppose in that case, a fast enough edit rate by a single use could make a pile-up. I wonder if the delay comes from COMMIT itself?

https://en.wikipedia.org/wiki/User:Sam_Sailor/CSD_log seems to be an offending page (many links, possible parallel updates).

Confirmed.

Aside from using a narrower exception type and catching it, it's probably even easier to make acquirePageLock() return a boolean and log the error to a channel (possibly INFO level). The page_id should be extra logstash metadata, to make grouping easier. I suspect certain pages (like Commonist gallery subpages or such) are more likely to be offenders that others.

I can't seem to reproduce this slowness (using mwdebug1002).

I don't recall. It's been long enough that it's worth testing how queries run without it.

Where are the jenkins jobs defined?

Can this task be closed?

Something like that approach seems worth trying.

Is it possible to just update pagetriage_page_tags on page saves (and other relavent POST requests) when there are already master connections? For anything that depends on things updated via the job queue (like backlinks), those would have to be attached such LinksUpdates (which already run in POST/jobs). Why do things have to be updated on page views?

I noticed that regular memcached counts ADD as it does SET (cmd_set). This for both STORED and NOT_STORED cases. There is no cmd_add. However, mcrouter does seem to expose a cmd_add counter. Perhaps there can be a mcrouter dashboard similar to the Memcache on in Grafana?

Are there any tasks here that remain and are blockers to multi-DC?

Regression from fb51330084b4bde1880c76589e55e7cd87ed0c6d I assume

In T200471#4454643, @Ciencia_Al_Poder wrote:

This may be caused by rMW14ee3f210782 self-merged by @aaron

From https://logstash.wikimedia.org/goto/0b9191830a12ab3d15bce062cdb36a93, this seems to be better. But we should wait longer.

From a glance, it looks like xtradb cluster is build on Galera (which is something itself to consider in the future). Use of GET_LOCK is tricky there since it would have to use wsrep or have such queries directed to dedicated master (perhaps with some HA in front that doesn't split brain).

Ah, right, I read that ternary backwards, <<$maxTime < PHP_INT_MAX ? PHP_INT_MAX : 1>>.

In T200420#4453134, @Addshore wrote:

Something to note, because the locks are no longer in the DB, we end up selecting the same 15 or so wikis that are locked all of the time.
It could be that the other wikis actually don't have locks:

before using the redis lock manager the status of the lock from the db was also in the select so that locked dbs would not be selected at all.

Normally, it would be odd to let jobs pile up but not execute them, though the multi-DC use case of $wgReadOnly in one of the DCs wasn't considered in T130795. Ideally, jobs enqueued on GET/HEAD wouldn't be a thing...but that's not going away anytime soon.

My first inclination is to try to reduce the refreshCounts() calls.

This was fixed by the 61a7e1acd0af4a5386df03335733accfde179fa1 backport.

Fixed with the 61a7e1acd0af4a5386df03335733accfde179fa1 backport.

In T194403#4415063, @gerritbot wrote:

Change 445110 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] rdbms: fix value of ChronologyProtector::POSITION_COOKIE_TTL

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

Given how low server_failure_limit is, it might help to lower server_retry_timeout from 30s to something < 5s. Consistent hash ejections seem like the most obvious thing that could cause an acknowledged write to be seen as not being there for any of the next 5 seconds.