[10:18:58] It would be great if someone could take a look at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1306796 and the parent https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1306796 [10:19:23] Thanks you! [13:37:18] Amir1: insert here [13:37:40] it's almost over! :D [14:52:49] I removed (proposed and someone else applied) removing the "purge" link from the "article not found" message. It was only there for 21 years xD [14:53:16] https://en.wikipedia.org/wiki/Template_talk:No_article_text#Remove_%22If_a_page_was_recently_created_here%22_purge_link [14:53:50] Originally due to concern of database log and/or lost purges anno 2005. [15:34:48] Amir1: I was halfway through https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1306793/2 and got stuck in a DBO_TRX rabbit hole...things looked ok in CLI mode but sketchy in web mode... LinksDeletionUpdate looks fine. LinksUpdate could use a call to flush the transaction rounds after the lock is acquired (and so page_link_touched sees the changes the lock waited on). I think having that explicit is more understandable and complete [15:34:48] anyway. [16:01:01] AaronSchulz: sorry If I don't understand you correctly, but in our production it'll be based on redis so it'd be separate. Or you mean they could use the flush independent of the lock being acquired? [16:29:58] I mean that the callers should clear any view snapshot via commitPrimaryChanges() after/before calling acquirePageLock() so they can see what conflicting link updates did to the page/link tables. I guess before is slightly better (for row lock/vacuum reasons). Currently, getScopedLockAndFlush() flushes the most relevant DB, but it isn't the best place though, since there could be more than one DB and it also lets transactions from [16:29:58] explicit transaction round get flushed by a non-owner method and it does mix concerns a bit. Maybe getScopedLockAndFlush should just become getScopedLock and not try to flush. I guess it would need a big comment about using one of commitPrimaryChanges()/commit()/flushSnapshot(). [16:34:57] In RefreshLinks maintenance script and job, there's a empty ticket and commit round on each one afaik. Deferred updates I guess we have just the one at the start of it, between pre/post send. Or do we do a commit/flush between each dbwriting update as well? [16:38:09] RefreshLinksJob uses JOB_NO_EXPLICIT_TRX_ROUND but has a misleading comment "Tell JobRunner to not automatically wrap run() in a transaction round". There is still an implicit transaction round, it's just not an explict transaction round. [16:40:18] * AaronSchulz remembers working on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1100886 [16:42:22] ugh, so many things about RefreshLinksJob should change. It already has page IDs in the 'pages' field, runForTitle() should use that to lock first, then call loadPageData, so that the isAlreadyRefreshed() check makes more sense. [16:51:16] A lot of that is a pre-existing issue, so I'm tempted to say RefreshLinksJob is no worse with that patch. [17:14:45] Krinkle: deferredupdates each have an explicit trx round unless they use TRX_ROUND_ABSENT, in which case there is an implicit trx round. In wanted to clean that up in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1100886 to make TRX_ROUND_ABSENT actually have no trx round but never finished that patch. [17:16:44] RefreshSecondaryDataUpdate uses TRX_ROUND_ABSENT and runs through the LinksUpdate and DataUpdates for a page.