From 6030e9cf2c1df7929e2319602aa4d37aa641de11 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 24 Oct 2018 12:28:02 -0700 Subject: [PATCH] Create JobQueueEnqueueUpdate class to call JobQueueGroup::pushLazyJobs() This assures that MergeableUpdate tasks that lazy push job will actually have those jobs run instead of being added after the lone callback update to call JobQueueGroup::pushLazyJobs() already ran. This also makes it more obvious that push will happen, since a mergeable update is added each time lazyPush() is called and a job is buffered, rather than rely on some magic callback enqueued into DeferredUpdates at just the right point in multiple entry points. Bug: T207809 Change-Id: I13382ef4a17a9ba0fd3f9964b8c62f564e47e42d --- autoload.php | 1 + includes/MediaWiki.php | 3 - includes/deferred/JobQueueEnqueueUpdate.php | 64 +++++++++++++++++++++ includes/jobqueue/JobQueueGroup.php | 24 +------- includes/jobqueue/JobRunner.php | 2 - 5 files changed, 68 insertions(+), 26 deletions(-) create mode 100644 includes/deferred/JobQueueEnqueueUpdate.php diff --git a/autoload.php b/autoload.php index f951ce96f5..6261f1aebe 100644 --- a/autoload.php +++ b/autoload.php @@ -706,6 +706,7 @@ $wgAutoloadLocalClasses = [ 'JobQueueAggregatorRedis' => __DIR__ . '/includes/jobqueue/aggregator/JobQueueAggregatorRedis.php', 'JobQueueConnectionError' => __DIR__ . '/includes/jobqueue/JobQueue.php', 'JobQueueDB' => __DIR__ . '/includes/jobqueue/JobQueueDB.php', + 'JobQueueEnqueueUpdate' => __DIR__ . '/includes/deferred/JobQueueEnqueueUpdate.php', 'JobQueueError' => __DIR__ . '/includes/jobqueue/JobQueue.php', 'JobQueueFederated' => __DIR__ . '/includes/jobqueue/JobQueueFederated.php', 'JobQueueGroup' => __DIR__ . '/includes/jobqueue/JobQueueGroup.php', diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index bc576374d6..4b84fbd685 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -899,9 +899,6 @@ class MediaWiki { __METHOD__ ); - // Important: this must be the last deferred update added (T100085, T154425) - DeferredUpdates::addCallableUpdate( [ JobQueueGroup::class, 'pushLazyJobs' ] ); - // Do any deferred jobs; preferring to run them now if a client will not wait on them DeferredUpdates::doUpdates( $blocksHttpClient ? 'enqueue' : 'run' ); diff --git a/includes/deferred/JobQueueEnqueueUpdate.php b/includes/deferred/JobQueueEnqueueUpdate.php new file mode 100644 index 0000000000..1691da216e --- /dev/null +++ b/includes/deferred/JobQueueEnqueueUpdate.php @@ -0,0 +1,64 @@ + IJobSpecification[]) */ + private $jobsByDomain; + + /** + * @param string $domain DB domain ID + * @param IJobSpecification[] $jobs + */ + public function __construct( $domain, array $jobs ) { + $this->jobsByDomain[$domain] = $jobs; + } + + public function merge( MergeableUpdate $update ) { + /** @var JobQueueEnqueueUpdate $update */ + Assert::parameterType( __CLASS__, $update, '$update' ); + + foreach ( $update->jobsByDomain as $domain => $jobs ) { + $this->jobsByDomain[$domain] = $this->jobsByDomain[$domain] ?? []; + $this->jobsByDomain[$domain] = array_merge( $this->jobsByDomain[$domain], $jobs ); + } + } + + public function doUpdate() { + foreach ( $this->jobsByDomain as $domain => $jobs ) { + $group = JobQueueGroup::singleton( $domain ); + try { + $group->push( $jobs ); + } catch ( Exception $e ) { + // Get in as many jobs as possible and let other post-send updates happen + MWExceptionHandler::logException( $e ); + } + } + } +} diff --git a/includes/jobqueue/JobQueueGroup.php b/includes/jobqueue/JobQueueGroup.php index 820c492e85..06646a5ddc 100644 --- a/includes/jobqueue/JobQueueGroup.php +++ b/includes/jobqueue/JobQueueGroup.php @@ -43,9 +43,6 @@ class JobQueueGroup { /** @var array Map of (bucket => (queue => JobQueue, types => list of types) */ protected $coalescedQueues; - /** @var Job[] */ - protected $bufferedJobs = []; - const TYPE_DEFAULT = 1; // integer; jobs popped by default const TYPE_ANY = 2; // integer; any job @@ -203,7 +200,7 @@ class JobQueueGroup { // Throw errors now instead of on push(), when other jobs may be buffered $this->assertValidJobs( $jobs ); - $this->bufferedJobs = array_merge( $this->bufferedJobs, $jobs ); + DeferredUpdates::addUpdate( new JobQueueEnqueueUpdate( $this->wiki, $jobs ) ); } /** @@ -211,17 +208,10 @@ class JobQueueGroup { * * @return void * @since 1.26 + * @deprecated Since 1.33 Not needed anymore */ public static function pushLazyJobs() { - foreach ( self::$instances as $group ) { - try { - $group->push( $group->bufferedJobs ); - $group->bufferedJobs = []; - } catch ( Exception $e ) { - // Get in as many jobs as possible and let other post-send updates happen - MWExceptionHandler::logException( $e ); - } - } + wfDeprecated( __METHOD__, '1.33' ); } /** @@ -464,12 +454,4 @@ class JobQueueGroup { } } } - - function __destruct() { - $n = count( $this->bufferedJobs ); - if ( $n > 0 ) { - $type = implode( ', ', array_unique( array_map( 'get_class', $this->bufferedJobs ) ) ); - trigger_error( __METHOD__ . ": $n buffered job(s) of type(s) $type never inserted." ); - } - } } diff --git a/includes/jobqueue/JobRunner.php b/includes/jobqueue/JobRunner.php index 39b5b3bee4..676659f384 100644 --- a/includes/jobqueue/JobRunner.php +++ b/includes/jobqueue/JobRunner.php @@ -290,8 +290,6 @@ class JobRunner implements LoggerAwareInterface { $status = $job->run(); $error = $job->getLastError(); $this->commitMasterChanges( $lbFactory, $job, $fnameTrxOwner ); - // Important: this must be the last deferred update added (T100085, T154425) - DeferredUpdates::addCallableUpdate( [ JobQueueGroup::class, 'pushLazyJobs' ] ); // Run any deferred update tasks; doUpdates() manages transactions itself DeferredUpdates::doUpdates(); } catch ( Exception $e ) { -- 2.20.1