Timing appears to line up with the train deployment to meta yesterday, which had a couple of CentralNotice related changes https://www.mediawiki.org/wiki/MediaWiki_1.44/wmf.23#CentralNotice

@Reedy looks like something did go wrong with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralNotice/+/1129227

I’ll have a look when I’m back in half an hour or so.

The problems from relying on vague behaviour, I’m sure

@Pcoombe Can you check if it's broken on testwiki too please? I'm presuming it will be, because configuration wise it's testwiki => testwiki in the same way that your attempts are metawiki => metawiki (so no edge cases of cross wiki/db connections).

Similar for your own test/dev wiki if possible.

The error message is... less than useful, but it's a bit hard to see how that change (especially in Banner::save()) would cause this specifically. The functional change is very explicitly just changing to using a explicit primary DB call.

The DB getting doesn't really change anything either. Some different abstraction, but in theory the same otherwise.

Annoyingly, we're apparently losing the exception (because the rollback one is thrown first, so we don't re-throw whatever has already failed...) from Banner::save(), which may explain why this is actually failing in the first place.

		} catch ( Exception $ex ) {
			$db->rollback( __METHOD__ );
			throw $ex;
		}

The whole function is suspicious...

	public function save( User $user, $summary = null ) {
		$db = CNDatabase::getPrimaryDb();
		$action = 'modified';

		try {
			// Don't move this to saveBannerInternal--can't be in a transaction
			// TODO: explain why not. Is text in another database?
			$this->saveBodyContent( $summary, $user );

			// Open a transaction so that everything is consistent
			$db->startAtomic( __METHOD__ );

			if ( !$this->exists() ) {
				$action = 'created';
				$this->initializeDbForNewBanner( $db );
			}
			$this->saveBannerInternal( $db );
			$this->logBannerChange( $action, $user, $summary );

			$db->endAtomic( __METHOD__ );

			// Clear the dirty flags
			foreach ( $this->dirtyFlags as $flag => &$value ) {
				$value = false;
			}

			if ( $this->runTranslateJob ) {
				// Must be run after banner has finished saving due to some dependencies that
				// exist in the render job.
				// TODO: This will go away if we start tracking messages in database :)
				MessageGroups::singleton()->recache();
				MediaWikiServices::getInstance()->getJobQueueGroup()->push(
					RebuildMessageIndexJob::newJob()
				);
				$this->runTranslateJob = false;
			}

		} catch ( Exception $ex ) {
			$db->rollback( __METHOD__ );
			throw $ex;
		}

		return $this;
	}

We do something before starting the transactions... And if that failed (for example), we could be trying to rollback... even though that transaction (atomic) isn't open/valid/whatever.

Should that be wrapped in the same transaction? As it does various inserts etc... It seems weird to only rollback some of it...

Of course it's been like that for a long time...

Maybe the moving to virtual domains is actually just bringing over issues to light.

Catching db errors is usually highly discouraged: https://www.mediawiki.org/wiki/Database_transactions#Use_of_transaction_rollback

In T390956#10708258, @Ladsgroup wrote:

Catching db errors is usually highly discouraged: https://www.mediawiki.org/wiki/Database_transactions#Use_of_transaction_rollback

Yeah, it's unclear what is actually trying to be caught.. It's been there since rECNO5dd5896b3a18: OOPifying CentralNotice Banner Class

In T390956#10708258, @Ladsgroup wrote:

Catching db errors is usually highly discouraged: https://www.mediawiki.org/wiki/Database_transactions#Use_of_transaction_rollback

We do the rollback ourselves. It's not needed.

Unless you use non-standard transaction modes which I'm not seeing using those here.

I mean the comments in themselves...

			// Don't move this to saveBannerInternal--can't be in a transaction
			// TODO: explain why not. Is text in another database?
			$this->saveBodyContent( $summary, $user );

As we go down that tree...

protectBannerContent can throw a BannerContentException

addTag can throw a LogicException

@Reedy I don't have Centralnotice admin rights on testwiki. Tried on https://meta.wikimedia.beta.wmflabs.org/ and got the following

[Z-6ddbcmMgFKU3Hf1VExEQAAAEA] /wiki/Special:CentralNoticeBanners Wikimedia\Rdbms\DBUnexpectedError: Banner::save: Expected mass rollback of all peer transactions (DBO_TRX set)

Backtrace:

from /srv/mediawiki/php-master/includes/libs/rdbms/database/Database.php(2391)
#0 /srv/mediawiki/php-master/includes/libs/rdbms/database/DBConnRef.php(127): Wikimedia\Rdbms\Database->rollback(string)
#1 /srv/mediawiki/php-master/includes/libs/rdbms/database/DBConnRef.php(673): Wikimedia\Rdbms\DBConnRef->__call(string, array)
#2 /srv/mediawiki/php-master/extensions/CentralNotice/includes/Banner.php(1096): Wikimedia\Rdbms\DBConnRef->rollback(string)
#3 /srv/mediawiki/php-master/extensions/CentralNotice/includes/Banner.php(1525): Banner->save(MediaWiki\User\User, string)
#4 /srv/mediawiki/php-master/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(339): Banner::addBanner(string, string, MediaWiki\User\User, bool, bool, array, array, array, string)
#5 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(823): SpecialCentralNoticeBanners->processBannerList(array, CentralNoticeHtmlForm)
#6 /srv/mediawiki/php-master/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(146): MediaWiki\HTMLForm\HTMLForm->trySubmit()
#7 /srv/mediawiki/php-master/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(92): SpecialCentralNoticeBanners->showBannerList()
#8 /srv/mediawiki/php-master/includes/specialpage/SpecialPage.php(729): SpecialCentralNoticeBanners->execute(null)
#9 /srv/mediawiki/php-master/includes/specialpage/SpecialPageFactory.php(1737): MediaWiki\SpecialPage\SpecialPage->run(null)
#10 /srv/mediawiki/php-master/includes/actions/ActionEntryPoint.php(499): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, MediaWiki\Context\RequestContext)
#11 /srv/mediawiki/php-master/includes/actions/ActionEntryPoint.php(143): MediaWiki\Actions\ActionEntryPoint->performRequest()
#12 /srv/mediawiki/php-master/includes/MediaWikiEntryPoint.php(202): MediaWiki\Actions\ActionEntryPoint->execute()
#13 /srv/mediawiki/php-master/index.php(58): MediaWiki\MediaWikiEntryPoint->run()
#14 /srv/mediawiki/w/index.php(3): require(string)
#15 {main}

let me just remove the exception and rollback part and see what happens afterwards.

Change #1133936 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/extensions/CentralNotice@wmf_deploy] Remove catching of db exception

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

In my dev wiki, with or without your patch... I see

[e050be778f06119faff7ed92] /wiki/Special:CentralNoticeBanners BannerDataException: Device name 'desktop' not known! Cannot add.

Backtrace:

from /var/www/wiki/mediawiki/extensions/CentralNotice/includes/Banner.php(493)
#0 /var/www/wiki/mediawiki/extensions/CentralNotice/includes/Banner.php(1518): Banner->setDevices()
#1 /var/www/wiki/mediawiki/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(331): Banner::addBanner()
#2 /var/www/wiki/mediawiki/core/includes/htmlform/HTMLForm.php(823): SpecialCentralNoticeBanners->processBannerList()
#3 /var/www/wiki/mediawiki/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(146): MediaWiki\HTMLForm\HTMLForm->trySubmit()
#4 /var/www/wiki/mediawiki/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(92): SpecialCentralNoticeBanners->showBannerList()
#5 /var/www/wiki/mediawiki/core/includes/specialpage/SpecialPage.php(729): SpecialCentralNoticeBanners->execute()
#6 /var/www/wiki/mediawiki/core/includes/specialpage/SpecialPageFactory.php(1737): MediaWiki\SpecialPage\SpecialPage->run()
#7 /var/www/wiki/mediawiki/core/includes/actions/ActionEntryPoint.php(499): MediaWiki\SpecialPage\SpecialPageFactory->executePath()
#8 /var/www/wiki/mediawiki/core/includes/actions/ActionEntryPoint.php(143): MediaWiki\Actions\ActionEntryPoint->performRequest()
#9 /var/www/wiki/mediawiki/core/includes/MediaWikiEntryPoint.php(202): MediaWiki\Actions\ActionEntryPoint->execute()
#10 /var/www/wiki/mediawiki/core/index.php(58): MediaWiki\MediaWikiEntryPoint->run()
#11 {main}

Reverted rECNOc0953d878a53: CNDatabasePatcher: Drop migration code from REL1_24 and +2'd that to fix that (and it needs further changes, but...).

I don't actually see an error locally either, even without Amir's patch... So it's feeling something prod-specific...

Change #1133936 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf_deploy] Remove catching of db exception

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

Change #1133944 had a related patch set uploaded (by Reedy; author: Amir Sarabadani):

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Remove catching of db exception

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

Change #1133945 had a related patch set uploaded (by Reedy; author: Amir Sarabadani):

[mediawiki/extensions/CentralNotice@master] Remove catching of db exception

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

Change #1133944 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Remove catching of db exception

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T15:22:38Z] <reedy@deploy1003> Started scap sync-world: Backport for [[gerrit:1133944|Remove catching of db exception (T390956)]]

Change #1133945 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Remove catching of db exception

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T15:30:33Z] <reedy@deploy1003> reedy: Backport for [[gerrit:1133944|Remove catching of db exception (T390956)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

[92d18923-4810-9437-91b5-149d5113e54c] 2025-04-03 15:31:47: Fatal exception of type "RangeException"

Error

• mwversion: 1.44.0-wmf.23
• reqId: 92d18923-4810-9437-91b5-149d5113e54c
• Find reqId in Logstash

[{reqId}] {exception_url}   RangeException: Banner doesn't exist!

Impact

Notes

So this is replag-y.

The "we've done a write, so start reading from the primary" is such an anti-pattern... And of course, is going to cause problems when we try and undo it.

Mentioned in SAL (#wikimedia-operations) [2025-04-03T15:40:05Z] <reedy@deploy1003> Finished scap sync-world: Backport for [[gerrit:1133944|Remove catching of db exception (T390956)]] (duration: 17m 28s)

The "we've done a write, so start reading from the primary" is such an anti-pattern... And of course, is going to cause problems when we try and undo it.

Yes, we should get rid of them even if it means we learn the hard-way where it should be reading from primary too.

In T390956#10708953, @Ladsgroup wrote:

The "we've done a write, so start reading from the primary" is such an anti-pattern... And of course, is going to cause problems when we try and undo it.

Yes, we should get rid of them even if it means we learn the hard-way where it should be reading from primary too.

Well, we did. This is why we're having the problem :)

Change #1133954 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@master] Banner: Explicitly check for banner existence from primary

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

I wouldn't be surprised if most of the getReplicaDb in Banner should be getPrimaryDb... But we'll wait for that.

Change #1133954 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Banner: Conditionally check for banner existence from primary db

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

Change #1133958 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: Conditionally check for banner existence from primary db

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

Change #1133959 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: Conditionally check for banner existence from primary db

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

Change #1133959 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: Conditionally check for banner existence from primary db

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

Change #1133958 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: Conditionally check for banner existence from primary db

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:14:25Z] <reedy@deploy1003> Started scap sync-world: Backport for [[gerrit:1133959|Banner: Conditionally check for banner existence from primary db (T390956)]]

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:21:40Z] <reedy@deploy1003> reedy: Backport for [[gerrit:1133959|Banner: Conditionally check for banner existence from primary db (T390956)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

🐢

Error

• mwversion: 1.44.0-wmf.23
• reqId: 1d10f30a-9f0c-9988-ac2b-089f64d25d48
• Find reqId in Logstash

[{reqId}] {exception_url}   BannerExistenceException: No banner exists where tmp_name = T390956. Could not load.

Impact

Notes

Change #1133965 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@master] Banner: While saving, do exists() against primary

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:29:39Z] <reedy@deploy1003> Finished scap sync-world: Backport for [[gerrit:1133959|Banner: Conditionally check for banner existence from primary db (T390956)]] (duration: 15m 13s)

Change #1133965 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Banner: While saving, do exists() against primary

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

Change #1133968 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: While saving, do exists() against primary

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

Change #1133969 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: While saving, do exists() against primary

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

Change #1133969 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: While saving, do exists() against primary

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:37:11Z] <reedy@deploy1003> Started scap sync-world: Backport for [[gerrit:1133969|Banner: While saving, do exists() against primary (T390956)]]

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:44:25Z] <reedy@deploy1003> reedy: Backport for [[gerrit:1133969|Banner: While saving, do exists() against primary (T390956)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Error

• mwversion: 1.44.0-wmf.23
• reqId: d222944e-c66f-9b40-8727-aafb488e5d8e
• Find reqId in Logstash

[{reqId}] {exception_url}   BannerExistenceException: No banner exists where tmp_name = T390956. Could not load.

Impact

Notes

Change #1133970 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@master] Banner: More reading from primary...

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

Change #1133968 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: While saving, do exists() against primary

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T16:58:44Z] <reedy@deploy1003> Finished scap sync-world: Backport for [[gerrit:1133969|Banner: While saving, do exists() against primary (T390956)]] (duration: 21m 33s)

Change #1133970 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Banner: More reading from primary...

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

Change #1133983 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: More reading from primary...

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

Change #1133984 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: More reading from primary...

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

Change #1133984 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf/1.44.0-wmf.23] Banner: More reading from primary...

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T17:39:46Z] <reedy@deploy1003> Started scap sync-world: Backport for [[gerrit:1133984|Banner: More reading from primary... (T390956)]], [[gerrit:1133156|CommonSettings-labs: Update BounceHandler config]]

Change #1133983 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf_deploy] Banner: More reading from primary...

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

Mentioned in SAL (#wikimedia-operations) [2025-04-03T17:47:48Z] <reedy@deploy1003> reedy: Backport for [[gerrit:1133984|Banner: More reading from primary... (T390956)]], [[gerrit:1133156|CommonSettings-labs: Update BounceHandler config]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-04-03T17:57:30Z] <reedy@deploy1003> Finished scap sync-world: Backport for [[gerrit:1133984|Banner: More reading from primary... (T390956)]], [[gerrit:1133156|CommonSettings-labs: Update BounceHandler config]] (duration: 17m 43s)

Thanks so much @Reedy