jenkins-bot has submitted this change and it was merged. Change subject: Avoid holding many open DB connections ......................................................................
Avoid holding many open DB connections
It has been decided that the small risk of inconsistent data due to
individual jump-wiki updates failing is worth avoiding breaking the DBs
by opening hundreds of connections.
Bug: T78013
Change-Id: Ib24609134dfa81bd26706e620badea4ae7baea0c
---
M includes/pages/CreatePage.php
M includes/pages/VoterEligibilityPage.php
2 files changed, 295 insertions(+), 237 deletions(-)
Approvals:
Tim Starling: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/pages/CreatePage.php b/includes/pages/CreatePage.php
index 91219c5..8e89d70 100644
--- a/includes/pages/CreatePage.php
+++ b/includes/pages/CreatePage.php
@@ -370,15 +370,12 @@
return Status::newFatal(
'securepoll-create-fail-bad-id' );
}
- $dbws = array();
- $dbw = wfGetDB( DB_MASTER );
- $dbw->begin();
- $dbws[] = $dbw;
+ $dbw = $this->context->getDB();
try {
$properties = array();
$messages = array();
- // Check for duplicate titles
+ // Check for duplicate titles on the local wiki
$id = $dbw->selectField( 'securepoll_elections',
'el_entity', array(
'el_title' => $election->title
), __METHOD__, array( 'FOR UPDATE' ) );
@@ -388,105 +385,150 @@
);
}
- if ( $election->getId() > 0 ) {
- $id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
- 'el_entity' => $election->getId()
- ), __METHOD__, array( 'FOR UPDATE' ) );
- if ( !$id ) {
- return Status::newFatal(
'securepoll-create-fail-id-missing' );
- }
- }
+ // Check for duplicate titles on jump wikis too
+ // (There's the possibility for a race here, but
hopefully it won't
+ // matter in practice)
+ if ( $store->rId ) {
+ foreach ( $store->remoteWikis as $dbname ) {
+ $lb = wfGetLB( $dbname );
+ $rdbw = $lb->getConnection( DB_MASTER,
array(), $dbname );
- // Insert or update the election entity
- $fields = array(
- 'el_title' => $election->title,
- 'el_ballot' => $election->ballotType,
- 'el_tally' => $election->tallyType,
- 'el_primary_lang' => $election->getLanguage(),
- 'el_start_date' => $dbw->timestamp(
$election->getStartDate() ),
- 'el_end_date' => $dbw->timestamp(
$election->getEndDate() ),
- 'el_auth_type' => $election->authType,
- );
- if ( $election->getId() < 0 ) {
- $eId = self::insertEntity( $dbw, 'election' );
- $qIds = array();
- $oIds = array();
- $fields['el_entity'] = $eId;
- $dbw->insert( 'securepoll_elections', $fields,
__METHOD__ );
- } else {
- $eId = $election->getId();
- $dbw->update( 'securepoll_elections', $fields,
array( 'el_entity' => $eId ), __METHOD__ );
-
- // Delete any questions or options that weren't
included in the
- // form submission.
- $qIds = array();
- $res = $dbw->select( 'securepoll_questions',
'qu_entity', array( 'qu_election' => $eId ) );
- foreach ( $res as $row ) {
- $qIds[] = $row->qu_entity;
- }
- $oIds = array();
- $res = $dbw->select( 'securepoll_options',
'op_entity', array( 'op_election' => $eId ) );
- foreach ( $res as $row ) {
- $oIds[] = $row->op_entity;
- }
- $deleteIds = array_merge(
- array_diff( $qIds, $store->qIds ),
- array_diff( $oIds, $store->oIds )
- );
- if ( $deleteIds ) {
- $dbw->delete( 'securepoll_msgs', array(
'msg_entity' => $deleteIds ), __METHOD__ );
- $dbw->delete( 'securepoll_properties',
array( 'pr_entity' => $deleteIds ), __METHOD__ );
- $dbw->delete( 'securepoll_questions',
array( 'qu_entity' => $deleteIds ), __METHOD__ );
- $dbw->delete( 'securepoll_options',
array( 'op_entity' => $deleteIds ), __METHOD__ );
- $dbw->delete( 'securepoll_entity',
array( 'en_id' => $deleteIds ), __METHOD__ );
- }
- }
- self::savePropertiesAndMessages( $dbw, $eId, $election
);
-
- // Now do questions and options
- $qIndex = 0;
- foreach ( $election->getQuestions() as $question ) {
- $qId = $question->getId();
- if ( !in_array( $qId, $qIds ) ) {
- $qId = self::insertEntity( $dbw,
'question' );
- }
- $dbw->replace( 'securepoll_questions',
- array( 'qu_entity' ),
- array(
- 'qu_entity' => $qId,
- 'qu_election' => $eId,
- 'qu_index' => ++$qIndex,
- ),
- __METHOD__
- );
- self::savePropertiesAndMessages( $dbw, $qId,
$question );
-
- foreach ( $question->getOptions() as $option ) {
- $oId = $option->getId();
- if ( !in_array( $oId, $oIds ) ) {
- $oId = self::insertEntity(
$dbw, 'option' );
- }
- $dbw->replace( 'securepoll_options',
- array( 'op_entity' ),
+ // Find an existing dummy election, if
any
+ $rId = $rdbw->selectField(
+ array( 'p1' =>
'securepoll_properties', 'p2' => 'securepoll_properties' ),
+ 'p1.pr_entity',
array(
- 'op_entity' => $oId,
- 'op_election' => $eId,
- 'op_question' => $qId,
+ 'p1.pr_entity =
p2.pr_entity',
+ 'p1.pr_key' =>
'jump-id',
+ 'p1.pr_value' =>
$election->getId(),
+ 'p2.pr_key' =>
'main-wiki',
+ 'p2.pr_value' =>
wfWikiID(),
+ )
+ );
+
+ // Test for duplicate title
+ $id = $rdbw->selectField(
'securepoll_elections', 'el_entity', array(
+ 'el_title' =>
$formData['election_title']
+ ) );
+ if ( $id && $id !== $rId ) {
+ throw new
SecurePoll_StatusException( 'securepoll-create-duplicate-title',
+
SecurePoll_FormStore::getWikiName( $dbname ), $dbname
+ );
+ }
+ $lb->reuseConnection( $rdbw );
+ }
+ }
+
+ // Ok, begin the actual work
+ $dbw->begin();
+ try {
+ if ( $election->getId() > 0 ) {
+ $id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
+ 'el_entity' =>
$election->getId()
+ ), __METHOD__, array( 'FOR UPDATE' ) );
+ if ( !$id ) {
+ return Status::newFatal(
'securepoll-create-fail-id-missing' );
+ }
+ }
+
+ // Insert or update the election entity
+ $fields = array(
+ 'el_title' => $election->title,
+ 'el_ballot' => $election->ballotType,
+ 'el_tally' => $election->tallyType,
+ 'el_primary_lang' =>
$election->getLanguage(),
+ 'el_start_date' => $dbw->timestamp(
$election->getStartDate() ),
+ 'el_end_date' => $dbw->timestamp(
$election->getEndDate() ),
+ 'el_auth_type' => $election->authType,
+ );
+ if ( $election->getId() < 0 ) {
+ $eId = self::insertEntity( $dbw,
'election' );
+ $qIds = array();
+ $oIds = array();
+ $fields['el_entity'] = $eId;
+ $dbw->insert( 'securepoll_elections',
$fields, __METHOD__ );
+ } else {
+ $eId = $election->getId();
+ $dbw->update( 'securepoll_elections',
$fields, array( 'el_entity' => $eId ), __METHOD__ );
+
+ // Delete any questions or options that
weren't included in the
+ // form submission.
+ $qIds = array();
+ $res = $dbw->select(
'securepoll_questions', 'qu_entity', array( 'qu_election' => $eId ) );
+ foreach ( $res as $row ) {
+ $qIds[] = $row->qu_entity;
+ }
+ $oIds = array();
+ $res = $dbw->select(
'securepoll_options', 'op_entity', array( 'op_election' => $eId ) );
+ foreach ( $res as $row ) {
+ $oIds[] = $row->op_entity;
+ }
+ $deleteIds = array_merge(
+ array_diff( $qIds, $store->qIds
),
+ array_diff( $oIds, $store->oIds
)
+ );
+ if ( $deleteIds ) {
+ $dbw->delete(
'securepoll_msgs', array( 'msg_entity' => $deleteIds ), __METHOD__ );
+ $dbw->delete(
'securepoll_properties', array( 'pr_entity' => $deleteIds ), __METHOD__ );
+ $dbw->delete(
'securepoll_questions', array( 'qu_entity' => $deleteIds ), __METHOD__ );
+ $dbw->delete(
'securepoll_options', array( 'op_entity' => $deleteIds ), __METHOD__ );
+ $dbw->delete(
'securepoll_entity', array( 'en_id' => $deleteIds ), __METHOD__ );
+ }
+ }
+ self::savePropertiesAndMessages( $dbw, $eId,
$election );
+
+ // Now do questions and options
+ $qIndex = 0;
+ foreach ( $election->getQuestions() as
$question ) {
+ $qId = $question->getId();
+ if ( !in_array( $qId, $qIds ) ) {
+ $qId = self::insertEntity(
$dbw, 'question' );
+ }
+ $dbw->replace( 'securepoll_questions',
+ array( 'qu_entity' ),
+ array(
+ 'qu_entity' => $qId,
+ 'qu_election' => $eId,
+ 'qu_index' => ++$qIndex,
),
__METHOD__
);
- self::savePropertiesAndMessages( $dbw,
$oId, $option );
+ self::savePropertiesAndMessages( $dbw,
$qId, $question );
+
+ foreach ( $question->getOptions() as
$option ) {
+ $oId = $option->getId();
+ if ( !in_array( $oId, $oIds ) )
{
+ $oId =
self::insertEntity( $dbw, 'option' );
+ }
+ $dbw->replace(
'securepoll_options',
+ array( 'op_entity' ),
+ array(
+ 'op_entity' =>
$oId,
+ 'op_election'
=> $eId,
+ 'op_question'
=> $qId,
+ ),
+ __METHOD__
+ );
+
self::savePropertiesAndMessages( $dbw, $oId, $option );
+ }
}
+ $dbw->commit();
+ } catch ( Exception $ex ) {
+ $dbw->rollback();
+ throw $ex;
}
+ } catch ( SecurePoll_StatusException $ex ) {
+ return $ex->status;
+ }
- // Create the "redirect" polls on all the local wikis
- if ( $store->rId ) {
- $election = $context->getElection( $store->rId
);
- foreach ( $store->remoteWikis as $dbname ) {
- $dbw = wfGetDB( DB_MASTER, array(),
$dbname );
- $dbw->begin();
- $dbws[] = $dbw;
-
+ // Create the "redirect" polls on all the local wikis
+ if ( $store->rId ) {
+ $election = $context->getElection( $store->rId );
+ foreach ( $store->remoteWikis as $dbname ) {
+ $lb = wfGetLB( $dbname );
+ $dbw = $lb->getConnection( DB_MASTER, array(),
$dbname );
+ $dbw->begin();
+ try {
// Find an existing dummy election, if
any
$rId = $dbw->selectField(
array( 'p1' =>
'securepoll_properties', 'p2' => 'securepoll_properties' ),
@@ -501,16 +543,6 @@
);
if ( !$rId ) {
$rId = self::insertEntity(
$dbw, 'election' );
- }
-
- // Check for duplicate title
- $id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
- 'el_title' =>
$formData['election_title']
- ) );
- if ( $id && $id !== $rId ) {
- throw new
SecurePoll_StatusException( 'securepoll-create-duplicate-title',
-
SecurePoll_FormStore::getWikiName( $dbname ), $dbname
- );
}
// Insert it! We don't have to care
about questions or options here.
@@ -536,23 +568,13 @@
array( 'pr_entity' => $rId,
'pr_key' => 'jump-id' ),
__METHOD__
);
+ $dbw->commit();
+ } catch ( Exception $ex ) {
+ $dbw->rollback();
+ MWExceptionHandler::logException( $ex );
}
+ $lb->reuseConnection( $dbw );
}
-
- // Now commit all the transactions at once
- foreach ( $dbws as $dbw ) {
- $dbw->commit();
- }
- } catch ( SecurePoll_StatusException $ex ) {
- foreach ( $dbws as $dbw ) {
- $dbw->rollback();
- }
- return $ex->status;
- } catch ( Exception $ex ) {
- foreach ( $dbws as $dbw ) {
- $dbw->rollback();
- }
- throw $ex;
}
// Record this election to the SecurePoll namespace, if so
configured.
diff --git a/includes/pages/VoterEligibilityPage.php
b/includes/pages/VoterEligibilityPage.php
index b8e7662..d79fea5 100644
--- a/includes/pages/VoterEligibilityPage.php
+++ b/includes/pages/VoterEligibilityPage.php
@@ -80,61 +80,65 @@
private function saveProperties( $properties, $delete, $comment ) {
global $wgSecurePollUseNamespace;
+ $localWiki = wfWikiID();
$wikis = $this->election->getProperty( 'wikis' );
if ( $wikis ) {
$wikis = explode( "\n", $wikis );
- if ( !in_array( wfWikiID(), $wikis ) ) {
- $wikis[] = wfWikiID();
+ $i = array_search( $localWiki, $wikis );
+ if ( $i !== false ) {
+ unset( $wikis[$i] );
}
+ array_unshift( $wikis, $localWiki );
} else {
- $wikis = array( wfWikiID() );
+ $wikis = array( $localWiki );
}
- $dbws = array();
- try {
- foreach ( $wikis as $dbname ) {
- $dbw = wfGetDB( DB_MASTER, array(), $dbname );
+ foreach ( $wikis as $dbname ) {
+ if ( $dbname === $localWiki ) {
+ $dbw = $this->context->getDB();
+ } else {
+ $lb = wfGetLB( $dbname );
+ $dbw = $lb->getConnection( DB_MASTER, array(),
$dbname );
+ }
+ try {
$dbw->begin();
- $dbws[] = $dbw;
-
$id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
'el_title' => $this->election->title
) );
- if ( !$id ) {
- // WTF?
- continue;
- }
+ if ( $id ) {
+ $ins = array();
+ foreach ( $properties as $key => $value
) {
+ $ins[] = array(
+ 'pr_entity' => $id,
+ 'pr_key' => $key,
+ 'pr_value' => $value,
+ );
+ }
- $ins = array();
- foreach ( $properties as $key => $value ) {
- $ins[] = array(
- 'pr_entity' => $id,
- 'pr_key' => $key,
- 'pr_value' => $value,
+ $dbw->delete( 'securepoll_properties',
+ array(
+ 'pr_entity' => $id,
+ 'pr_key' =>
array_merge( $delete, array_keys( $properties ) ),
+ )
);
+
+ if ( $ins ) {
+ $dbw->insert(
'securepoll_properties', $ins );
+ }
}
-
- $dbw->delete( 'securepoll_properties',
- array(
- 'pr_entity' => $id,
- 'pr_key' => array_merge(
$delete, array_keys( $properties ) ),
- )
- );
-
- if ( $ins ) {
- $dbw->insert( 'securepoll_properties',
$ins );
- }
- }
-
- // Now commit all the transactions at once
- foreach ( $dbws as $dbw ) {
$dbw->commit();
- }
- } catch ( Exception $ex ) {
- foreach ( $dbws as $dbw ) {
+ } catch ( Exception $ex ) {
$dbw->rollback();
+ // If it's for the local wiki, rethrow.
Otherwise, just log but
+ // still update the jump wikis.
+ if ( $dbname === $localWiki ) {
+ throw $ex;
+ }
+ MWExceptionHandler::logException( $ex );
}
- throw $ex;
+ if ( $dbname !== $localWiki ) {
+ $lb->reuseConnection( $dbw );
+ }
}
// Record this election to the SecurePoll namespace, if so
configured.
@@ -230,61 +234,77 @@
$list = "{$this->election->getId()}/list/$property";
- $dbws = array();
- try {
- foreach ( $wikis as $dbname ) {
- $dbw = wfGetDB( DB_MASTER, array(), $dbname );
+ $localWiki = wfWikiID();
+ $wikis = $this->election->getProperty( 'wikis' );
+ if ( $wikis ) {
+ $wikis = explode( "\n", $wikis );
+ $i = array_search( $localWiki, $wikis );
+ if ( $i !== false ) {
+ unset( $wikis[$i] );
+ }
+ array_unshift( $wikis, $localWiki );
+ } else {
+ $wikis = array( $localWiki );
+ }
+
+ foreach ( $wikis as $dbname ) {
+ if ( $dbname === $localWiki ) {
+ $dbw = $this->context->getDB();
+ } else {
+ $lb = wfGetLB( $dbname );
+ $dbw = $lb->getConnection( DB_MASTER, array(),
$dbname );
+ }
+ try {
$dbw->begin();
- $dbws[] = $dbw;
$id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
'el_title' => $this->election->title
) );
- if ( !$id ) {
- // WTF?
- continue;
- }
-
- $dbw->replace( 'securepoll_properties',
- array( 'pr_entity', 'pr_key' ),
- array(
- 'pr_entity' => $id,
- 'pr_key' => $property,
- 'pr_value' => $list,
- )
- );
-
- if ( isset( $wikiNames[$dbname] ) ) {
- $queryNames = array_merge(
$wikiNames['*'], $wikiNames[$dbname] );
- } else {
- $queryNames = $wikiNames['*'];
- }
-
- $dbw->delete( 'securepoll_lists', array(
'li_name' => $list ) );
- if ( $queryNames ) {
- $dbw->insertSelect(
- 'securepoll_lists',
- 'user',
+ if ( $id ) {
+ $dbw->replace( 'securepoll_properties',
+ array( 'pr_entity', 'pr_key' ),
array(
- 'li_name' =>
$dbw->addQuotes( $list ),
- 'li_member' => 'user_id'
- ),
- array(
- 'user_name' =>
$queryNames,
+ 'pr_entity' => $id,
+ 'pr_key' => $property,
+ 'pr_value' => $list,
)
);
- }
- }
- // Now commit all the transactions at once
- foreach ( $dbws as $dbw ) {
+ if ( isset( $wikiNames[$dbname] ) ) {
+ $queryNames = array_merge(
$wikiNames['*'], $wikiNames[$dbname] );
+ } else {
+ $queryNames = $wikiNames['*'];
+ }
+
+ $dbw->delete( 'securepoll_lists',
array( 'li_name' => $list ) );
+ if ( $queryNames ) {
+ $dbw->insertSelect(
+ 'securepoll_lists',
+ 'user',
+ array(
+ 'li_name' =>
$dbw->addQuotes( $list ),
+ 'li_member' =>
'user_id'
+ ),
+ array(
+ 'user_name' =>
$queryNames,
+ )
+ );
+ }
+ }
+
$dbw->commit();
- }
- } catch ( Exception $ex ) {
- foreach ( $dbws as $dbw ) {
+ } catch ( Exception $ex ) {
$dbw->rollback();
+ // If it's for the local wiki, rethrow.
Otherwise, just log but
+ // still update the jump wikis.
+ if ( $dbname === $localWiki ) {
+ throw $ex;
+ }
+ MWExceptionHandler::logException( $ex );
}
- throw $ex;
+ if ( $dbname !== $localWiki ) {
+ $lb->reuseConnection( $dbw );
+ }
}
// Record this election to the SecurePoll namespace, if so
configured.
@@ -788,52 +808,68 @@
$wikis = array( wfWikiID() );
}
- $dbws = array();
- try {
- foreach ( $wikis as $dbname ) {
- $dbw = wfGetDB( DB_MASTER, array(), $dbname );
+ $localWiki = wfWikiID();
+ $wikis = $this->election->getProperty( 'wikis' );
+ if ( $wikis ) {
+ $wikis = explode( "\n", $wikis );
+ $i = array_search( $localWiki, $wikis );
+ if ( $i !== false ) {
+ unset( $wikis[$i] );
+ }
+ array_unshift( $wikis, $localWiki );
+ } else {
+ $wikis = array( $localWiki );
+ }
+
+ foreach ( $wikis as $dbname ) {
+ if ( $dbname === $localWiki ) {
+ $dbw = $this->context->getDB();
+ } else {
+ $lb = wfGetLB( $dbname );
+ $dbw = $lb->getConnection( DB_MASTER, array(),
$dbname );
+ }
+ try {
$dbw->begin();
- $dbws[] = $dbw;
$id = $dbw->selectField(
'securepoll_elections', 'el_entity', array(
'el_title' => $this->election->title
) );
- if ( !$id ) {
- // WTF?
- continue;
- }
-
- $list = $dbw->selectField(
'securepoll_properties', 'pr_value', array(
- 'pr_entity' => $id,
- 'pr_key' => $property,
- ) );
- if ( $list ) {
- $dbw->delete( 'securepoll_lists',
array( 'li_name' => $list ) );
- $dbw->delete( 'securepoll_properties',
- array( 'pr_entity' => $id,
'pr_key' => $property ) );
- }
-
- if ( $which === 'voter' ) {
- $dbw->delete( 'securepoll_properties',
array(
+ if ( $id ) {
+ $list = $dbw->selectField(
'securepoll_properties', 'pr_value', array(
'pr_entity' => $id,
- 'pr_key' => array(
- 'list_populate',
'list_job-key',
- 'list_total-count',
'list_complete-count',
- 'list_job-key',
- ),
+ 'pr_key' => $property,
) );
- }
- }
+ if ( $list ) {
+ $dbw->delete(
'securepoll_lists', array( 'li_name' => $list ) );
+ $dbw->delete(
'securepoll_properties',
+ array( 'pr_entity' =>
$id, 'pr_key' => $property ) );
+ }
- // Now commit all the transactions at once
- foreach ( $dbws as $dbw ) {
+ if ( $which === 'voter' ) {
+ $dbw->delete(
'securepoll_properties', array(
+ 'pr_entity' => $id,
+ 'pr_key' => array(
+
'list_populate', 'list_job-key',
+
'list_total-count', 'list_complete-count',
+ 'list_job-key',
+ ),
+ ) );
+ }
+ }
+
$dbw->commit();
- }
- } catch ( Exception $ex ) {
- foreach ( $dbws as $dbw ) {
+ } catch ( Exception $ex ) {
$dbw->rollback();
+ // If it's for the local wiki, rethrow.
Otherwise, just log but
+ // still update the jump wikis.
+ if ( $dbname === $localWiki ) {
+ throw $ex;
+ }
+ MWExceptionHandler::logException( $ex );
}
- throw $ex;
+ if ( $dbname !== $localWiki ) {
+ $lb->reuseConnection( $dbw );
+ }
}
// Record this election to the SecurePoll namespace, if so
configured.
--
To view, visit https://gerrit.wikimedia.org/r/178541
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib24609134dfa81bd26706e620badea4ae7baea0c
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/SecurePoll
Gerrit-Branch: master
Gerrit-Owner: Anomie <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
