Page MenuHomePhabricator

Transaction warning: WikiPage::doEdit (User::loadFromDatabase) (TranslateMetadata::get)
Closed, ResolvedPublic

Description

3 PHP Notice: WikiPage::doEdit: Transaction already in progress (from DatabaseBase::query (User::loadFromDatabase)), performing implicit commit! [Called from DatabaseBase::begin in /www/translatewiki.net/w/includes/db/Database.php at line 2888] in /www/translatewiki.net/w/includes/debug/Debug.php on line 282

6 PHP Notice:  WikiPage::doEdit: Transaction already in progress (from DatabaseBase::query (TranslateMetadata::get)),  performing implicit commit! [Called from DatabaseBase::begin in /www/translatewiki.net/w/includes/db/Database.php at line 2888] in /www/translatewiki.net/w/includes/debug/Debug.php on line 282

Version: 1.20.x
Severity: normal

Details

Reference
bz40378

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 1:02 AM
bzimport set Reference to bz40378.
1 PHP Notice:  WikiPage::doEdit: Transaction already in progress (from DatabaseBase::query (LinkCache::addLinkObj)),  performing implicit commit! [Called from DatabaseBase::begin in /www/translatewiki.net/w/includes/db/Database.php at line 2888] in /www/translatewiki.net/w/includes/debug/Debug.php on line 282

Seeing that WikiPage::doEdit is called while transactions from TranslateMetadata::get, User::loadFromDatabase, and LinkCache::addLinkObj are still in progress seems odd, and quite broken. Needs fixing...

Niklas, can you please investigat the case of TranslateMetadata::get?

I suspect that issues like https://bugzilla.wikimedia.org/show_bug.cgi?id=37225 are caused by transaction being broken by this kind of incorrect transaction behavior. Supporting nested transactions in mediawiki would probably elevate the problem, but care must still be taken to match every begin with a commit or rollback.

I don't know what to investigate. The code for TranslateMetadata::get is very simple, I don't see how it would start or end transaction:

public static function get( $group, $key ) {

		if ( self::$cache === null ) {
			$dbr = wfGetDB( DB_SLAVE );
			self::$cache = $dbr->select( 'translate_metadata', '*', array(), __METHOD__ );
		}

		foreach ( self::$cache as $row ) {
			if ( $row->tmd_group === $group && $row->tmd_key === $key ) {
				return $row->tmd_value;
			}
		}

		return false;

}

Don't forget implicit transactions (DBO_TRX). They cause this error more with single DB setups, since getting a DB with DB_SLAVE still gives the master. This is why I made https://gerrit.wikimedia.org/r/#/c/24655/ yesterday.

(In reply to comment #4)

Don't forget implicit transactions (DBO_TRX). They cause this error more with
single DB setups, since getting a DB with DB_SLAVE still gives the master. This
is why I made https://gerrit.wikimedia.org/r/#/c/24655/ yesterday.

Thanks Aaron for pointing to DBO_TRX, I didn't even know that existed. Does that mean that wfGetDB() may implicitly start a transaction? That would be quite horrible - because that would kill any transaction that is already open!

The current transaction level and possible support (or lack of support) for nested transactions must be considered in any such automatism. I suppose that needs more investigation.

(Still - note that the new warnings do not cause the actual problem with that behavior, they just expose them).

As the warnings were killed, I guess not. Once the warning are enabled again, probably still (?).

These notices have been fixed by now (in the trx warning saga).