[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Avoid INSERT..SELECT in LocalFileDeleteBatch
jenkins-bot has submitted this change and it was merged.
Change subject: Avoid INSERT..SELECT in LocalFileDeleteBatch
..
Avoid INSERT..SELECT in LocalFileDeleteBatch
That construct has poor locking characteristics in terms of
auto-inc columns as well as not allowing such inserts concurrently
for statement-based replication. Also, the INSERT..SELECT did not
have an ORDER BY, which could lead to fa_id drift with statement
based replication.
Change-Id: Iaacb75d9931b4cd24b70bdcaadd0e3979c7e9c90
---
M includes/filerepo/file/LocalFile.php
1 file changed, 51 insertions(+), 38 deletions(-)
Approvals:
Krinkle: Looks good to me, but someone else must approve
BryanDavis: Looks good to me, but someone else must approve
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/filerepo/file/LocalFile.php
b/includes/filerepo/file/LocalFile.php
index 7e6e651..40141c9 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -2216,8 +2216,9 @@
}
protected function doDBInserts() {
+ $now = time();
$dbw = $this->file->repo->getMasterDB();
- $encTimestamp = $dbw->addQuotes( $dbw->timestamp() );
+ $encTimestamp = $dbw->addQuotes( $dbw->timestamp( $now ) );
$encUserId = $dbw->addQuotes( $this->user->getId() );
$encReason = $dbw->addQuotes( $this->reason );
$encGroup = $dbw->addQuotes( 'deleted' );
@@ -2239,15 +2240,15 @@
}
if ( $deleteCurrent ) {
- $concat = $dbw->buildConcat( [ "img_sha1", $encExt ] );
- $where = [ 'img_name' => $this->file->getName() ];
- $dbw->insertSelect( 'filearchive', 'image',
+ $dbw->insertSelect(
+ 'filearchive',
+ 'image',
[
'fa_storage_group' => $encGroup,
'fa_storage_key' => $dbw->conditional(
[ 'img_sha1' => '' ],
$dbw->addQuotes( '' ),
- $concat
+ $dbw->buildConcat( [
"img_sha1", $encExt ] )
),
'fa_deleted_user' => $encUserId,
'fa_deleted_timestamp' => $encTimestamp,
@@ -2268,44 +2269,56 @@
'fa_user' => 'img_user',
'fa_user_text' => 'img_user_text',
'fa_timestamp' => 'img_timestamp',
- 'fa_sha1' => 'img_sha1',
- ], $where, __METHOD__ );
+ 'fa_sha1' => 'img_sha1'
+ ],
+ [ 'img_name' => $this->file->getName() ],
+ __METHOD__
+ );
}
if ( count( $oldRels ) ) {
- $concat = $dbw->buildConcat( [ "oi_sha1", $encExt ] );
- $where = [
- 'oi_name' => $this->file->getName(),
- 'oi_archive_name' => array_keys( $oldRels ) ];
- $dbw->insertSelect( 'filearchive', 'oldimage',
+ $res = $dbw->select(
+ 'oldimage',
+ OldLocalFile::selectFields(),
[
- 'fa_storage_group' => $encGroup,
- 'fa_storage_key' => $dbw->conditional(
- [ 'oi_sha1' => '' ],
- $dbw->addQuotes( '' ),
- $concat
- ),
- 'fa_deleted_user' => $encUserId,
- 'fa_deleted_timestamp' => $encTimestamp,
- 'fa_deleted_reason' => $encReason,
- 'fa_deleted' => $this->suppress ?
$bitfield : 'oi_deleted',
+ 'oi_name' => $this->file->getName(),
+ 'oi_archive_name' => array_keys(
$oldRels )
+ ],
+ __METHOD__,
+ [ 'FOR UPDATE' ]
+ );
+ $rowsInsert = [];
+ foreach ( $res as $row ) {
+
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Avoid INSERT..SELECT in LocalFileDeleteBatch
Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/306249
Change subject: Avoid INSERT..SELECT in LocalFileDeleteBatch
..
Avoid INSERT..SELECT in LocalFileDeleteBatch
That construct has poor locking characteristics in terms of
auto-inc columns as well as not allowing such inserts concurrently
for statement-based replication. Also, the INSERT..SELECT did not
have an ORDER BY, which could lead to fa_id drift with statement
based replication.
Change-Id: Iaacb75d9931b4cd24b70bdcaadd0e3979c7e9c90
---
M includes/filerepo/file/LocalFile.php
1 file changed, 51 insertions(+), 38 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/49/306249/1
diff --git a/includes/filerepo/file/LocalFile.php
b/includes/filerepo/file/LocalFile.php
index 7e6e651..40141c9 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -2216,8 +2216,9 @@
}
protected function doDBInserts() {
+ $now = time();
$dbw = $this->file->repo->getMasterDB();
- $encTimestamp = $dbw->addQuotes( $dbw->timestamp() );
+ $encTimestamp = $dbw->addQuotes( $dbw->timestamp( $now ) );
$encUserId = $dbw->addQuotes( $this->user->getId() );
$encReason = $dbw->addQuotes( $this->reason );
$encGroup = $dbw->addQuotes( 'deleted' );
@@ -2239,15 +2240,15 @@
}
if ( $deleteCurrent ) {
- $concat = $dbw->buildConcat( [ "img_sha1", $encExt ] );
- $where = [ 'img_name' => $this->file->getName() ];
- $dbw->insertSelect( 'filearchive', 'image',
+ $dbw->insertSelect(
+ 'filearchive',
+ 'image',
[
'fa_storage_group' => $encGroup,
'fa_storage_key' => $dbw->conditional(
[ 'img_sha1' => '' ],
$dbw->addQuotes( '' ),
- $concat
+ $dbw->buildConcat( [
"img_sha1", $encExt ] )
),
'fa_deleted_user' => $encUserId,
'fa_deleted_timestamp' => $encTimestamp,
@@ -2268,44 +2269,56 @@
'fa_user' => 'img_user',
'fa_user_text' => 'img_user_text',
'fa_timestamp' => 'img_timestamp',
- 'fa_sha1' => 'img_sha1',
- ], $where, __METHOD__ );
+ 'fa_sha1' => 'img_sha1'
+ ],
+ [ 'img_name' => $this->file->getName() ],
+ __METHOD__
+ );
}
if ( count( $oldRels ) ) {
- $concat = $dbw->buildConcat( [ "oi_sha1", $encExt ] );
- $where = [
- 'oi_name' => $this->file->getName(),
- 'oi_archive_name' => array_keys( $oldRels ) ];
- $dbw->insertSelect( 'filearchive', 'oldimage',
+ $res = $dbw->select(
+ 'oldimage',
+ OldLocalFile::selectFields(),
[
- 'fa_storage_group' => $encGroup,
- 'fa_storage_key' => $dbw->conditional(
- [ 'oi_sha1' => '' ],
- $dbw->addQuotes( '' ),
- $concat
- ),
- 'fa_deleted_user' => $encUserId,
- 'fa_deleted_timestamp' => $encTimestamp,
- 'fa_deleted_reason' => $encReason,
- 'fa_deleted' => $this->suppress ?
$bitfield : 'oi_deleted',
+ 'oi_name' => $this->file->getName(),
+ 'oi_archive_name' => array_keys(
$oldRels )
+ ],
+ __METHOD__,
+ [ 'FOR UPDATE' ]
+ );
+ $rowsInsert = [];
+ foreach ( $res as $row ) {
+ $rowsInsert[] = [
+
