bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2712) Bug#43344

[Kevin Lewis]

#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on revid:kevin.lewis@stripped

 2712 Kevin Lewis	2009-05-26
      Bug#43344 - This assert can still happen even after the chance of it being a chilled record is eliminated.  
      Since Table::checkUniqueRecordVersion() can be called for any record along a prior record chain at almost any time, it is possible that valid lock or deleted record versions have
      other temporary states such as recNoChill, recDeleting & recEndChain.  So this assertion is not valid and should be deleted.  The code after "getRelativeState(dup, DO_NOT_WAIT)" and "if (!dup->hasRecord()" should be changed to only check the state for a deleted record if the relative state is Us or CommittedVisible.

    modified:
      storage/falcon/Record.h
      storage/falcon/Table.cpp
=== modified file 'storage/falcon/Record.h'
--- a/storage/falcon/Record.h	2009-05-20 16:58:14 +0000
+++ b/storage/falcon/Record.h	2009-05-26 18:11:13 +0000
@@ -41,19 +41,19 @@ enum RecordEncoding {
 
 // Record states
 
-static const int recData      = 0;		// record pointer is valid or record is deleted
-static const int recDeleted   = 1;		// record has been deleted
-//static const int recChilled   = 2;		// record data is temporarily stored in serial log
-static const int recOnDisk    = 3;		// record is on disk and must be read
-static const int recLock      = 4;		// this is a "record lock" and not a record
-static const int recNoChill   = 5;		// record is in use and should not be chilled
-static const int recRollback  = 6;		// record is being rolled back
-static const int recUnlocked  = 7;		// record is being unlocked
-static const int recInserting = 8;		// record is being physically inserted
-static const int recDeleting  = 9;		// record is being physically deleted
-static const int recPruning   = 10;		// record is being pruned
-static const int recEndChain  = 11;		// end of chain for garbage collection
-static const int recQueuedForDelete = 12;		// Record is in purgatory.
+static const int recData            =  0;	// record pointer is valid or record is deleted
+static const int recDeleted         =  1;	// record has been deleted
+//static const int recChilled       =  2;	// obsolete - record data is temporarily stored in serial log
+static const int recOnDisk          =  3;	// record is on disk and must be read
+static const int recLock            =  4;	// this is a "record lock" and not a record
+static const int recNoChill         =  5;	// record is in use and should not be chilled
+static const int recRollback        =  6;	// record is being rolled back
+//static const int recUnlocked      =  7;	// obsolete - record is being unlocked
+static const int recInserting       =  8;	// record is being physically inserted
+static const int recDeleting        =  9;	// record is being physically deleted
+//static const int recPruning       = 10;	// obsolete - record is being pruned
+static const int recEndChain        = 11;	// end of chain for garbage collection
+static const int recQueuedForDelete = 12;	// Record is in purgatory.
 
 // Special value for data.record
 static const char* recordIsChilled = (char*) -1;

=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-05-20 16:58:14 +0000
+++ b/storage/falcon/Table.cpp	2009-05-26 18:11:13 +0000
@@ -2577,48 +2577,53 @@ bool Table::checkUniqueRecordVersion(int
 
 		if (!dup->hasRecord())
 			{
-			// If the record is a lock record, keep looking for a dup.
-
-			if (dup->state == recLock)
-				continue;  // Next record version.
-
-			if (dup->state == recRollback)
-				continue;  // Next record version.
-			
-			// The record has been deleted.
-			ASSERT(dup->state == recDeleted);
+			// A record without a data buffer is either a lock record or a deleted record.
+			// Checking the state is tenuous because it can be changed temporarilty
+			// (See recNoChill, recInserting, recDeleting, recEndChain)
+			// So we usualy keep looking at older record versions for duplicates.
 
 			switch (state)
 				{
 				case CommittedVisible:
 				case Us:
 					// No conflict with a visible deleted record.
-					rec->release(REC_HISTORY);
-					
-					if (activeTransState)
-						activeTransState->release();
-					
-					return false;	// Check next record number.
+					// If the state was recDeleted and is now recDeleting, it is being scavenged.
+					// So there is no harm in checking prior.  
+					// The other temporary states are not possible.
+
+					if (dup->state != recDeleted)
+						{
+						rec->release(REC_HISTORY);
+
+						if (activeTransState)
+							activeTransState->release();
+
+						return false;	// Check next record number.
+						}
+					else
+						continue;	// Next record version.
 
 				case CommittedInvisible:
 					// This state only happens for consistent read
+
 					ASSERT(IS_CONSISTENT_READ(transaction->isolationLevel));
-					foundFirstCommitted = true;
+
+					foundFirstCommitted = true;  // skip other committedInvisible
 					
 					continue;	// Next record version.
 
 				case Active:
-					// A pending transaction deleted a record.
 					// Keep looking for a possible duplicate conflict,
 					// either visible, or pending at a savepoint.
+					// We may have to come back and wait on this transaction
 
 					activeTransState = dup->getTransactionState();
 					activeTransState->addRef();
 					
-					continue;
+					continue;	// Next record version.
 
 				default:
-					continue;   // record was deleted, keep looking for a dup.
+					continue;   // Next record version, keep looking for a duplicate.
 				}
 			}
 


Attachment: [text/bzr-bundle] bzr/[e-mail]