[07:28:06] is it intentional that IConnectionProvider does not seem to have an equivalent for ILBFactory::waitForReplication()? [10:25:24] Amir1: could you give https://gerrit.wikimedia.org/r/c/mediawiki/core/+/923750 another shot? I would like to see it being stable so that I can work on some follow-ups. [10:29:24] taavi: yes, for that in maint scripts you need to call $this->waitForReplication() which internally also make other important stuff as well [10:34:51] and if you want to move code out of your maintenance class into any kind of service or helper then you’re out of luck… [10:36:26] yup as you shouldn't call LBF::waitForReplication directly anyway [10:41:26] does that take into account that CentralAuth writes into a separate database? for context I'm looking to update CentralAuthDatabaseManager which calls $this->lbFactory->waitForReplication with the 'domain' parameter set [11:23:09] taavi: Amir1 we included commitAndWaitForReplication in IConnProv which takes care of other domains as well. [11:23:40] That's the general pattern we're aiming for, to commit and wait together for any dbs that the code has interacted with [11:25:06] The Maintenance base class has commitTransaction which in spirit does the same thing, except it only commits the given DB so that's not suitable as-is for complex cases like CentralAuth. [11:25:12] Would commitAndWaitForReplication work? [11:27:28] I think for now could we just pass the domain to Maintenance::waitForReplication as in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/928820 ? [11:27:58] Lucas_WMDE: Yeah, for portability I'd say there is by design no answer. There can't be because transaction committing is final so it acts on all code that ran. That's not safe to hide into reusable service logic. Rdbms uses a ticket token to enforce this concept and avoid mistakes. Only top level code such as your maintenance script or job class can obtain such a ticket when claiming the empty transaction. [11:29:14] Our usual pattern is to move responsibility into the business logic up to the size of a single batch. The wiring is then owned by how you run it. Whether web, api, special, deferred, job, or cli. So many! [11:30:54] The way that CentralAuth recreates and proxies all db handling makes for a difficult fit. I'd say it would also be okay to leave as is. We're not deprecating those functions afaik. It's better to discourage this in new code by not advertising it in the clean ICP interface (and leave CA using the older mechanism for now). [11:31:27] To leave... until someone has time to dissolve more of that db proxy logic into adopting the new patterns directly [12:18:36] Krinkle: I never said anything about committing… [13:33:21] Lucas_WMDE: usually, the reason to call waitForReplication is after calling commit() as part of a loop that writes in batches. But yeah there's a few other use cases we have for it as well!