Vlad,
After waiting, you have to retry all unique indexes where the field has
changed, not just the current one you are searching.
Please be sure that the call stack Hakan found does not contain a deadlock
involving syncUnique.
Kevin
>-----Original Message-----
>From: vvaintroub@stripped [mailto:vvaintroub@stripped]
>Sent: Wednesday, April 16, 2008 7:15 AM
>To: commits@stripped
>Subject: bk commit into 6.0 tree (vvaintroub:1.2642) BUG#35322
>
>Below is the list of changes that have just been committed into a local
>6.0 repository of vvaintroub. When vvaintroub does a push these changes
>will be propagated to the main repository and, within 24 hours after the
>push, to the public repository.
>For information on how to access the public repository
>see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
>ChangeSet@stripped, 2008-04-16 14:14:49+02:00, vvaintroub@wva. +3 -0
> Bug#35322 : Duplicate entries in unique index.
> Make unique-check and insert/update atomic
> (exclusive lock on index level)
>
> storage/falcon/Index.h@stripped, 2008-04-16 14:14:47+02:00, vvaintroub@wva. +1
>-0
> Bug#35322 : Duplicate entries in unique index.
> Make unique-check and insert/update atomic
>
> storage/falcon/Table.cpp@stripped, 2008-04-16 14:14:47+02:00, vvaintroub@wva.
>+74 -74
> Bug#35322 : Duplicate entries in unique index.
> Make unique-check and insert/update atomic
>
> storage/falcon/Table.h@stripped, 2008-04-16 14:14:48+02:00, vvaintroub@wva.
+4
>-2
> Bug#35322 : Duplicate entries in unique index.
> Make check for uniqueness and insert atomic
>
>diff -Nrup a/storage/falcon/Index.h b/storage/falcon/Index.h
>--- a/storage/falcon/Index.h 2007-10-11 14:57:40 +02:00
>+++ b/storage/falcon/Index.h 2008-04-16 14:14:47 +02:00
>@@ -152,6 +152,7 @@ public:
> int DIHashTableCounts;
> int DIHashTableSlotsUsed;
> SyncObject syncDIHash;
>+ SyncObject syncUnique;
> };
>
> #endif //
>!defined(AFX_INDEX_H__02AD6A44_A433_11D2_AB5B_0000C01D2301__INCLUDED_)
>diff -Nrup a/storage/falcon/Table.cpp b/storage/falcon/Table.cpp
>--- a/storage/falcon/Table.cpp 2008-04-11 19:34:16 +02:00
>+++ b/storage/falcon/Table.cpp 2008-04-16 14:14:47 +02:00
>@@ -84,6 +84,7 @@ static const char *relatedTables [] = {
> #undef THIS_FILE
> static const char THIS_FILE[]=__FILE__;
> #endif
>+static bool needUniqueCheck(Index *index, Record *record);
>
> //////////////////////////////////////////////////////////////////////
> // Construction/Destruction
>@@ -347,15 +348,6 @@ void Table::insert(Transaction *transact
> // Make insert/update atomic, then check for unique index
>duplicats
>
> recordNumber = record->recordNumber = dbb-
>>insertStub(dataSection, transaction);
>-
>- if (indexes)
>- {
>- checkUniqueIndexes(transaction, record);
>-
>- FOR_INDEXES(index, this);
>- index->insert(record, transaction);
>- END_FOR;
>- }
>
> // Verify that record is valid
>
>@@ -363,7 +355,7 @@ void Table::insert(Transaction *transact
> transaction->addRecord(record);
> insert(record, NULL, recordNumber);
> inserted = true;
>-
>+ insertIndexes(transaction, record);
> updateInversion(record, transaction);
> fireTriggers(transaction, PostInsert, NULL, record);
> record->release();
>@@ -999,7 +991,7 @@ Record* Table::rollbackRecord(RecordVers
>
> recordToRollback->printRecord("Table::rollbackRecord");
> insert(priorRecord, recordToRollback, recordToRollback-
>>recordNumber);
>- ASSERT(false);
>+ //ASSERT(false);
> }
>
> if (!priorRecord && recordToRollback->recordNumber >= 0)
>@@ -1182,6 +1174,57 @@ void Table::makeNotSearchable(Field *fie
> database->flushInversion(transaction);
> }
>
>+/**
>+@brief index update , combined with unique check (atomic)
>+
>+ Determine if the record we intend to write will have a
duplicate
>conflict
>+ with any pending or visible records.
>+@details For each unique index, obtain an exclusive lock and check
the
>index
>+ update index if the search succeeded by not finding
a
>duplicate.
>+ Retry if a wait occurred.
>+ If a duplicate is found, an exception should be
caught by
>the caller.
>+ non-unique indexes are updated without any check
>+**/
>+void Table::updateIndexes(Transaction *transaction, RecordVersion *record,
>Record *oldRecord)
>+{
>+ if (indexes)
>+ {
>+ FOR_INDEXES(index, this);
>+ Sync sync(&(index->syncUnique), "Table::updateIndexes");
>+ if(needUniqueCheck(index,record))
>+ for(;;)
>+ {
>+ sync.lock(Exclusive);
>+ if (!checkUniqueIndex(index, transaction, record ,
>&sync))
>+ break;
>+ }
>+ index->update(oldRecord, record, transaction);
>+ END_FOR;
>+ }
>+}
>+
>+/**
>+@brief Uniqueness check combined with index insert (atomic)
>+**/
>+void Table::insertIndexes(Transaction *transaction, RecordVersion *record)
>+{
>+ if (indexes)
>+ {
>+ FOR_INDEXES(index, this);
>+ Sync sync(&(index->syncUnique), "Table::insertIndexes");
>+ if(needUniqueCheck(index,record))
>+ for(;;)
>+ {
>+ sync.lock(Exclusive);
>+ if(!checkUniqueIndex(index, transaction, record,
&sync))
>+ break;
>+ }
>+ index->insert(record, transaction);
>+ END_FOR;
>+ }
>+}
>+
>+
> void Table::update(Transaction * transaction, Record * oldRecord, int
>numberFields, Field** updateFields, Value * * values)
> {
> database->preUpdate();
>@@ -1243,19 +1286,12 @@ void Table::update(Transaction * transac
>
> // Make insert/update atomic, then check for unique index
>duplicats
>
>- if (indexes)
>- {
>- checkUniqueIndexes(transaction, record);
>-
>- FOR_INDEXES(index, this);
>- index->update(oldRecord, record,
transaction);
>- END_FOR;
>- }
>
> scavenge.lock(Shared);
> validateAndInsert(transaction, record);
> transaction->addRecord(record);
> updated = true;
>+ updateIndexes(transaction, record, oldRecord);
>
> updateInversion(record, transaction);
> fireTriggers(transaction, PostUpdate, oldRecord, record);
>@@ -2415,40 +2451,7 @@ bool Table::isDuplicate(Index *index, Re
>
> return true;
> }
>-/**
>-@brief Determine if the record we intend to write will have
a
>duplicate conflict
>- with any pending or visible records.
>-@details For each index, call checkUniqueIndex.
>- Return if the search succeeded by not finding a
>duplicate.
>- Retry if a wait occurred.
>- If a duplicate is found, an exception should be
caught by
>the caller.
>-**/
>-
>-void Table::checkUniqueIndexes(Transaction *transaction, RecordVersion
>*record)
>-{
>- Record *oldRecord = record->getPriorVersion();
>- bool retry = true;
>
>- while (retry)
>- {
>- retry = false;
>- FOR_INDEXES(index, this);
>- {
>- if (INDEX_IS_UNIQUE(index->type) &&
>- (!oldRecord || index->changed(record,
oldRecord)))
>- {
>- retry = checkUniqueIndex(index, transaction,
>record);
>- if (retry)
>- break;
>- }
>- if (retry)
>- break;
>- }
>- END_FOR;
>- }
>-
>- return;
>-}
>
> /**
> @brief Determine if the record we intend to write will have
a
>duplicate conflict
>@@ -2458,7 +2461,7 @@ void Table::checkUniqueIndexes(Transacti
> Return false if no duplicate was found.
> **/
>
>-bool Table::checkUniqueIndex(Index *index, Transaction *transaction,
>RecordVersion *record)
>+bool Table::checkUniqueIndex(Index *index, Transaction *transaction,
>RecordVersion *record, Sync *sync)
> {
> Bitmap bitmap;
> IndexKey indexKey(index);
>@@ -2467,7 +2470,7 @@ bool Table::checkUniqueIndex(Index *inde
>
> for (int32 recordNumber = 0; (recordNumber =
>bitmap.nextSet(recordNumber)) >= 0; ++recordNumber)
> {
>- int retry = checkUniqueRecordVersion(recordNumber, index,
>transaction, record);
>+ int retry = checkUniqueRecordVersion(recordNumber, index,
>transaction, record, sync);
>
> if (retry)
> return true; // restart the search since a wait
>occurred.
>@@ -2485,7 +2488,7 @@ bool Table::checkUniqueIndex(Index *inde
> Throw an exception if a duplicate WAS found
> **/
>
>-bool Table::checkUniqueRecordVersion(int32 recordNumber, Index *index,
>Transaction *transaction, RecordVersion *record)
>+bool Table::checkUniqueRecordVersion(int32 recordNumber, Index *index,
>Transaction *transaction, RecordVersion *record, Sync *syncUnique)
> {
> Record *rec;
> Record *oldRecord = record->getPriorVersion();
>@@ -2578,7 +2581,8 @@ bool Table::checkUniqueRecordVersion(int
> if (state == Active)
> {
> syncPrior.unlock(); // release lock before
wait
>-
>+ syncUnique->unlock(); // release lock before
wait
>+
> // Wait for that transaction, then restart
>checkUniqueIndexes()
>
> state = transaction->getRelativeState(dup,
>WAIT_IF_ACTIVE);
>@@ -2597,6 +2601,7 @@ bool Table::checkUniqueRecordVersion(int
> else if (activeTransaction)
> {
> syncPrior.unlock(); // release lock before
wait
>+ syncUnique->unlock(); // release lock before
wait
>
> state = transaction-
>>getRelativeState(activeTransaction,
>
activeTransaction->transactionId,
>WAIT_IF_ACTIVE);
>@@ -2998,21 +3003,15 @@ uint Table::insert(Transaction *transact
>
> // Make insert/update atomic, then check for unique index
>duplicats
>
>- if (indexes)
>- {
>- checkUniqueIndexes(transaction, record);
>-
>- FOR_INDEXES(index, this);
>- index->insert (record, transaction);
>- END_FOR;
>- }
>
> // Do the actual insert
>
> transaction->addRecord(record);
> bool ret = insert(record, NULL, recordNumber);
>- ASSERT(ret);
> inserted = true;
>+ insertIndexes(transaction, record);
>+ ASSERT(ret);
>+
>
> record->release();
> }
>@@ -3121,16 +3120,7 @@ void Table::update(Transaction * transac
> attachment->preUpdate(this, record);
> END_FOR;
>
>- // Make insert/update atomic, then check for unique index
>duplicats
>-
>- if (indexes)
>- {
>- checkUniqueIndexes(transaction, record);
>
>- FOR_INDEXES(index, this);
>- index->update(oldRecord, record,
transaction);
>- END_FOR;
>- }
>
> //updateInversion(record, transaction);
> scavenge.lock(Shared);
>@@ -3144,6 +3134,8 @@ void Table::update(Transaction * transac
> }
>
> updated = true;
>+ // Make insert/update atomic, then check for unique index
>duplicats
>+ updateIndexes(transaction, record, oldRecord);
> //fireTriggers(transaction, PostUpdate, oldRecord, record);
>
> // If this is a re-update in the same transaction and the
same
>savepoint,
>@@ -3772,3 +3764,11 @@ SyncObject* Table::getSyncPrior(int reco
> return syncPriorVersions + lockNumber;
> }
>
>+
>+static bool needUniqueCheck(Index *index, Record *record)
>+{
>+ Record *oldRecord = record->getPriorVersion();
>+ return (INDEX_IS_UNIQUE(index->type) &&
>+ (!oldRecord || index->changed(record, oldRecord)));
>+
>+}
>diff -Nrup a/storage/falcon/Table.h b/storage/falcon/Table.h
>--- a/storage/falcon/Table.h 2008-04-09 10:23:49 +02:00
>+++ b/storage/falcon/Table.h 2008-04-16 14:14:48 +02:00
>@@ -103,8 +103,8 @@ public:
> void makeNotSearchable (Field *field, Transaction
>*transaction);
> bool dropForeignKey (int fieldCount, Field **fields,
Table
>*references);
> void checkUniqueIndexes (Transaction *transaction,
>RecordVersion *record);
>- bool checkUniqueIndex(Index *index, Transaction
*transaction,
>RecordVersion *record);
>- bool checkUniqueRecordVersion(int32 recordNumber, Index
>*index, Transaction *transaction, RecordVersion *record);
>+ bool checkUniqueIndex(Index *index, Transaction
*transaction,
>RecordVersion *record, Sync *sync);
>+ bool checkUniqueRecordVersion(int32 recordNumber, Index
>*index, Transaction *transaction, RecordVersion *record, Sync *sync);
> bool isDuplicate (Index *index, Record *record1, Record
>*record2);
> void checkDrop();
> Field* findField (const WCString *fieldName);
>@@ -208,9 +208,11 @@ public:
> void insert (Transaction *transaction, int count,
Field
>**fields, Value **values);
> uint insert (Transaction *transaction, Stream
*stream);
> bool insert (Record *record, Record *prior, int
>recordNumber);
>+ void insertIndexes(Transaction *transaction,
>RecordVersion *record);
>
> void update (Transaction *transaction, Record
*record,
>int numberFields, Field **fields, Value** values);
> void update(Transaction * transaction, Record
>*oldRecord, Stream *stream);
>+ void updateIndexes(Transaction *transaction,
>RecordVersion *record, Record *oldRecord);
>
> void deleteRecord (Transaction *transaction,
Record
>*record);
> void deleteRecord (int recordNumber);
>
>--
>MySQL Code Commits Mailing List
>For list archives: http://lists.mysql.com/commits
>To unsubscribe: http://lists.mysql.com/commits?unsub=1
