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

[Christopher Powers]

 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

 2904 Vladislav Vaintroub	2008-11-05
       Bug #40302 error on tablespace creation is not handled cleanly 
            
      Problem: If tablespace.create() fails, system transaction that updates tablespace
      info is not rolled back. It can be possibly commited with the next system
transaction,
      that will add a tablespace to system tables even if datafile does not exist.
            
      Additionally, createTableSpace log record (indicating creation of datafile)  that
logically 
      belongs to the same transaction as  system tables and has the same transaction
      id was wrongly written after "commit".
            
      With this patch, 
       -  there is a  rollback in case of error and appropriate cleanup in the filesystem
       -  createTableSpace now belongs to the same transaction that changes system tables.
modified:
  storage/falcon/TableSpaceManager.cpp

=== 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;