jenkins-bot has submitted this change and it was merged.

Change subject: Limit the number of Changes we put into each 
ChangeNotificationJob
......................................................................


Limit the number of Changes we put into each ChangeNotificationJob

… and add tests for JobQueueChangeNotificationSender.

Right now this is just the batch size of the dispatchChanges
script (275 in production), which probably makes the average
job do too much.

This is related to T107722, but certainly wont fix the problems
mentioned there.

Change-Id: I8677fe131f69ef69443383a88292cd43dd266e28
---
M repo/includes/Notifications/JobQueueChangeNotificationSender.php
M 
repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
2 files changed, 103 insertions(+), 14 deletions(-)

Approvals:
  Daniel Kinzler: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/repo/includes/Notifications/JobQueueChangeNotificationSender.php 
b/repo/includes/Notifications/JobQueueChangeNotificationSender.php
index 5c53b9f..b053e2f 100644
--- a/repo/includes/Notifications/JobQueueChangeNotificationSender.php
+++ b/repo/includes/Notifications/JobQueueChangeNotificationSender.php
@@ -2,7 +2,6 @@
 
 namespace Wikibase\Repo\Notifications;
 
-use JobQueueGroup;
 use Wikibase\Change;
 use Wikibase\ChangeNotificationJob;
 
@@ -13,6 +12,7 @@
  *
  * @license GPL-2.0+
  * @author Daniel Kinzler
+ * @author Marius Hoch
  */
 class JobQueueChangeNotificationSender implements ChangeNotificationSender {
 
@@ -27,12 +27,32 @@
        private $wikiDBNames;
 
        /**
+        * @var int
+        */
+       private $batchSize;
+
+       /**
+        * @var callable
+        */
+       private $jobQueueGroupFactory;
+
+       /**
         * @param string $repoDB
         * @param string[] $wikiDBNames An associative array mapping site IDs 
to logical database names.
+        * @param int $batchSize Number of changes to push per job.
+        * @param callable|null $jobQueueGroupFactory Function that returns a 
JobQueueGroup for a given wiki.
         */
-       public function __construct( $repoDB, array $wikiDBNames = array() ) {
+       public function __construct(
+               $repoDB,
+               array $wikiDBNames = array(),
+               $batchSize = 50,
+               $jobQueueGroupFactory = null
+       ) {
                $this->repoDB = $repoDB;
                $this->wikiDBNames = $wikiDBNames;
+               $this->batchSize = $batchSize;
+               $this->jobQueueGroupFactory =
+                       $jobQueueGroupFactory === null ? 
'JobQueueGroup::singleton' : $jobQueueGroupFactory;
        }
 
        /**
@@ -52,14 +72,20 @@
                        $wikiDB = $siteID;
                }
 
-               $job = ChangeNotificationJob::newFromChanges( $changes, 
$this->repoDB );
+               $qgroup = call_user_func( $this->jobQueueGroupFactory, $wikiDB 
);
+               $chunks = array_chunk( $changes, $this->batchSize );
 
-               // @todo: inject JobQueueGroup
-               $qgroup = JobQueueGroup::singleton( $wikiDB );
-               $qgroup->push( $job );
+               $jobs = [];
+               foreach ( $chunks as $chunk ) {
+                       $jobs[] = ChangeNotificationJob::newFromChanges( 
$chunk, $this->repoDB );
+               }
+               $qgroup->push( $jobs );
 
-               wfDebugLog( __METHOD__, "Posted notification job for site 
$siteID with "
-                       . count( $changes ) . " changes to $wikiDB." );
+               wfDebugLog(
+                       __METHOD__,
+                       "Posted " . count( $jobs ) . " notification jobs for 
site $siteID with " .
+                               count( $changes ) . " changes to $wikiDB."
+               );
        }
 
 }
diff --git 
a/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
 
b/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
index 085fd98..c4eccdd 100644
--- 
a/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
+++ 
b/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
@@ -2,6 +2,12 @@
 
 namespace Wikibase\Test;
 
+use JobQueueGroup;
+use PHPUnit_Framework_TestCase;
+use Wikibase\Change;
+use Wikibase\ChangeNotificationJob;
+use Wikibase\Repo\Notifications\JobQueueChangeNotificationSender;
+
 /**
  * @covers Wikibase\Repo\Notifications\JobQueueChangeNotificationSender
  *
@@ -9,16 +15,73 @@
  * @group WikibaseStore
  * @group WikibaseChange
  * @group WikibaseRepo
- * @group Database
  *
  * @license GPL-2.0+
- * @author Daniel Kinzler
+ * @author Marius Hoch
  */
-class JobQueueChangeNotificationSenderTest extends \MediaWikiTestCase {
+class JobQueueChangeNotificationSenderTest extends PHPUnit_Framework_TestCase {
 
-       public function testSendNotification() {
-               // @todo: inject JobQueueGroup into 
JobQueueChangeNotificationSender
-               $this->markTestIncomplete( 'Need a good way to test job queue 
interaction!' );
+       /**
+        * @return JobQueueChangeNotificationSender
+        */
+       private function getSender( $batchSize, $expectedChunks ) {
+               $jobQueueGroup = $this->getMockBuilder( JobQueueGroup::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               $jobQueueGroup->expects( $this->exactly( $expectedChunks ? 1 : 
0 ) )
+                       ->method( 'push' )
+                       ->with( $this->isType( 'array' ) )
+                       ->will( $this->returnCallback(
+                               function( array $jobs ) use ( $expectedChunks ) 
{
+                                       $this->assertCount( $expectedChunks, 
$jobs );
+                                       $this->assertContainsOnlyInstancesOf(
+                                               ChangeNotificationJob::class,
+                                               $jobs
+                                       );
+                               } )
+                       );
+
+               $jobQueueGroupFactory = function( $wikiId ) use ( 
$jobQueueGroup ) {
+                       $this->assertSame( 'database-name-0', $wikiId );
+                       return $jobQueueGroup;
+               };
+
+               return new JobQueueChangeNotificationSender(
+                       'repo-db',
+                       [ 'site-id-0' => 'database-name-0' ],
+                       $batchSize,
+                       $jobQueueGroupFactory
+               );
+       }
+
+       public function sendNotificationProvider() {
+               $change = $this->getMock( Change::class );
+
+               return [
+                       'no changes' => [
+                               100,
+                               []
+                       ],
+                       'one batch' => [
+                               100,
+                               [ $change, $change, $change ]
+                       ],
+                       'three batches' => [
+                               2,
+                               [ $change, $change, $change, $change, $change ]
+                       ]
+               ];
+       }
+
+       /**
+        * @dataProvider sendNotificationProvider
+        */
+       public function testSendNotification( $batchSize, $changes ) {
+               $expectedChunks = intval( ceil( count( $changes ) / $batchSize 
) );
+
+               $sender = $this->getSender( $batchSize, $expectedChunks );
+               $sender->sendNotification( 'site-id-0', $changes );
        }
 
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/309972
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8677fe131f69ef69443383a88292cd43dd266e28
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <h...@online.de>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to