In T210992#4793399, @Marostegui wrote:

@Krinkle can you confirm whether those are the two reverts we have to do?
I have been looking around and I haven't found anything else to revert
Thank you!

In T203786#4805938, @elukey wrote:

Today another mediawiki alert from ~12:24 to ~12:27 UTC. @Nikerabbit, @aaron - do you think that we can narrow down specific events (beside TTL expiring that may cause this?

To clarify, the $useMutex logic in WAN cache never triggers due to minAsOf=INF, resulting in stampedes when someone invalidates the cache. Instead, this should be treated like a regular TTL expiration and have one thread at a time doing regeneration.

In T203786#4791225, @Nikerabbit wrote:

Is the value so big that every time it is SET it causes a TKO? Besides the 24h TTL, the cache value is updated when there are changes to the underlying data. In other words: when people create or remove translatable pages, aggregate message groups or other similar stuff.

@Gilles: Comcast only has cable infrastructure in terms what the ISP provides itself. For customers with cable, they can also get XFinity Mobile (https://www.tomsguide.com/us/xfinity-mobile-faq,news-25223.html) . That's basically just a bunch of Wi-Fi hotspots build off of Verizon. I don't know how many people are using that and it seems new-ish. Also, the latency figures are quite low, which makes me doubt that it is XFinity Mobile and more likely regular wireless/xfinity.

It looks sane, though I wonder why Comcast is so high in usage for mobile? Is that mostly from touchpad devices instead of smartphones?

In T196378#4550382, @jcrespo wrote:

It definitely seems like something worth doing. Having the potential for high use cache keys becoming unusable for undefined periods of time is too much of a stability concern.

In T157651#3018942, @Tgr wrote:

Indeed. The updater calls the LoadExtensionSchemaUpdates hook from the constructor (bleh) and Echo and SecurePoll use dropTable/modifyField (which are executed immediately) instead of dropExtensionTable/modifyExtensionField (which would just push an entry to the update list).

This is a very, very ugly accident waiting to happen. @aaron any preference how to prevent it? We could make getSchemaVars static, or split out the extension loading part from the constructor, or add some flag that prevents calling non-extension methods on the updater. Maybe even a unit test which calls the hook with a fake updater which fails the test if any non-extension methods are called on it.

Since CategoryMembershipChangeJob runs via the job queue, wouldn't that have little effect on save timing itself? I guess it wouldn't hurt to optimize.

In T203786#4730954, @elukey wrote:

In T203786#4729548, @aaron wrote:

Keys are set by add/cas normally, so it seems like some key that takes a long time to regenerate might have expired (there are two data points at the elevated value over more than just a few seconds) or a class of many keys expired. The other possibility is some sudden change in access patterns for keys, which seems less likely, especially the more periodic this is.

Checking keys via tcpdump is not super easy, so I tried to order the packets by size inspecting the keys with bigger size and lower TTL. I keep seeing this pattern of SETs for metawiki:translate-groups (TTL 2 hours) followed by gets during the timeframe in which timeouts happen, that given the size (~380k) could surely aggravate the bandwidth problem that we are seeing. Would it be possible to increase this TTL to say a 24h to see if anything changes? Or possibly migrate the code to something more like gadget-definition, namely setting the new key only if really needed and not every two hours by default. It would help removing a (big) variable from the problem..

To answer the point brought up by @Nikerabbit about the code not changed in ages - I suspect that in the past nutcracker might have masked problems like this one (see T208934), and that the change to mcrouter caused more errors raised due to bandwidth/latency variations. I am probably not right about the metawiki:translate-groups key as culprit, but I'd just need to see if removing a big key makes any difference in what we see in the graphs, of course if this is not going to affect users in any way.

wl_notificationtimestamp is not meant to store the time the article was watched but the last revision the user saw on the page (NULL if they saw the latest revision). This would require a new column. Ideally, if watchlist sizes were limited, this woudn't need an index, but they are not.

Keys are set by add/cas normally, so it seems like some key that takes a long time to regenerate might have expired (there are two data points at the elevated value over more than just a few seconds) or a class of many keys expired. The other possibility is some sudden change in access patterns for keys, which seems less likely, especially the more periodic this is.

Fixed in bf30fcb71427d673f7c83a067b3241040d3470b6. Rollback is used instead and uses $ignoreErrors so as not to trigger the exception in reportQueryError().

Cleaned up in 633eb437a3b808518469c6eaf4e86a436941d837

In T194299#4714630, @daniel wrote:

In T194299#4714614, @aaron wrote:

openConnection is badly named and still reuses connections. You'd probably want getConnection with CONN_TRX_AUTO

I hate this hack. This may *still* re-use connections, if anything else used CONN_TRX_AUTO. We should have CONN_NEW.

openConnection is badly named and still reuses connections. You'd probably want getConnection with CONN_TRX_AUTO

What about our use of register_postsend_function? Is there anything equivalant?

In T207223#4703802, @Alexia wrote:

The recommend fix does work for our extension. I apparently had configured Echo properly in the past so it was working properly. For AbuseFilter I had to patch it to use the same pattern since it only specifies the database and not the cluster to use.

In T207809#4701489, @Krinkle wrote:

@aaron The fix LGTM, but do we know why this started happening? I'd be nice to know what commit or task prompted it so that we can learn why it wasn't prevented by our tests and/or Jenkins.

In theory, this kind of warning can be triggered in tests and would be captured by Jenkins in a way that fail the build. That definitely worked at some point.

Closing, per " The Error Occurs if the memcache is too slow".

This will be better with a3d6c1411dad3e057b if there are many message pages that exists for extension use.

4b1db1190bb8f2a115c6a81a5ee487b7d18cd303 seems more likely.

Note that git master (19dd28798163) installs fine with postgres, which has the same DB domain patches as 1.32.

It looks like the errors come from some tool (JS?) that fires a bunch of API requests from a Special:Search tab to edit numerous pages in parallel. Each burst always for a certain user ID with a single referrer URL.

Does this really need to call commitAndWaitForReplication() when there is only one batch? Is it ever called thousands of times in a row?

In T202715#4688919, @jcrespo wrote:

COMMIT takes a few seconds

It is unlikely for a commit to take a lot, as in, the actual physical changes that happen then- at commit time there is only a metadata change + flush to disk cache, which should be very fast. Look for contention somewhere as a first option when you see commit taking a long time (e.g. large transactions blocking each other).

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?