namespace MediaWiki\Storage;
+use Exception;
use IExpiringStore;
use Psr\Log\LoggerInterface;
use WANObjectCache;
* Acquire the id of the given name.
* This creates a row in the table if it doesn't already exist.
*
+ * @note If called within an atomic section, there is a chance for the acquired ID
+ * to be lost on rollback. A best effort is made to re-insert the mapping
+ * in this case, and consistency of the cache with the database table is ensured
+ * by re-loading the map after a failed atomic section. However, there is no guarantee
+ * that an ID returned by this method is valid outside the transaction in which it
+ * was produced. This means that calling code should not retain the return value beyond
+ * the scope of a transaction, but rather call acquireId() again after the transaction
+ * is complete. In some rare cases, this may produce an ID different from the first call.
+ *
* @param string $name
* @throws NameTableAccessException
* @return int
$this->logger->error( $m );
throw new NameTableAccessException( $m );
}
- } elseif ( isset( $table[$id] ) ) {
- throw new NameTableAccessException(
- "Expected unused ID from database insert for '$name' "
- . " into '{$this->table}', but ID $id is already associated with"
- . " the name '{$table[$id]}'! This may indicate database corruption!" );
} else {
+ if ( isset( $table[$id] ) ) {
+ // This can happen when a transaction is rolled back and acquireId is called in
+ // an onTransactionResolution() callback, which gets executed before retryStore()
+ // has a chance to run. The right thing to do in this case is to discard the old
+ // value. According to the contract of acquireId, the caller should not have
+ // used it outside the transaction, so it should not be persisted anywhere after
+ // the rollback.
+ $m = "Got ID $id for '$name' from insert"
+ . " into '{$this->table}', but ID $id was previously associated with"
+ . " the name '{$table[$id]}'. Overriding the old value, which presumably"
+ . " has been removed from the database due to a transaction rollback.";
+
+ $this->logger->warning( $m );
+ }
+
$table[$id] = $name;
$searchResult = $id;
* @return string[] The freshly reloaded name map
*/
public function reloadMap( $connFlags = 0 ) {
+ if ( $connFlags !== 0 && defined( 'MW_PHPUNIT_TEST' ) ) {
+ // HACK: We can't use $connFlags while doing PHPUnit tests, because the
+ // fake database tables are bound to a single connection.
+ $connFlags = 0;
+ }
+
$dbw = $this->getDBConnection( DB_MASTER, $connFlags );
$this->tableCache = $this->loadTable( $dbw );
$dbw->onTransactionPreCommitOrIdle( function () {
$dbw = $this->getDBConnection( DB_MASTER );
- $dbw->insert(
- $this->table,
- $this->getFieldsToStore( $name ),
+ $id = null;
+ $dbw->doAtomicSection(
__METHOD__,
- [ 'IGNORE' ]
+ function ( IDatabase $unused, $fname )
+ use ( $name, &$id, $dbw ) {
+ // NOTE: use IDatabase from the parent scope here, not the function parameter.
+ // If $dbw is a wrapper around the actual DB, we need to call the wrapper here,
+ // not the inner instance.
+ $dbw->insert(
+ $this->table,
+ $this->getFieldsToStore( $name ),
+ $fname,
+ [ 'IGNORE' ]
+ );
+
+ if ( $dbw->affectedRows() === 0 ) {
+ $this->logger->info(
+ 'Tried to insert name into table ' . $this->table . ', but value already existed.'
+ );
+
+ return;
+ }
+
+ $id = $dbw->insertId();
+
+ // Any open transaction may still be rolled back. If that happens, we have to re-try the
+ // insertion and restore a consistent state of the cached table.
+ $dbw->onAtomicSectionCancel(
+ function ( $trigger, IDatabase $unused ) use ( $name, $id, $dbw ) {
+ $this->retryStore( $dbw, $name, $id );
+ },
+ $fname );
+ },
+ IDatabase::ATOMIC_CANCELABLE
);
- if ( $dbw->affectedRows() === 0 ) {
- $this->logger->info(
- 'Tried to insert name into table ' . $this->table . ', but value already existed.'
+ return $id;
+ }
+
+ /**
+ * After the initial insertion got rolled back, this can be used to try the insertion again,
+ * and ensure a consistent state of the cache.
+ *
+ * @param IDatabase $dbw
+ * @param string $name
+ * @param int $id
+ */
+ private function retryStore( IDatabase $dbw, $name, $id ) {
+ // NOTE: in the closure below, use the IDatabase from the original method call,
+ // not the one passed to the closure as a parameter.
+ // If $dbw is a wrapper around the actual DB, we need to call the wrapper,
+ // not the inner instance.
+
+ try {
+ $dbw->doAtomicSection(
+ __METHOD__,
+ function ( IDatabase $unused, $fname ) use ( $name, $id, &$ok, $dbw ) {
+ // Try to insert a row with the ID we originally got.
+ // If that fails (because of a key conflict), we will just try to get another ID again later.
+ $dbw->insert(
+ $this->table,
+ $this->getFieldsToStore( $name, $id ),
+ $fname
+ );
+
+ // Make sure we re-load the map in case this gets rolled back again.
+ // We could re-try once more, but that bears the risk of an infinite loop.
+ // So let's just give up on the ID.
+ $dbw->onAtomicSectionCancel(
+ function ( $trigger, IDatabase $unused ) use ( $name, $id, $dbw ) {
+ $this->logger->warning(
+ 'Re-insertion of name into table ' . $this->table
+ . ' was rolled back. Giving up and reloading the cache.'
+ );
+ $this->reloadMap( ILoadBalancer::CONN_TRX_AUTOCOMMIT );
+ },
+ $fname
+ );
+
+ $this->logger->info(
+ 'Re-insert name into table ' . $this->table . ' after failed transaction.'
+ );
+ },
+ IDatabase::ATOMIC_CANCELABLE
+ );
+ } catch ( Exception $ex ) {
+ $this->logger->error(
+ 'Re-insertion of name into table ' . $this->table . ' failed: ' . $ex->getMessage()
);
- return null;
+ } finally {
+ // NOTE: we reload regardless of whether the above insert succeeded. There is
+ // only three possibilities: the insert succeeded, so the new map will have
+ // the desired $id/$name mapping. Or the insert failed because another
+ // process already inserted that same $id/$name mapping, in which case the
+ // new map will also have it. Or another process grabbed the desired ID for
+ // another name, or the database refuses to insert the given ID into the
+ // auto increment field - in that case, the new map will not have a mapping
+ // for $name (or has a different mapping for $name). In that last case, we can
+ // only hope that the ID produced within the failed transaction has not been
+ // used outside that transaction.
+
+ $this->reloadMap( ILoadBalancer::CONN_TRX_AUTOCOMMIT );
}
-
- return $dbw->insertId();
}
/**
* @param string $name
+ * @param int|null $id
* @return array
*/
- private function getFieldsToStore( $name ) {
- $fields = [ $this->nameField => $name ];
+ private function getFieldsToStore( $name, $id = null ) {
+ $fields = [];
+
+ $fields[$this->nameField] = $name;
+
+ if ( $id !== null ) {
+ $fields[$this->idField] = $id;
+ }
+
if ( $this->insertCallback !== null ) {
$fields = call_user_func( $this->insertCallback, $fields );
}
dépôts / lhc / web / wiklou.git / blobdiff