Page MenuHomePhabricator

DBPerformance: Unexpected masterConns on GET from ApiQueryInfo via BlockManager.php
Closed, ResolvedPublic

Description

  • Request URL: /w/api.php?action=query&format=json&formatversion=2&prop=revisions%7Cinfo&rvprop=content%7Ctimestamp&titles=…&intestactions=edit&intestactionsdetail=full&rvsection=1
Expectation (masterConns <= 0) by ApiMain::setRequestExpectations not met (actual: 1):
trace
#0 /srv/mediawiki/php-1.35.0-wmf.5/includes/libs/rdbms/TransactionProfiler.php(189): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated('masterConns', '[connect to 10....', 1)
#1 /srv/mediawiki/php-1.35.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(943): Wikimedia\Rdbms\TransactionProfiler->recordConnection('10.64.16.104', 'frwiktionary', true)
#2 /srv/mediawiki/php-1.35.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(899): Wikimedia\Rdbms\LoadBalancer->getServerConnection(0, 'frwiktionary', 4)
#3 /srv/mediawiki/php-1.35.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1044): Wikimedia\Rdbms\LoadBalancer->getConnection(-2, Array, 'frwiktionary', 4)
#4 /srv/mediawiki/php-1.35.0-wmf.5/includes/GlobalFunctions.php(2580): Wikimedia\Rdbms\LoadBalancer->getMaintenanceConnectionRef(-2, Array, 'frwiktionary')
#5 /srv/mediawiki/php-1.35.0-wmf.5/includes/block/DatabaseBlock.php(257): wfGetDB(-2)
#6 /srv/mediawiki/php-1.35.0-wmf.5/includes/block/DatabaseBlock.php(1190): MediaWiki\Block\DatabaseBlock::newLoad(Object(User), 2, true, '2001:18c0:822:c...')
#7 /srv/mediawiki/php-1.35.0-wmf.5/includes/block/BlockManager.php(132): MediaWiki\Block\DatabaseBlock::newListFromTarget(Object(User), '2001:18c0:822:c...', true)
#8 /srv/mediawiki/php-1.35.0-wmf.5/includes/user/User.php(1802): MediaWiki\Block\BlockManager->getUserBlock(Object(User), Object(WebRequest), false)
#9 /srv/mediawiki/php-1.35.0-wmf.5/includes/user/User.php(2120): User->getBlockedStatus(false)
#10 /srv/mediawiki/php-1.35.0-wmf.5/includes/Permissions/PermissionManager.php(664): User->getBlock(false)
#11 /srv/mediawiki/php-1.35.0-wmf.5/includes/Permissions/PermissionManager.php(402): MediaWiki\Permissions\PermissionManager->checkUserBlock('edit', Object(User), Array, 'secure', false, Object(Title))
#12 /srv/mediawiki/php-1.35.0-wmf.5/includes/Permissions/PermissionManager.php(282): MediaWiki\Permissions\PermissionManager->getPermissionErrorsInternal('edit', Object(User), Object(Title), 'secure')
#13 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiQueryInfo.php(560): MediaWiki\Permissions\PermissionManager->getPermissionErrors('edit', Object(User), Object(Title), 'secure')
#14 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiQueryInfo.php(386): ApiQueryInfo->extractPageInfo(1209041, Object(Title))
#15 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiQuery.php(255): ApiQueryInfo->execute()
#16 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiMain.php(1603): ApiQuery->execute()
#17 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiMain.php(539): ApiMain->executeAction()
#18 /srv/mediawiki/php-1.35.0-wmf.5/includes/api/ApiMain.php(510): ApiMain->executeActionWithErrorHandling()
#19 /srv/mediawiki/php-1.35.0-wmf.5/api.php(83): ApiMain->execute()
#20 /srv/mediawiki/w/api.php(3): require('/srv/mediawiki/...')

Details

Related Gerrit Patches:

Event Timeline

Krinkle created this task.Nov 29 2019, 4:57 AM
Restricted Application added subscribers: MGChecker, Aklapper. · View Herald TranscriptNov 29 2019, 4:57 AM
Krinkle renamed this task from DBPerformance: Unexpected masterConns on GET from BlockManager.php to DBPerformance: Unexpected masterConns on GET from ApiQueryInfo via BlockManager.php.Nov 29 2019, 4:57 AM
Umherirrender added a subscriber: Umherirrender.

DatabaseBlock can use both connections

		$db = wfGetDB( $fromMaster ? DB_MASTER : DB_REPLICA );

PermissionManager decide to use master, because it was requested to use 'secure' as detail level from ApiQueryInfo

		$useReplica = ( $rigor !== self::RIGOR_SECURE );
		$block = $user->getBlock( $useReplica );

	 * @param string $rigor One of PermissionManager::RIGOR_ constants
	 *   - RIGOR_QUICK  : does cheap permission checks from replica DBs (usable for GUI creation)
	 *   - RIGOR_FULL   : does cheap and expensive checks possibly from a replica DB
	 *   - RIGOR_SECURE : does cheap and expensive checks, using the master as needed

Maybe api should only use RIGOR_FULL and RIGOR_SECURE is limited when doing actions.

The api detail level is also called full, so that looks like a easy and safe adjustment

			$rigor = $detailLevel === 'quick' ? 'quick' : 'secure';

			'testactionsdetail' => [
				ApiBase::PARAM_TYPE => [ 'boolean', 'full', 'quick' ],
				ApiBase::PARAM_DFLT => 'boolean',
				ApiBase::PARAM_HELP_MSG_PER_VALUE => [],
			],
Restricted Application added a project: Core Platform Team. · View Herald TranscriptNov 29 2019, 7:37 PM
Anomie added a subscriber: Anomie.

But would making intestactions=full be subject to replica lag harm the use cases it's meant to support?

rMW20d18cf3cb11: API: Allow prop=info intestactions to return reasons didn't expose both "full" and "secure" because explaining the difference in an API context seemed pointless

But would making intestactions=full be subject to replica lag harm the use cases it's meant to support?

Do you have an example?

I guess a use-case could be checking permissions before performing an action, but that is unreliable anyways because the permissions could change between those two calls even if you are reading from master. It would be better, I think, if clients would attempt to preform the action and catch the error.

But would making intestactions=full be subject to replica lag harm the use cases it's meant to support?

Do you have an example?
I guess a use-case could be checking permissions before performing an action, but that is unreliable anyways because the permissions could change between those two calls even if you are reading from master. It would be better, I think, if clients would attempt to preform the action and catch the error.

Well, that’s the typical time-of-check-to-time-of-use problem. But there is some value in doing the permission checks upfront – if you do it early, you can show the user “you can’t edit” before they even start editing, rather than having to throw away their work at save time after they already spent time on it. (That’s what we intend to do in Wikidata-Bridge – see T235154 for story and T237522 for investigation.)

For such cases, RIGOR_FULL would be enough, though. And I think that’s true in general – the potential replication lag affecting a RIGOR_FULL result isn’t really worse than the potential lag between the API request that checks permissions and the request that does the action, and RIGOR_SECURE only protects against the former.

Change 555603 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Avoid master connections for prop=info and intestactionsdetail=full

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

Change 555603 merged by jenkins-bot:
[mediawiki/core@master] Avoid master connections for prop=info and intestactionsdetail=full

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

Umherirrender closed this task as Resolved.Wed, Jan 8, 4:01 PM
Umherirrender claimed this task.
Umherirrender triaged this task as Medium priority.