List:Commits« Previous MessageNext Message »
From:Kevin Lewis Date:June 3 2009 8:49pm
Subject:bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2717)
Bug#43961 Bug#44223
View as plain text  
#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on revid:kevin.lewis@stripped

 2717 Kevin Lewis	2009-06-03
      Bug#43961 & Bug#44223  Both of these bugs are caused by a record that gets queued for delete while still chained up to the transaction.  Then during Transaction::rolback  (44223)   a record to rollback is queued for delete a second time.
      Or during Transaction::rollbackSavepoint  (43961), the code assumes that this record on the transaction and savepoint is still the base record since it has Record::superceded=false, and tries to call insertIntoTree to rerplace it with its prior.  But this record is not the base record either.
      
      This can happen in Table::update(Transaction * transaction, Record *orgRecord, Stream *stream);
      If this update replaces a lock record with an updated record and an exception occurred, it may or may not take the record off the transaction and the tree, but it would always queue the lock record for delete.  This code has gone through some iterations lately.  But in the most recent code, I was able to reporduce 44223. 
      
      This patch cleans it up even more by distinguishing whether a record is actually inserted into the tree and added to the transaction.  It undoes those actions only if they were done. Then if the update started with a lock record, that recordversion will remain on the tree with superceded=false and it will not be queued for delete.  It correctly uses the 'wasLock' variable instead of the 'updated' or 'wasUpdated' flag.

    modified:
      storage/falcon/Table.cpp
=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-06-03 18:15:32 +0000
+++ b/storage/falcon/Table.cpp	2009-06-03 20:49:27 +0000
@@ -1337,7 +1337,8 @@ void Table::update(Transaction * transac
 {
 	database->preUpdate();
 	RecordVersion *record = NULL;
-	bool wasUpdated = false;
+	bool insertedIntoTree = false;
+	bool addedToTransaction = false;
 	int recordNumber = oldRecord->recordNumber;
 	CycleLock cycleLock(database);
 
@@ -1396,8 +1397,9 @@ void Table::update(Transaction * transac
 		// Make insert/update atomic, then check for unique index duplicats
 
 		validateAndInsert(transaction, record);
+		insertedIntoTree = true;
 		transaction->addRecord(record);
-		wasUpdated = true;
+		addedToTransaction = true;
 		updateIndexes(transaction, record, oldRecord);
 
 		updateInversion(record, transaction);
@@ -1412,12 +1414,12 @@ void Table::update(Transaction * transac
 		}
 	catch (...)
 		{
-		if (wasUpdated)
-			{
-			transaction->removeRecord(record);
+		if (insertedIntoTree)
 			if (!insertIntoTree(oldRecord, record, recordNumber))
 				Log::debug("record backout failed after failed update\n");
-			}
+
+		if (addedToTransaction)
+			transaction->removeRecord(record);
 
 		garbageCollect(record, oldRecord, transaction, true);
 
@@ -3132,7 +3134,8 @@ void Table::update(Transaction * transac
 	RecordVersion *record = NULL;
 	Record *oldRecord;
 	bool wasLock = false;
-	bool wasUpdated = false;
+	bool insertedIntoTree = false;
+	bool addedToTransaction = false;
 
 	// Set the oldRecord, which we will attempt to update.
 
@@ -3202,11 +3205,11 @@ void Table::update(Transaction * transac
 		if (!wasLock)
 			{
 			validateAndInsert(transaction, record);
+			insertedIntoTree = true;
 			transaction->addRecord(record);
+			addedToTransaction = true;
 			}
 
-		wasUpdated = true;
-
 		// Make insert/update atomic, then check for unique index duplicats
 
 		updateIndexes(transaction, record, oldRecord);
@@ -3224,31 +3227,41 @@ void Table::update(Transaction * transac
 		}
 	catch (...)
 		{
-		if (wasUpdated)
-			{
-			transaction->removeRecord(record);
-
+		if (insertedIntoTree)
 			if (!insertIntoTree(oldRecord, record, record->recordNumber))
 				Log::debug("record backout failed after failed update\n");
-			}
+
+		if (addedToTransaction)
+			transaction->removeRecord(record);
 
 		garbageCollect(record, oldRecord, transaction, true);
+		oldRecord->release(REC_HISTORY);
 
 		if (record)
 			{
-			Record* prior = record->getPriorVersion();
-			if (prior)
-				prior->setSuperceded(false);
+			if (wasLock)
+				{
+				// Put it back to a lock record.
 
-			char* recordData = record->exchangeData((char*) (wasLock ? recordIsALock : recordDataIsFreed));
-			record->deleteData(recordData, false);
+				char* recordData = record->exchangeData((char*) recordIsALock);
+				record->deleteData(recordData, false);
 
-			SET_RECORD_ACTIVE(record, false);
-			oldRecord->release(REC_HISTORY);
-			record->state = recNormal;
-			record->queueForDelete();
+				record->state = recNormal;
+				}
+			else
+				{
+				Record* prior = record->getPriorVersion();
+				if (prior)
+					prior->setSuperceded(false);
+
+				char* recordData = record->exchangeData((char*) recordDataIsFreed);
+				record->deleteData(recordData, false);
+
+				SET_RECORD_ACTIVE(record, false);
+				record->state = recNormal;
+				record->queueForDelete();
+				}
 			}
-
 		throw;
 		}
 }


Attachment: [text/bzr-bundle] bzr/kevin.lewis@sun.com-20090603204927-azigi38qkjnol34v.bundle
Thread
bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2717)Bug#43961 Bug#44223Kevin Lewis3 Jun