Page MenuHomePhabricator
Create Task
Maniphest T291212

ChangeMentor sometimes takes over a second per change
Closed, ResolvedPublic
Actions

Description

When working on T291194, I noticed that ChangeMentor sometimes takes over a second per mentor.

2021-09-16 18:36:17 [dc3e712e-4631-41b0-b7e1-af159d7e4c5d] mw1392 arwiki 1.37.0-wmf.21 api INFO: API POST Martin_Urbanec_(WMF) [redacted] T=172ms action=growthsetmentor [redacted]
2021-09-16 18:36:27 [e0325fa8-ee09-481b-a3fa-8cf5d54001c2] mw1443 arwiki 1.37.0-wmf.21 api INFO: API POST Martin_Urbanec_(WMF) [redacted] T=1100ms action=growthsetmentor [redacted]
2021-09-16 18:36:29 [74966b8c-ce1e-4f34-b2a4-868830efe134] mw1410 arwiki 1.37.0-wmf.21 api INFO: API POST Martin_Urbanec_(WMF) [redacted] T=1136ms action=growthsetmentor [redacted]

https://performance.wikimedia.org/xhgui/run/view?id=614386f2f09ba49ec8ee85e8 says MultiHttpClient::runMultiCurl takes most time in those requests. By live testing on production, I confirmed sending an Echo notification from as a deferred update speeds the API a lot for the client (while taking similar time once requests finishes).

Since notifying the mentee does not need to happen during the request, I decided to put it in a deferred update.

Details

SubjectRepoBranchLines +/-
ChangeMentor: Execute notify as a deferred updatemediawiki/extensions/GrowthExperimentsmaster+16 -13
Customize query in gerrit

Related Objects

Event Timeline

Urbanecm_WMF created this task.Sep 16 2021, 6:50 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptSep 16 2021, 6:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
gerritbot added a comment.Sep 16 2021, 6:50 PM

Change 721597 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] ChangeMentor: Execute notify as a deferred update

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

gerritbot added a project: Patch-For-Review.Sep 16 2021, 6:50 PM
Urbanecm_WMF claimed this task.Sep 16 2021, 6:51 PM
Urbanecm_WMF triaged this task as Medium priority.
Urbanecm_WMF edited projects, added Growth-Team (Sprint 0 (Growth Team)), User-Urbanecm_WMF (Engineering), Performance Issue; removed Growth-Team.
Urbanecm_WMF moved this task from Incoming to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.
Urbanecm_WMF moved this task from Backlog to In review on the User-Urbanecm_WMF (Engineering) board.Sep 16 2021, 6:58 PM
Urbanecm_WMF updated the task description. (Show Details)Sep 17 2021, 11:00 AM
Urbanecm_WMF added a comment.Sep 17 2021, 11:05 AM

Further investigation shows push notifications (enabled on all Wikipedias) indeed to make use of http requests (which, in WMF setup, likely calls curl):

urbanecm@notebook  ~/unsynced/gerrit/mediawiki/extensions/Echo
$ git grep Http
ServiceWiring.php:              $httpRequestFactory = $services->getHttpRequestFactory();
includes/ForeignWikiRequest.php:                $http = MediaWikiServices::getInstance()->getHttpRequestFactory()->createMultiClient();
includes/Push/NotificationServiceClient.php:use MediaWiki\Http\HttpRequestFactory;
includes/Push/NotificationServiceClient.php:use MWHttpRequest;
includes/Push/NotificationServiceClient.php:    /** @var HttpRequestFactory */
includes/Push/NotificationServiceClient.php:     * @param HttpRequestFactory $httpRequestFactory
includes/Push/NotificationServiceClient.php:    public function __construct( HttpRequestFactory $httpRequestFactory, string $endpointBase ) {
includes/Push/NotificationServiceClient.php:     * Construct a MWHttpRequest object based on the subscription and payload.
includes/Push/NotificationServiceClient.php:     * @return MWHttpRequest
includes/Push/NotificationServiceClient.php:    private function constructRequest( string $provider, array $payload ): MWHttpRequest {
tests/phpunit/integration/Push/NotificationServiceClientTest.php:       use MockHttpTrait;
tests/phpunit/integration/Push/NotificationServiceClientTest.php:               $this->installMockHttp( 'hi' );
tests/phpunit/integration/Push/NotificationServiceClientTest.php:               $this->assertInstanceOf( MWHttpRequest::class, $request );
tests/phpunit/revision_txt/138274875.txt:::::Irgendwo kam ein <code>forceHttps</code>-Cookie her, jetzt ist es weg. Es fallen immer noch viele Dinge beim reinen Überfliegen auf:
tests/phpunit/revision_txt/138275105.txt:::::Irgendwo kam ein <code>forceHttps</code>-Cookie her, jetzt ist es weg. Es fallen immer noch viele Dinge beim reinen Überfliegen auf:
urbanecm@notebook  ~/unsynced/gerrit/mediawiki/extensions/Echo
$

Considering putting ChangeMentor::notify() outside of request context speeded things up significantly, since push notifications were recently enabled by default with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/713708 and given push notifications indeed do use of http requests, I think push requests are the culprit here. Filling a new task to investigate performance improvements there as well – over a second to create a notification is a lot.

Urbanecm_WMF mentioned this in T291258: Push notifications take significant amount of time .Sep 17 2021, 11:14 AM

For my own curiosity, I ran the following query in Hadoop to get more insight into timing:

SELECT
    MAX(backend_time_ms) AS max_backend_time,
    PERCENTILE(backend_time_ms, 0.5) AS median_backend_time
FROM event.mediawiki_api_request
WHERE
    year=2021
    AND month=9
    AND day=16
    AND `database`='arwiki'
    AND params['action'] = 'growthsetmentor'
    AND performer.user_text = 'Martin Urbanec (WMF)'

Result:

max_backend_timemedian_backend_time
12996438.0
gerritbot added a comment.Sep 20 2021, 8:50 AM

Change 721597 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] ChangeMentor: Execute notify as a deferred update

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.1; 2021-09-21).Sep 20 2021, 9:00 AM
Maintenance_bot removed a project: Patch-For-Review.Sep 20 2021, 9:10 AM
Urbanecm_WMF closed this task as Resolved.Sep 23 2021, 6: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