In T212550#5124323, @Smalyshev wrote:

@aaron Is there any docs how to use the client ID on the client side? I see there's support for cpPosIndex cookie which is $index@$time#$clientId but if I have only client ID that is not going to help me. So I am not sure how would one use it - I can't find any trace of ChronologyClientId HTTP header.

In T212550#5124280, @Krinkle wrote:

Another question for @aaron as well - If we start storing this chronologyprotector field in kafka etc. that means it can survive longer and no longer has an expiry like we do for cookies. I vaguely recall an issue in the past where clients that kept the value too long caused subsequent requests to stall for many seconds as MW mistook the unknown value for a value that hasn't appeared yet.

It may've been mitigated already, but if not, we'll need to sort that out before this goes live.

If it's not actually part of a job, it should be DB_REPLICA. Also, even when it should use DB_MASTER it should not be using getCachedGroupDefinitions() since it should skip the cache.

I think you can just add a method, similar to LBFactory::getChronologyProtectorTouched, that exposes the client ID, maybe call it LBFactory::getChronologyProtectorClientId();

Can someone please prioritize this? This basically means that very few people are using edit stashing on enwiki, which has this editor as the default. That seems like a pretty bad regression from the regular editor being the default.

Given all the PHP memory/speed enhancements, some benchmarking is probably in order. Maybe BenchmarkParse could use a preprocess-only option. I'm curious how PHP7 compares to HHVM for large pages.

So, I don't really think loading the parsed HTML into a DOM object like this is really acceptable performance-wise, and I don't see any trick to make it fast.

I made a version of the page that extracts less djvu pages (but still many) and tried to profile again:

In T211488#5103435, @Joe wrote:

• Session
  
  We don't use the default session storage in PHP. But, we do use some of its functions and legacy variables like $_SESSION. I don't know whether some of those might vary their behaviour through these.

I'll be honest, I had no idea either, at least for some of these settings. But from a read of the docs, I don't see how the security ones might have an impact.

What does have an impact is

session.upload_progress.enabled = 1

which will indeed interact with $_SESSION by adding a variable named

session.upload_progress.name = PHP_SESSION_UPLOAD_PROGRESS

in the session to track file uploads progress.

Do we want this to be disabled? I'm not sure, maybe it's potentially useful?

I can't reproduce this on cf8f1fe9b0b5647d1ed3955f6ea6bce7d76fed44.

Have you compared LCStore with sqlite (as defined by the current installer) vs cdb?

In T210725#5065110, @Marostegui wrote:

I would like to elaborate more on my idea on how to deploy the change to eqiad for the first key (the one we already changed in codfw):

Someday at 05:00AM UTC

• Merge: https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/499737/
• Run scap pull on the canaries: ` `
• Create a manual lock on deploy1001 by doing: touch /var/lock/scap.operations_mediawiki-config.lock
• Check the parsercache graphs, errors, browse the sites using mwdebug1001 and mwdebug1002, keep checking for errors.
• After 3-4 hours, if all goes good and nothing weird is seeing on graphs and logs, remove the lock.
• Do a normal scap sync-file to deploy the key everywhere.

In T137900#5087502, @geraki wrote:

What about {{REVISIONTIMESTAMP}}?

Some cases still tale scan, but those are pages or API modules that aggregate everything, so it's not avoidable there.

LCStore_DB has gone through numerous changes since this report. For example, with mysql/postgres a separate auto-commit mode connection is used to avoid staleness and contention. The sqlite installer make a dedicated LCStore_DB database file to avoid contention and failed writes.

Table not used anymore.

Does this still occur?

FYI, I was playing around with my PHP interpreter last night using:

They don't seem related nor are actually thrown exceptions (just warnings caught by MWExceptionHandler and logged using Exception objects).

In T218473#5045018, @AndyRussG wrote:

T217469 had a patch deployed... Is the issue on this task fixed, also, maybe?

In T218087#5055353, @Mholloway wrote:

@aaron How are we doing on this? Do you expect for this to be complete in time to enable in production on Thursday of this week? Seems like you've already gone over all of the codebase and made tweaks, but I'm not sure if there's more you had planned.

You can even just omit the dbname from $wgDBservers. That is also a cleaner config for wiki farms. The servers (masters + X replicas) themselves don't really have a concept of "the table prefix", or even "the database" if it's mysql (rather than sqlite/postgres). Each wiki uses some db/prefix (might be the same DB or per-wiki or grouped or whatever). Calls to LoadBalancer::getConnection() or wfGetDB( DB_REPLICA, [], $wikiDbDomain ) can just use the db/prefix contained in $wikiDBDomain, which is $wgDBname and $wgDBprefix if it's the current wiki.

Code should always handle the current wiki (e.g. wfWikiId/$wgDBname) case and not require it to be set in $wgLocalDatabases. The job queue should be fine now (at least in master, 32, and 31). If anything somewhere still has that problem, then it's a bug.

In T218511#5040211, @Jeff_G wrote:

In T218511#5034922, @Catrope wrote:

This may have been addressed by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/496563/

Thanks, when might that change go live?

From T193832:

In T193832#5047227, @aaron wrote:

I don't see a way to avoid having to set $wgDBname. It was already used to determine things like wfWikiId(), which is used by some things to get DB connections. Having them both set and mismatched has long since been a use case that is not well supported. The correspondence needs better documentation though.

Maybe I could detect this in MWLBFactory and error out with an informative message. Mysql does not require specifying a DB in order to get a connection (unlike postgres) and 'dbname' is only used for when the $domain to wfGetDB() is '' (e.g. unspecified). That means that it's pretty much pointless to have it in $wgDBservers now, so if it exists and does not match $wgDBname, an error could be thrown. The 'schema' is always NULL for mysql. I suppose 'tablePrefix' could be checked against $wgDBprefix too.

I don't see a way to avoid having to set $wgDBname. It was already used to determine things like wfWikiId(), which is used by some things to get DB connections. Having them both set and mismatched has long since been a use case that is not well supported. The correspondence needs better documentation though.

Why does the job itself have all of the transformed text rather than just a revision/page ID and use them to derive the transformed text? I get that some metadata is not stored elsewhere and would have to go in the job.

In T218087#5042864, @jcrespo wrote:

66 millon rows is a small table, the size itself does not worry me, how writes when updating the table works does, specially with wikidata, where normally multiple writes of the same item happens at the same time.

The wetede_rand index seems like it will need to be changed as I mentioned, given the size. Otherwise, there are two many cases that could involve massing scanning.

The $wiki !== wfWikiID() check was meant for the local wiki case (as wgLocalDatabases is either fully set for some farms and often empty for smaller sites).

@Tgr might know better.

Maybe the User::saveSettings() call has its clearSharedCache() call deferred in the old code, due to hasOrMadeRecentMasterChanges(), but is immediate in the new code. If the tests relied on the old cached value being there, then they could fail.

Actually, given the nature of tests all using DB_MASTER, I don't yet see what would be significantly affected here. It can't be hasMasterChanges() since that wouldn't have changed, doneWrites() is not used, lastDoneWrites() is only used by lastMasterChangeTimestamp(). That in turn is used by hasOrMadeRecentMasterChanges() and waitForReplication().

I'd imagine --use-normal tables would pass and not having that would fail. In production, there is no reason to treat temporary table operations as writes (e.g. for triggering commitMasterChanges as so on). They would just be used for complex reads and accumulated results. For tests, without --use-normal-tables, everything is temporary (though it it's meant to represent permanent stuff). Even still, this rarely matters, though things like User do check hasOrMadeRecentMasterChanges(), which would hit the edge case of everything being temporary (no longer returning true).

Seems to rarely occur (usually either 0 times per 30 minutes or 1 time per 30 minutes).

Seems like it might be T217649 (which should be re-titled itself).

In T188801#5027011, @TheDJ wrote:

Report on WP:VP/T:

Today is Thursday. And now when I visit a link from my watchlist, and then go back to the watchlist, the green bullet stays green instead of turning blue. This problem occurs even for deleted redlinked pages.

Might be related to this change ?

In T215107#4963154, @Tgr wrote:

So that's an impressive cascade of failures:

• Updates for the image table are not batched so the query times out. (Why doesn't the DB error have a stack trace? I thoght we are logging that.)

Yes, and CacheAwarePropertyInfoStore should use delete() or such for purges rather than set(). Using set() would only effect one DC.

Can the (extra) space be dedicated more so towards the larger slabs, were we have more problems AFAIK?

How will wikimedia_editor_tasks_entity_description_exists be populated and about how large will it be? Can wetede_language actually go up to 255 length? If the table can be huge, then I also wonder if the wetede_rand index should be on (wetede_language,wetede_description_exists,wetede_rand) instead.

In T138619#5019310, @Krinkle wrote:

Is this the same as T204026?

It's also useful as a tunable option for spreading I/O accross cache nodes rather than only being used to get around hard limits.

Which SQL layer? We have SQLBagOStuff using pcxxxx servers, which I suppose could be co-opted for this. As for ES, I wouldn't want to spam a bunch of one-off blobs in there since it is meant to be append-only. Depending on how much code it is I don't mind a bit of complexity. I'm still experimenting around with different ways to do it, but I don't think it has to be that complex.

Does that still happen? I can't see how that would happen in master without files from two versions.

Two pieces, one of them still huge. I'm working on a generic segmentation wrapper for BagOStuff atm, but that will take longer to do (lots of ways to arrange the classes, hard to choice from).

The SET metric for redis is very slow, so wouldn't use 10x that figure.

Job queue has long since been migrated to changeprop

It reduces set()/setInterimKey() calls to volatile (recently invalidated) keys. Each of those calls translates into a GET/CAS since they use merge(). I don't think it will prevent the huge GET spikes we saw (since most did not correspond to CAS and were probably not merge() calls).

Here is a form of the query without filesorts: