While RefreshLinksJob is de-duplicated by page-id, it is possible
for two jobs to run for the same page ID if the second one was queued
after the first one started running. In that case they the newer
one must not be skipped or ignored because it will have newer
information to record to the database, but it also has no way
to stop the old one, and we can't run them concurrently.
Instead of letting the lock exception mark the job as error,
making it implicitly retry, do this more explicitly, which avoids
logspam.
Bug: T170596
Co-Authored-By: Aaron Schulz <aschulz@wikimedia.org>
Change-Id: Id2852d73d00daf83f72cf5ff778c638083f5fc73
// Make sure all links update threads see the changes of each other.
// This handles the case when updates have to batched into several COMMITs.
$scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id );
// Make sure all links update threads see the changes of each other.
// This handles the case when updates have to batched into several COMMITs.
$scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id );
+ if ( !$scopedLock ) {
+ throw new RuntimeException( "Could not acquire lock for page ID '{$id}'." );
+ }
}
$title = $this->page->getTitle();
}
$title = $this->page->getTitle();
*/
use Wikimedia\Rdbms\IDatabase;
*/
use Wikimedia\Rdbms\IDatabase;
+use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
use Wikimedia\ScopedCallback;
use MediaWiki\MediaWikiServices;
use Wikimedia\ScopedCallback;
// Make sure all links update threads see the changes of each other.
// This handles the case when updates have to batched into several COMMITs.
$scopedLock = self::acquirePageLock( $this->getDB(), $this->mId );
// Make sure all links update threads see the changes of each other.
// This handles the case when updates have to batched into several COMMITs.
$scopedLock = self::acquirePageLock( $this->getDB(), $this->mId );
+ if ( !$scopedLock ) {
+ throw new RuntimeException( "Could not acquire lock for page ID '{$this->mId}'." );
+ }
}
// Avoid PHP 7.1 warning from passing $this by reference
}
// Avoid PHP 7.1 warning from passing $this by reference
* @param IDatabase $dbw
* @param int $pageId
* @param string $why One of (job, atomicity)
* @param IDatabase $dbw
* @param int $pageId
* @param string $why One of (job, atomicity)
- * @return ScopedCallback
- * @throws RuntimeException
+ * @return ScopedCallback|null
* @since 1.27
*/
public static function acquirePageLock( IDatabase $dbw, $pageId, $why = 'atomicity' ) {
$key = "LinksUpdate:$why:pageid:$pageId";
$scopedLock = $dbw->getScopedLockAndFlush( $key, __METHOD__, 15 );
if ( !$scopedLock ) {
* @since 1.27
*/
public static function acquirePageLock( IDatabase $dbw, $pageId, $why = 'atomicity' ) {
$key = "LinksUpdate:$why:pageid:$pageId";
$scopedLock = $dbw->getScopedLockAndFlush( $key, __METHOD__, 15 );
if ( !$scopedLock ) {
- throw new RuntimeException( "Could not acquire lock '$key'." );
+ $logger = LoggerFactory::getInstance( 'SecondaryDataUpdate' );
+ $logger->info( "Could not acquire lock '{key}' for page ID '{page_id}'.", [
+ 'key' => $key,
+ 'page_id' => $pageId,
+ ] );
+ return null;
// Serialize links updates by page ID so they see each others' changes
$scopedLock = LinksUpdate::acquirePageLock( wfGetDB( DB_MASTER ), $pageId, 'job' );
// Serialize links updates by page ID so they see each others' changes
$scopedLock = LinksUpdate::acquirePageLock( wfGetDB( DB_MASTER ), $pageId, 'job' );
+ if ( $scopedLock === null ) {
+ $this->setLastError( 'LinksUpdate already running for this page, try again later.' );
+ return false;
+ }
if ( WikiPage::newFromID( $pageId, WikiPage::READ_LATEST ) ) {
// The page was restored somehow or something went wrong
if ( WikiPage::newFromID( $pageId, WikiPage::READ_LATEST ) ) {
// The page was restored somehow or something went wrong
$dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
/** @noinspection PhpUnusedLocalVariableInspection */
$scopedLock = LinksUpdate::acquirePageLock( $dbw, $page->getId(), 'job' );
$dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
/** @noinspection PhpUnusedLocalVariableInspection */
$scopedLock = LinksUpdate::acquirePageLock( $dbw, $page->getId(), 'job' );
+ if ( $scopedLock === null ) {
+ // Another job is already updating the page, likely for an older revision (T170596).
+ $this->setLastError( 'LinksUpdate already running for this page, try again later.' );
+ return false;
+ }
// Get the latest ID *after* acquirePageLock() flushed the transaction.
// This is used to detect edits/moves after loadPageData() but before the scope lock.
// The works around the chicken/egg problem of determining the scope lock key.
// Get the latest ID *after* acquirePageLock() flushed the transaction.
// This is used to detect edits/moves after loadPageData() but before the scope lock.
// The works around the chicken/egg problem of determining the scope lock key.