Aude has uploaded a new change for review.
https://gerrit.wikimedia.org/r/233970
Change subject: Update Wikidata - wrap usage tracking batch updates in
transaction
......................................................................
Update Wikidata - wrap usage tracking batch updates in transaction
Change-Id: I70f59485d8f3b10264d6e210636a72ca2204b1c9
---
M composer.lock
M extensions/Wikibase/client/includes/Usage/Sql/EntityUsageTable.php
M extensions/Wikibase/client/includes/Usage/Sql/SqlUsageTracker.php
M
extensions/Wikibase/client/includes/store/sql/ConsistentReadConnectionManager.php
M
extensions/Wikibase/client/tests/phpunit/includes/store/sql/ConsistentReadConnectionManagerTest.php
M vendor/composer/installed.json
6 files changed, 52 insertions(+), 20 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikidata
refs/changes/70/233970/1
diff --git a/composer.lock b/composer.lock
index 8d17676..6314265 100644
--- a/composer.lock
+++ b/composer.lock
@@ -949,7 +949,7 @@
"support": {
"issues":
"https://phabricator.wikimedia.org/project/profile/1202/";
},
- "time": "2015-08-14 11:26:42"
+ "time": "2015-08-14 10:25:37"
},
{
"name": "wikibase/data-model",
@@ -1330,7 +1330,7 @@
"support": {
"issues":
"https://phabricator.wikimedia.org/project/profile/989/";
},
- "time": "2015-08-14 10:27:43"
+ "time": "2015-08-14 10:23:53"
},
{
"name": "wikibase/serialization-javascript",
@@ -1379,7 +1379,7 @@
"source": {
"type": "git",
"url":
"https://gerrit.wikimedia.org/r/mediawiki/extensions/Wikibase";,
- "reference": "e83f1f85171d73035306021dabdc4cfd18c29ec6"
+ "reference": "4b7691ead1d9922d8c8ec1104b2c730c773d0001"
},
"require": {
"data-values/common": "~0.3.0",
@@ -1461,7 +1461,7 @@
"issues": "https://phabricator.wikimedia.org/";,
"irc": "irc://irc.freenode.net/wikidata"
},
- "time": "2015-08-25 13:18:04"
+ "time": "2015-08-25 17:34:58"
},
{
"name": "wikibase/wikimedia-badges",
@@ -1503,7 +1503,7 @@
"support": {
"irc": "irc://irc.freenode.net/wikidata"
},
- "time": "2015-08-12 09:30:01"
+ "time": "2015-08-04 17:18:47"
}
],
"packages-dev": [],
diff --git a/extensions/Wikibase/client/includes/Usage/Sql/EntityUsageTable.php
b/extensions/Wikibase/client/includes/Usage/Sql/EntityUsageTable.php
index f68b864..89e1a1a 100644
--- a/extensions/Wikibase/client/includes/Usage/Sql/EntityUsageTable.php
+++ b/extensions/Wikibase/client/includes/Usage/Sql/EntityUsageTable.php
@@ -4,6 +4,7 @@
use ArrayIterator;
use DatabaseBase;
+use Exception;
use InvalidArgumentException;
use Iterator;
use Wikibase\Client\Usage\EntityUsage;
@@ -131,6 +132,7 @@
* @param string $touched timestamp
*/
private function touchUsageBatch( array $rowIds, $touched ) {
+ $this->connection->begin( __METHOD__ );
$this->connection->update(
$this->tableName,
array(
@@ -141,6 +143,7 @@
),
__METHOD__
);
+ $this->connection->commit( __METHOD__ );
}
/**
@@ -195,8 +198,12 @@
$c = 0;
foreach ( $batches as $rows ) {
+ $this->connection->begin( __METHOD__ );
+
$this->connection->insert( $this->tableName, $rows,
__METHOD__, array( 'IGNORE' ) );
$c += $this->connection->affectedRows();
+
+ $this->connection->commit( __METHOD__ );
}
return $c;
@@ -344,6 +351,7 @@
$batches = array_chunk( $idStrings, $this->batchSize );
foreach ( $batches as $batch ) {
+ $this->connection->begin( __METHOD__ );
$this->connection->delete(
$this->tableName,
array(
@@ -351,6 +359,7 @@
),
__METHOD__
);
+ $this->connection->commit( __METHOD__ );
}
}
diff --git a/extensions/Wikibase/client/includes/Usage/Sql/SqlUsageTracker.php
b/extensions/Wikibase/client/includes/Usage/Sql/SqlUsageTracker.php
index 34356dc..fd35634 100644
--- a/extensions/Wikibase/client/includes/Usage/Sql/SqlUsageTracker.php
+++ b/extensions/Wikibase/client/includes/Usage/Sql/SqlUsageTracker.php
@@ -37,7 +37,7 @@
/**
* @var int
*/
- private $batchSize = 1000;
+ private $batchSize = 100;
/**
* @param EntityIdParser $idParser
@@ -129,7 +129,9 @@
return;
}
- $db = $this->connectionManager->beginAtomicSection( __METHOD__
);
+ // NOTE: while logically we'd like the below to be atomic, we
don't wrap it in a
+ // transaction to prevent long lock retention during big
updates.
+ $db = $this->connectionManager->getWriteConnection();
try {
$usageTable = $this->newUsageTable( $db );
@@ -145,9 +147,9 @@
$usageTable->touchUsages( $pageId, $keep, $touched );
$usageTable->addUsages( $pageId, $added, $touched );
- $this->connectionManager->commitAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
} catch ( Exception $ex ) {
- $this->connectionManager->rollbackAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
if ( $ex instanceof DBError ) {
throw new UsageTrackerException(
$ex->getMessage(), $ex->getCode(), $ex );
@@ -168,16 +170,18 @@
* @throws UsageTrackerException
*/
public function pruneStaleUsages( $pageId, $lastUpdatedBefore ) {
- $db = $this->connectionManager->beginAtomicSection( __METHOD__
);
+ // NOTE: while logically we'd like the below to be atomic, we
don't wrap it in a
+ // transaction to prevent long lock retention during big
updates.
+ $db = $this->connectionManager->getWriteConnection();
try {
$usageTable = $this->newUsageTable( $db );
$pruned = $usageTable->pruneStaleUsages( $pageId,
$lastUpdatedBefore );
- $this->connectionManager->commitAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
return $pruned;
} catch ( Exception $ex ) {
- $this->connectionManager->rollbackAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
if ( $ex instanceof DBError ) {
throw new UsageTrackerException(
$ex->getMessage(), $ex->getCode(), $ex );
@@ -200,15 +204,17 @@
return;
}
- $db = $this->connectionManager->beginAtomicSection( __METHOD__
);
+ // NOTE: while logically we'd like the below to be atomic, we
don't wrap it in a
+ // transaction to prevent long lock retention during big
updates.
+ $db = $this->connectionManager->getWriteConnection();
try {
$usageTable = $this->newUsageTable( $db );
$usageTable->removeEntities( $entityIds );
- $this->connectionManager->commitAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
} catch ( Exception $ex ) {
- $this->connectionManager->rollbackAtomicSection( $db,
__METHOD__ );
+ $this->connectionManager->releaseConnection( $db );
if ( $ex instanceof DBError ) {
throw new UsageTrackerException(
$ex->getMessage(), $ex->getCode(), $ex );
diff --git
a/extensions/Wikibase/client/includes/store/sql/ConsistentReadConnectionManager.php
b/extensions/Wikibase/client/includes/store/sql/ConsistentReadConnectionManager.php
index 9e48c45..503a3b0 100644
---
a/extensions/Wikibase/client/includes/store/sql/ConsistentReadConnectionManager.php
+++
b/extensions/Wikibase/client/includes/store/sql/ConsistentReadConnectionManager.php
@@ -70,7 +70,8 @@
}
/**
- * Returns a database connection for reading.
+ * Returns a database connection for reading. The connection should
later be released by
+ * calling releaseConnection().
*
* @note: If forceMaster() or beginAtomicSection() were previously
called on this
* ConsistentReadConnectionManager instance, this method will return a
connection to the master database,
@@ -84,11 +85,12 @@
}
/**
- * Returns a connection to the master DB, for updating.
+ * Returns a connection to the master DB, for updating. The connection
should later be released
+ * by calling releaseConnection().
*
* @return DatabaseBase
*/
- private function getWriteConnection() {
+ public function getWriteConnection() {
return $this->loadBalancer->getConnection( DB_MASTER, array(),
$this->dbName );
}
diff --git
a/extensions/Wikibase/client/tests/phpunit/includes/store/sql/ConsistentReadConnectionManagerTest.php
b/extensions/Wikibase/client/tests/phpunit/includes/store/sql/ConsistentReadConnectionManagerTest.php
index 6190ac9..cfaeb9c 100644
---
a/extensions/Wikibase/client/tests/phpunit/includes/store/sql/ConsistentReadConnectionManagerTest.php
+++
b/extensions/Wikibase/client/tests/phpunit/includes/store/sql/ConsistentReadConnectionManagerTest.php
@@ -43,6 +43,21 @@
$this->assertSame( $connection, $actual );
}
+ public function testGetWriteConnection() {
+ $connection = $this->getConnectionMock();
+ $lb = $this->getLoadBalancerMock();
+
+ $lb->expects( $this->once() )
+ ->method( 'getConnection' )
+ ->with( DB_MASTER )
+ ->will( $this->returnValue( $connection ) );
+
+ $manager = new ConsistentReadConnectionManager( $lb );
+ $actual = $manager->getWriteConnection();
+
+ $this->assertSame( $connection, $actual );
+ }
+
public function testForceMaster() {
$connection = $this->getConnectionMock();
$lb = $this->getLoadBalancerMock();
diff --git a/vendor/composer/installed.json b/vendor/composer/installed.json
index 4f327c0..cead815 100644
--- a/vendor/composer/installed.json
+++ b/vendor/composer/installed.json
@@ -1467,7 +1467,7 @@
"source": {
"type": "git",
"url":
"https://gerrit.wikimedia.org/r/mediawiki/extensions/Wikibase";,
- "reference": "e83f1f85171d73035306021dabdc4cfd18c29ec6"
+ "reference": "4b7691ead1d9922d8c8ec1104b2c730c773d0001"
},
"require": {
"data-values/common": "~0.3.0",
@@ -1497,7 +1497,7 @@
"require-dev": {
"squizlabs/php_codesniffer": "~2.1"
},
- "time": "2015-08-10 11:08:57",
+ "time": "2015-08-25 15:36:29",
"type": "mediawiki-extension",
"installation-source": "source",
"autoload": {
--
To view, visit https://gerrit.wikimedia.org/r/233970
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I70f59485d8f3b10264d6e210636a72ca2204b1c9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikidata
Gerrit-Branch: wmf/1.26wmf20
Gerrit-Owner: Aude <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
