Page MenuHomePhabricator
Create Task
Maniphest T239451

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

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 Changes in Gerrit:
SubjectRepoBranchLines +/-
Avoid master connections for prop=info and intestactionsdetail=fullmediawiki/coremaster+5 -1
Customize query in gerrit

Related Objects

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 project: MediaWiki-Action-API.Nov 29 2019, 7:37 PM
Umherirrender subscribed.

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: Platform Engineering. · View Herald TranscriptNov 29 2019, 7:37 PM
Izno moved this task from Backlog to User blocking on the MediaWiki-User-management board.Dec 1 2019, 6:11 PM
Krinkle added a project: Wikimedia-production-error.Dec 1 2019, 7:49 PM
Anomie moved this task from Unsorted to Needs details or plan on the MediaWiki-Action-API board.Dec 2 2019, 1:48 PM
Anomie subscribed.

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

Lucas_Werkmeister_WMDE subscribed.Dec 2 2019, 3:51 PM
Anomie moved this task from Inbox to Triage Meeting Inbox on the Platform Engineering board.Dec 2 2019, 3:58 PM
dbarratt subscribed.Dec 2 2019, 4:01 PM
In T239451#5705017, @Anomie wrote:

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.

Lucas_Werkmeister_WMDE added a comment.Dec 2 2019, 4:09 PM
In T239451#5705598, @dbarratt wrote:
In T239451#5705017, @Anomie wrote:

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.

Krinkle moved this task from Untriaged to Nov2019/1.35.wmf.5+ on the Wikimedia-production-error board.Dec 2 2019, 8:39 PM
Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.Dec 4 2019, 9:29 PM
gerritbot added a comment.Dec 6 2019, 8:29 PM

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

gerritbot added a project: Patch-For-Review.Dec 6 2019, 8:29 PM
gerritbot added a comment.Jan 7 2020, 8:56 PM

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

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.15; 2020-01-14).Jan 7 2020, 9:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jan 7 2020, 9:10 PM
Niharika moved this task from Untriaged to Tracking work by others on the Anti-Harassment-Team board.Jan 8 2020, 12:35 AM
Umherirrender closed this task as Resolved.Jan 8 2020, 4:01 PM
Umherirrender claimed this task.
Umherirrender triaged this task as Medium priority.
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:41 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits