Nikerabbit has uploaded a new change for review.
https://gerrit.wikimedia.org/r/189319
Change subject: Avoid deadlocks in Special:(MessageGroup|Language)Stats
......................................................................
Avoid deadlocks in Special:(MessageGroup|Language)Stats
Takes Iae9a18b0c30b4 a bit further. The original patch assumed that
these updates only happen via JobQueue, which is not true. Just by
vising either of Special:MessageGroupStats or Special:LanguageStats
will trigger calculation of missing stats, and hence no locking was
applied.
In this patch I batch all updates (so one query instead of hundreds
of small ones) into one query. That batch is then executed on the
onTranslationIdle, which makes the locking code work during web
requests as well.
I lowered the timelimit on the special pages from 8 seconds to 2
seconds for faster page loading as well as smaller batches.
With this patch I am no longer able to produce deadlocks by
running ab with concurrent requests on the stats page, which were
happening before this patch. I also verified that some updates are
indeed dropped when lock cannot be acquired.
This could be futher improved by scheduling updates jobs via JobQueue
or limiting the maximum size of $updates array.
Bug: T53410
Change-Id: I2162dd657ed7054f579bdce8f738a93abd5f2c67
---
M specials/SpecialLanguageStats.php
M utils/MessageGroupStats.php
2 files changed, 54 insertions(+), 44 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Translate
refs/changes/19/189319/1
diff --git a/specials/SpecialLanguageStats.php
b/specials/SpecialLanguageStats.php
index a690165..4c2e8c9 100644
--- a/specials/SpecialLanguageStats.php
+++ b/specials/SpecialLanguageStats.php
@@ -41,7 +41,7 @@
* bailing out.
* @var int
*/
- protected $timelimit = 8;
+ protected $timelimit = 2;
/**
* Flag to set if nothing to show.
diff --git a/utils/MessageGroupStats.php b/utils/MessageGroupStats.php
index 05ca3f4..93bb908 100644
--- a/utils/MessageGroupStats.php
+++ b/utils/MessageGroupStats.php
@@ -29,6 +29,8 @@
protected static $timeStart = null;
/// @var float
protected static $limit = null;
+ /// @var array
+ protected static $updates = array();
/**
* Set the maximum time statistics are calculated.
@@ -83,6 +85,8 @@
$stats[$id][$code] = self::forItemInternal( $stats,
$group, $code );
}
+ self::queueUpdates();
+
return $stats[$id][$code];
}
@@ -98,6 +102,8 @@
$flattened[$group] = $languages[$code];
}
+ self::queueUpdates();
+
return $flattened;
}
@@ -112,6 +118,8 @@
return array();
}
$stats = self::forGroupInternal( $group );
+
+ self::queueUpdates();
return $stats[$id];
}
@@ -129,6 +137,8 @@
$stats = self::forGroupInternal( $g, $stats );
}
+ self::queueUpdates();
+
return $stats;
}
@@ -142,28 +152,14 @@
$code = $handle->getCode();
$ids = $handle->getGroupIds();
$dbw = wfGetDB( DB_MASTER );
-
- $locked = false;
- // Try to avoid deadlocks with duplicated deletes where there
is no row
- // @note: this only helps in auto-commit mode (which job
runners use)
- if ( !$dbw->getFlag( DBO_TRX ) && count( $ids ) == 1 ) {
- $key = __CLASS__ . ":modify:{$ids[0]}";
- $locked = $dbw->lock( $key, __METHOD__, 1 );
- if ( !$locked ) {
- return true; // raced out
- }
- }
-
$conds = array( 'tgs_group' => $ids, 'tgs_lang' => $code );
- $dbw->delete( self::TABLE, $conds, __METHOD__ );
- wfDebugLog( 'messagegroupstats', "Cleared " . serialize( $conds
) );
- if ( $locked ) {
- $dbw->unlock( $key, __METHOD__ );
- }
+ $key = md5( serialize( $conds ) );
- // Hooks must return value
- return true;
+ self::runWithLock( $dbw, $key, __METHOD__, function ( $dbw,
$method ) use ( $conds ) {
+ $dbw->delete( self::TABLE, $conds, $method );
+ wfDebugLog( 'messagegroupstats', "Cleared " .
serialize( $conds ) );
+ } );
}
public static function clearGroup( $id ) {
@@ -362,7 +358,7 @@
return $aggregates;
}
- $data = array(
+ self::$updates[] = array(
'tgs_group' => $id,
'tgs_lang' => $code,
'tgs_total' => $aggregates[self::TOTAL],
@@ -370,29 +366,6 @@
'tgs_fuzzy' => $aggregates[self::FUZZY],
'tgs_proofread' => $aggregates[self::PROOFREAD],
);
-
- $dbw = wfGetDB( DB_MASTER );
- // Try to avoid deadlocks with S->X lock upgrades in MySQL
- // @note: this only helps in auto-commit mode (which job
runners use)
- $key = __CLASS__ . ":modify:$id";
- $locked = false;
- if ( !$dbw->getFlag( DBO_TRX ) ) {
- $locked = $dbw->lock( $key, __METHOD__, 1 );
- if ( !$locked ) {
- return $aggregates; // raced out
- }
- }
-
- $dbw->insert(
- self::TABLE,
- $data,
- __METHOD__,
- array( 'IGNORE' )
- );
-
- if ( $locked ) {
- $dbw->unlock( $key, __METHOD__ );
- }
return $aggregates;
}
@@ -468,4 +441,41 @@
return $number < 0 ? "$number" : "+$number";
}
+
+ protected static function queueUpdates() {
+ if ( !count( self::$updates ) ) {
+ return;
+ }
+
+ $dbw = wfGetDB( DB_MASTER );
+ self::runWithLock( $dbw, 'updates', __METHOD__, function (
$dbw, $method ) {
+ $dbw->insert(
+ self::TABLE,
+ self::$updates,
+ $method,
+ array( 'IGNORE' )
+ );
+
+ self::$updates = array();
+ } );
+ }
+
+ protected static function runWithLock( $dbw, $key, $method, $callback )
{
+ $dbw->onTransactionIdle( function () use ( $dbw, $key, $method,
$callback ) {
+ $key = __CLASS__ . ':' . $key;
+ $locked = false;
+ if ( !$dbw->getFlag( DBO_TRX ) ) {
+ $locked = $dbw->lock( $key, $method, 1 );
+ if ( !$locked ) {
+ return; // Raced out
+ }
+ }
+
+ call_user_func( $callback, $dbw, $method );
+
+ if ( $locked ) {
+ $dbw->unlock( $key, $method );
+ }
+ } );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/189319
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2162dd657ed7054f579bdce8f738a93abd5f2c67
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Translate
Gerrit-Branch: master
Gerrit-Owner: Nikerabbit <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
