Page MenuHomePhabricator

'isWriteMode' method overrides on ApiQueryBase subclasses is ignored causing transaction profiler expectations to not be met
Closed, ResolvedPublic1 Estimated Story PointsBUG REPORT

Description

The ApiQueryCheckUser API module defines ::isWriteMode to indicate that a CheckUser log entry will be created by the query. However, this definition is never called because ApiQuery::isWriteMode is not defined like is done for ::isReadMode.

ApiQuery::isWriteMode should be defined in a similar way to avoid transaction profiler warnings when writes are specifically intended.

Steps to replicate the issue
  1. Install CheckUser
  2. Open Special:ApiSandbox and make a CheckUser API request
  3. Inspect the server logs for the API request that was performed

What happens?:
A TransactionProfiler warning is raised:

[rdbms] Expectation (writes <= 0) by ApiMain::setRequestExpectations not met (actual: 1) in trx #ef7f7dc58c:
INSERT INTO `cu_log` (cul_timestamp,cul_actor,cul_type,cul_target_id,cul_target_text,cul_target_hex,cul_range_start,cul_range_end,cul_reason_id,cul_reason_plaintext_id) VALUES '?'

#0 /var/www/html/w/includes/libs/rdbms/TransactionProfiler.php(330): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated()
#1 /var/www/html/w/includes/libs/rdbms/database/TransactionManager.php(615): Wikimedia\Rdbms\TransactionProfiler->recordQueryCompletion()
#2 /var/www/html/w/includes/libs/rdbms/database/Database.php(828): Wikimedia\Rdbms\TransactionManager->recordQueryCompletion()
#3 /var/www/html/w/includes/libs/rdbms/database/Database.php(716): Wikimedia\Rdbms\Database->attemptQuery()
#4 /var/www/html/w/includes/libs/rdbms/database/Database.php(643): Wikimedia\Rdbms\Database->executeQuery()
#5 /var/www/html/w/includes/libs/rdbms/database/Database.php(1481): Wikimedia\Rdbms\Database->query()
#6 /var/www/html/w/includes/libs/rdbms/database/DBConnRef.php(119): Wikimedia\Rdbms\Database->insert()
#7 /var/www/html/w/includes/libs/rdbms/database/DBConnRef.php(407): Wikimedia\Rdbms\DBConnRef->__call()
#8 /var/www/html/w/includes/libs/rdbms/querybuilder/InsertQueryBuilder.php(343): Wikimedia\Rdbms\DBConnRef->insert()
#9 /var/www/html/w/extensions/CheckUser/src/Services/CheckUserLogService.php(118): Wikimedia\Rdbms\InsertQueryBuilder->execute()
#10 [internal function]: MediaWiki\CheckUser\Services\CheckUserLogService::MediaWiki\CheckUser\Services\{closure}()
#11 /var/www/html/w/includes/deferred/MWCallableUpdate.php(42): call_user_func()
#12 /var/www/html/w/includes/deferred/DeferredUpdates.php(486): MediaWiki\Deferred\MWCallableUpdate->doUpdate()
#13 /var/www/html/w/includes/deferred/DeferredUpdates.php(198): MediaWiki\Deferred\DeferredUpdates::attemptUpdate()
#14 /var/www/html/w/includes/deferred/DeferredUpdates.php(285): MediaWiki\Deferred\DeferredUpdates::run()
#15 /var/www/html/w/includes/deferred/DeferredUpdatesScope.php(269): MediaWiki\Deferred\DeferredUpdates::MediaWiki\Deferred\{closure}()
#16 /var/www/html/w/includes/deferred/DeferredUpdatesScope.php(198): MediaWiki\Deferred\DeferredUpdatesScope->processStageQueue()
#17 /var/www/html/w/includes/deferred/DeferredUpdates.php(304): MediaWiki\Deferred\DeferredUpdatesScope->processUpdates()
#18 /var/www/html/w/includes/MediaWikiEntryPoint.php(305): MediaWiki\Deferred\DeferredUpdates::doUpdates()
#19 /var/www/html/w/includes/MediaWikiEntryPoint.php(188): MediaWiki\MediaWikiEntryPoint->commitMainTransaction()
#20 /var/www/html/w/includes/MediaWikiEntryPoint.php(171): MediaWiki\MediaWikiEntryPoint->doPrepareForOutput()
#21 /var/www/html/w/includes/MediaWiki.php(90): MediaWiki\MediaWikiEntryPoint->prepareForOutput()
#22 /var/www/html/w/includes/api/ApiMain.php(950): MediaWiki::preOutputCommit()
#23 /var/www/html/w/includes/api/ApiMain.php(893): ApiMain->executeActionWithErrorHandling()
#24 /var/www/html/w/includes/api/ApiEntryPoint.php(158): ApiMain->execute()
#25 /var/www/html/w/includes/MediaWikiEntryPoint.php(199): MediaWiki\Api\ApiEntryPoint->execute()
#26 /var/www/html/w/api.php(44): MediaWiki\MediaWikiEntryPoint->run()
#27 {main}

What should have happened instead?:
No transaction profiler warning should have been logged, because ApiQueryCheckUser defines ::isWriteMode to return true.

Event Timeline

Dreamy_Jazz renamed this task from ApiQuery does not define a ::isWriteMode override to isWriteMode overrides on ApiQueryBase subclasses is ignored when setting transaction profiler expectations.Apr 5 2024, 3:31 PM

Change #1017317 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@master] Define ApiQuery::isWriteMode

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

Dreamy_Jazz renamed this task from isWriteMode overrides on ApiQueryBase subclasses is ignored when setting transaction profiler expectations to isWriteMode overrides on ApiQueryBase subclasses is ignored causing transaction profiler expectations to not be met.Apr 5 2024, 3:58 PM
Dreamy_Jazz set the point value for this task to 2.
Dreamy_Jazz changed the point value for this task from 2 to 1.
Dreamy_Jazz renamed this task from isWriteMode overrides on ApiQueryBase subclasses is ignored causing transaction profiler expectations to not be met to 'isWriteMode' method overrides on ApiQueryBase subclasses is ignored causing transaction profiler expectations to not be met.Apr 5 2024, 4:00 PM
Dreamy_Jazz updated the task description. (Show Details)

Change #1017317 merged by jenkins-bot:

[mediawiki/core@master] Define ApiQuery::isWriteMode

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

dom_walden subscribed.

I have been making various requests to action=query&list=checkuser on my local wiki and have not seen this warning.