bzr commit into mysql-6.0-falcon-team branch (cpowers:2905) Bug#39696

[Christopher Powers]

#At file:///home/cpowers/work/dev/dev-11/mysql/

 2905 Christopher Powers	2008-11-06
      Bug#39696, "Assertion in Table.cpp (dup->state == recDeleted) fails during
falcon_chill_thaw"
      
      Fixed problems with savepoint rollbacks that prevented some chilled records from
being committed.
modified:
  storage/falcon/RecordVersion.cpp
  storage/falcon/SRLUpdateRecords.cpp
  storage/falcon/Transaction.cpp

per-file messages:
  storage/falcon/SRLUpdateRecords.cpp
    Thawed records are no longer redundantly appended to the serial log.
    Added comments
  storage/falcon/Transaction.cpp
    Transaction::rollbackSavepoint()
    - SRLSavepointRollback records were incorrectly appended to the serial log for
savepoints that were not rolled back
    - Rolled back savepoints were being re-used, thus preventing some chilled records from
being committed
    - Added comments
=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp	2008-10-29 23:25:13 +0000
+++ b/storage/falcon/RecordVersion.cpp	2008-11-07 01:09:04 +0000
@@ -55,6 +55,9 @@ RecordVersion::RecordVersion(Table *tbl,
 		{
 		priorVersion->addRef();
 		recordNumber = oldVersion->recordNumber;
+
+		if (priorVersion->state == recChilled)
+			priorVersion->thaw();
 		
 		if (trans == priorVersion->getTransaction())
 			oldVersion->setSuperceded (true);

=== modified file 'storage/falcon/SRLUpdateRecords.cpp'
--- a/storage/falcon/SRLUpdateRecords.cpp	2008-10-29 23:25:13 +0000
+++ b/storage/falcon/SRLUpdateRecords.cpp	2008-11-07 01:09:04 +0000
@@ -128,6 +128,11 @@ void SRLUpdateRecords::append(Transactio
 	SerialLogTransaction *srlTrans = NULL;
 	int savepointId;
 	
+	// Generate one serial log record per write window. To ensure that
+	// record updates are grouped by savepoint, start a new serial
+	// log record for each savepoint. Several serial log records may
+	// be generated for one savepoint.
+	
 	for (RecordVersion *record = records; record;)
 		{
 		START_RECORD(srlUpdateRecords, "SRLUpdateRecords::append");
@@ -169,7 +174,8 @@ void SRLUpdateRecords::append(Transactio
 			if (record->state == recNoChill)
 				continue;
 
-			// If this is a different savepoint, start another record
+			// If this is a different savepoint, break out of the inner loop
+			// and start another serial log record
 			
 			if (chillRecords && record->savePointId != savepointId)
 				break;
@@ -178,20 +184,34 @@ void SRLUpdateRecords::append(Transactio
 			tableSpaceId = table->dbb->tableSpaceId;
 			Stream stream;
 			
-			// Thawed records are indicated by a non-zero virtual offset, and
-			// are already in the serial log. If this record is to be re-chilled,
-			// then no need to get the record data or set the virtual offset.
+			// A non-zero virtual offset indicates that the record was previously
+			// chilled and thawed and is already in the serial log.
+			//
+			// If this record is being re-chilled, then there is no need to get
+			// the record data or set the virtual offset.
+			//
+			// If this record is being committed, then there is nothing to do.
 
-			if (chillRecords && record->state != recDeleted &&
record->virtualOffset != 0)
+			if (record->virtualOffset != 0)
 				{
-				int chillBytes = record->getEncodedSize();
-				chill(transaction, record, chillBytes);
-				log->chilledRecords++;
-				log->chilledBytes += chillBytes;
-				ASSERT(transaction->thawedRecords > 0);
+				if (chillRecords && record->state != recDeleted)
+					{
+					int chillBytes = record->getEncodedSize();
 
-				if (transaction->thawedRecords)
-					transaction->thawedRecords--;
+					chill(transaction, record, chillBytes);
+					
+					log->chilledRecords++;
+					log->chilledBytes += chillBytes;
+					
+					ASSERT(transaction->thawedRecords > 0);
+
+					if (transaction->thawedRecords)
+						transaction->thawedRecords--;
+					}
+				else
+					{
+					// Record is already in serial log
+					}
 
 				continue;
 				}
@@ -219,9 +239,11 @@ void SRLUpdateRecords::append(Transactio
 
 			ASSERT(record->recordNumber >= 0);
 			ASSERT(log->writePtr > (UCHAR *)log->writeWindow->buffer);
+			
 			record->setVirtualOffset(log->writeWindow->currentLength +
log->writeWindow->virtualOffset);
 			uint32 sectionId = table->dataSectionId;
 			log->updateSectionUseVector(sectionId, tableSpaceId, 1);
+			
 			putInt(tableSpaceId);
 			putInt(record->getPriorVersion() ? sectionId : -(int) sectionId - 1);
 			putInt(record->recordNumber);
@@ -233,7 +255,7 @@ void SRLUpdateRecords::append(Transactio
 				chilledRecordsWindow++;
 				chilledBytesWindow += stream.totalLength;
 				}
-			} // next record
+			} // next record version
 		
 		int len = (int) (log->writePtr - start);
 		
@@ -253,7 +275,7 @@ void SRLUpdateRecords::append(Transactio
 			transaction->chilledBytes += chilledBytesWindow;
 			windowNumber = (uint32)log->writeWindow->virtualOffset / SRL_WINDOW_SIZE;
 			}
-		} // next window
+		} // next serial log record and write window
 }
 
 void SRLUpdateRecords::read(void)

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2008-11-03 00:33:04 +0000
+++ b/storage/falcon/Transaction.cpp	2008-11-07 01:09:04 +0000
@@ -538,8 +538,19 @@ void Transaction::chillRecords()
 		
 	uint32 chilledBefore = chilledRecords;
 	uint64 totalDataBefore = totalRecordData;
+	
 	database->dbb->logUpdatedRecords(this, *chillPoint, true);
 	
+	// At the start of a chill operation, all savepoints are updated with the id of the
+	// savepoint being chilled. This ensures that each savepoint in a transaction always
+	// has a bitmap of savepoints that were chilled after it.
+
+	// When a savepoint is rolled back, those newer savepoints for which records have also
+	// been chilled are recorded in the serial log.
+
+	// The idea is that if savepoint N is rolled back, then chilled records attached to
+	// savepoints >= N	are ignored and not committed to the database.
+	
 	for (SavePoint *savePoint = savePoints; savePoint; savePoint = savePoint->next)
 		if (savePoint->id != curSavePointId)
 			savePoint->setIncludedSavepoint(curSavePointId);
@@ -1100,6 +1111,8 @@ int Transaction::createSavepoint()
 	else
 		savePoint = new SavePoint;
 	
+	// The savepoint begins with the next record added to the transaction
+	
 	savePoint->records = (lastRecord) ? &lastRecord->nextInTrans :
&firstRecord;
 	savePoint->id = ++curSavePointId;
 	savePoint->next = savePoints;
@@ -1124,10 +1137,13 @@ void Transaction::releaseSavepoint(int s
 	for (SavePoint **ptr = &savePoints, *savePoint; (savePoint = *ptr); ptr =
&savePoint->next)
 		if (savePoint->id == savePointId)
 			{
+			
+			// Savepoints are linked in descending order, so the next lower id is next on the list
+			
 			int nextLowerSavePointId = (savePoint->next) ? savePoint->next->id : 0;
 			*ptr = savePoint->next;
 			
-			// If we have backed logged records, merge them in to the previous savepoint or the
transaction itself.
+			// If we have backed logged records, merge them in to the previous savepoint or the
transaction itself
 			
 			if (savePoint->backloggedRecords)
 				{
@@ -1155,7 +1171,8 @@ void Transaction::releaseSavepoint(int s
 			if (savePoint->savepoints)
 				savePoint->clear();
 
-			// commit pending record versions to the next pending savepoint
+			// This savepoint is no longer needed, so commit pending record versions to the next
pending savepoint
+			// Scavenge prior record versions having 1) the same transaction and 2) savepoint
>= the savepoint being released
 			
 			for (RecordVersion *record = *savePoint->records; record &&
record->savePointId == savePointId; record = record->nextInTrans)
 				{
@@ -1225,12 +1242,19 @@ void Transaction::rollbackSavepoint(int 
 	if ((savePoint) && (savePoint->id != savePointId))
 		throw SQLError(RUNTIME_ERROR, "invalid savepoint");
 
+	// Records within this savepoint or later may have been chilled and are
+	// already in the serial log, but they are now obsolete. To ensure that those
+	// records are not committed to the database, append the serial log with a
SRLSavepointRollback
+	// record for this savepoint and for any greater savepoint that has been chilled.
+	
 	if (chilledRecords)
 		{
 		database->serialLog->logControl->savepointRollback.append(transactionId,
savePointId);
 		
+		// SavePoint::savepoints is a bitmap of other savepoints that have been chilled
+		
 		if (savePoint->savepoints)
-			for (int n = 0; (n = savePoint->savepoints->nextSet(n)) >= 0; ++n)
+			for (int n = savePointId; (n = savePoint->savepoints->nextSet(n)) >=
savePointId; ++n)
 				database->serialLog->logControl->savepointRollback.append(transactionId, n);
 		}				
 
@@ -1292,10 +1316,9 @@ void Transaction::rollbackSavepoint(int 
 		if (savePoint->backloggedRecords)
 			database->backLog->rollbackRecords(savePoint->backloggedRecords, this);
 				
-		// Move skipped savepoints object to the free list
-		// Leave the target savepoint empty, but connected to the transaction.
+		// Move skipped savepoint objects to the free list
 		
-		if (savePoint->id > savePointId)
+		if (savePoint->id >= savePointId)
 			{
 			savePoints = savePoint->next;
 			savePoint->next = freeSavePoints;