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

 2715 Kevin Lewis	2009-06-02
      Bug#43344 - Reduce Falcon's dependency on Record::state since some of them are temporary states that overwrite other states.
      
      Record.cpp, Record.h, RecordVersion.cpp & RecordVersion.h
      - - - - - - - - - - - - - - -
      Replace states recDeleted and recLock with values for Record::data.record of recordIsALock (-2) and recordIsDeleted(-3).  This makes it much more deterministic to tell what kind of record this is.  Other states were found to be not needed since they were not being checked.  They include recOnDisk, recRollback, recUnlocked, recDeleting,and recPruning.  The initial state of zero previously called recData is now called recNormal.  A new value is used for data.record when the data buffer has been deleted prior to the object being scavenged.  It is called recordDataIsDeleted (-4).
      
      New functions are added to determine intrinsic record types and status including isAllocated, isALock(), findRecordData(), hasNoData(), setLock() & setDeleted().  ExchangeData is separated from deleteData() so that it can be used more flexibly.  validateData is deleted since it is not used.
      
      In several places, deleteData() is used before putting something else into the data buffer. Now, it is rearranged so that the ne data buffer is created first and the two are exhanged atomically. This continues the effort to change data.record atomically, even where it contains the non-pointer values.
      
      In the process of analyzing this code, several new ASSERTS are added.  These were tested as much as possible, but pushbuild and other DBT2 runs will need to completely verify them.
      
      
      Index.cpp, RecordScavenge.cpp, SRLUpdateRecords.cpp, StorageDatabase.cpp, Table.cpp, Transaction.cpp
      - - - - - - - - - - - - - - -
      The functions isALock() and isDeleted() are used everywhere the state was checked for recLocked and recDeleted.
      
      
      Table.cpp
      - - - - - - - - - - - - - - -
      One of the two flavors of Table::update() had two sections of code that tried to set 'oldRecord'  These were combined into more compact code and comments were added.

    modified:
      storage/falcon/Index.cpp
      storage/falcon/Record.cpp
      storage/falcon/Record.h
      storage/falcon/RecordScavenge.cpp
      storage/falcon/RecordVersion.cpp
      storage/falcon/RecordVersion.h
      storage/falcon/SRLUpdateRecords.cpp
      storage/falcon/StorageDatabase.cpp
      storage/falcon/Table.cpp
      storage/falcon/Transaction.cpp
=== modified file 'storage/falcon/Index.cpp'
--- a/storage/falcon/Index.cpp	2009-04-21 10:43:08 +0000
+++ b/storage/falcon/Index.cpp	2009-06-03 01:36:57 +0000
@@ -801,8 +801,7 @@ void Index::deleteIndex(Database *databa
 
 bool Index::changed(Record *record1, Record *record2)
 {
-	if (record2->state == recLock)
-		//return false;
+	if (record2->isALock())
 		record2 = record2->getPriorVersion();
 
 	Value value1, value2;

=== modified file 'storage/falcon/Record.cpp'
--- a/storage/falcon/Record.cpp	2009-05-20 16:58:14 +0000
+++ b/storage/falcon/Record.cpp	2009-06-03 01:36:57 +0000
@@ -68,7 +68,7 @@ Record::Record(Table *table, int32 recor
 	useCount = 0;
 	addRef(REC_HISTORY);
 	format = table->getCurrentFormat();
-	state = recData;
+	state = recNormal;
 	recordNumber = recordNum;
 	const short *p = (short*) stream->segments->address;
 	size = getSize();
@@ -118,7 +118,7 @@ Record::Record(Table * tbl, Format *fmt)
 
 	size = sizeof (RecordVersion);
 	encoding = noEncoding;
-	state = recData;
+	state = recNormal;
 	generation = format->table->database->currentGeneration;
 	SET_THIS_RECORD_ACTIVE(false);
 
@@ -188,10 +188,13 @@ void Record::setValue(TransactionState *
 	switch (encoding)
 		{
 		case noEncoding:
-			deleteData(false);
-
-			data.record = (char*) NEW Value[format->count];
+			{
+			char* newRecordData = (char*) NEW Value[format->count];
+			char* oldRecordData = exchangeData(newRecordData);
+			deleteData(oldRecordData, false);
 			encoding = valueVector;
+			}
+
 		case valueVector:
 			((Value*) data.record)[format->format[id].physicalId].setValue(value, copyFlag);
 			return;
@@ -737,8 +740,6 @@ void Record::finalize(Transaction *trans
 
 char* Record::setEncodedRecord(Stream *stream, bool interlocked)
 {
-	deleteData(false);
-
 	encoding = shortVector;
 	int vectorLength = format->count * sizeof(short);
 	int totalLength = vectorLength + stream->totalLength;
@@ -758,19 +759,20 @@ char* Record::setEncodedRecord(Stream *s
 
 		ASSERT(size == getSize() + totalLength);
 
-		// If data.record has changed since allocating the new buffer, then free the new buffer
+		// In case this is called from multiple threads thawing the same record,
+		// we will do a CAS.  The last one wins.  These buffers are held active 
+		// for threads by the CycleManager.
 
-		if (!COMPARE_EXCHANGE_POINTER(&data.record, NULL, dataBuffer))
-			{
-			DELETE_RECORD(dataBuffer);
-			totalLength = 0;
-			}
+		char* oldRecordData = exchangeData(dataBuffer);
+		deleteData(oldRecordData, false);
 		}
 	else
 		{
 		// Adjust 'size' only for non-interlocked calls.  These are new records.
 		// The allocated buffer size has not yet been added in.
 
+		ASSERT(data.record == NULL || data.record == recordIsALock);
+		ASSERT(size == getSize());
 		size += totalLength;
 		data.record = dataBuffer;
 		}
@@ -938,20 +940,35 @@ char* Record::setRecordData(const UCHAR 
 }
 
 
+char* Record::exchangeData(char* newRecordData)
+{
+	// Replace the value in data.record
+
+	char* oldRecordData;
+	for(;;)
+		{
+		oldRecordData = data.record;
+		if (COMPARE_EXCHANGE_POINTER(&data.record, oldRecordData, newRecordData))
+			break;
+		}
+
+	return oldRecordData;
+}
+
 void Record::deleteData(void)
 {
-	deleteData(false);
+	char* oldRecordData = exchangeData((char*) recordDataIsFreed);
+	deleteData(oldRecordData, false);
 }
 
 void Record::deleteData(bool now)
 {
-	// Detach the pointer from data.record
-
-	char* recordData;
-	while ((recordData = data.record))
-		if (COMPARE_EXCHANGE_POINTER(&data.record, recordData, NULL))
-			break;
+	char* oldRecordData = exchangeData((char*) recordDataIsFreed);
+	deleteData(oldRecordData, now);
+}
 
+void Record::deleteData(char* recordData, bool now)
+{
 	// Now free the detached buffer
 
 	if (isAllocated(recordData))
@@ -1015,7 +1032,7 @@ void Record::chillData(void)
 bool Record::hasRecord(void)
 {
 	char* recordData = data.record;
-	ASSERT(recordData != recordIsChilled);
+	ASSERT(isAllocated(recordData));
 	return (recordData != NULL);
 }
 
@@ -1024,9 +1041,19 @@ bool Record::isChilled(void)
 	return false;		// Only RecordVersions are ever chilled!
 }
 
+bool Record::isALock(void)
+{
+	return false;		// Only RecordVersions are used as lock records!
+}
+
 bool Record::isDeleted(void)
 {
-	return false;		// Only RecordVersions are ever chilled!
+	return false;		// Only RecordVersions are used to delete records!
+}
+
+bool Record::isRecordDataFreed(void)
+{
+	return (data.record == recordDataIsFreed);
 }
 
 void Record::print(void)
@@ -1041,11 +1068,6 @@ void Record::printRecord(const char* hea
 	print();
 }
 
-void Record::validateData(void)
-{
-	ASSERT(data.record);
-}
-
 // Allocate a record data buffer from the record cache.
 // Use an non-thread-safe increment of recordPoolAllocCount.  It allows 
 // full concurrency by multiple threads but it may miss a check every 
@@ -1117,7 +1139,7 @@ int Record::getSize(void)
 
 int Record::getDataMemUsage(void)
 {
-	char* recordData = findRecordData();
+	char* recordData = data.record;
 
 	return (isAllocated(recordData) ? MemMgr::blockSize(recordData) : 0);
 }
@@ -1219,11 +1241,28 @@ void Record::queueForDelete(void)
 
 char* Record::getRecordData(void)
 {
-	return data.record;
+	char* recordData = data.record;
+	ASSERT(isAllocated(recordData));
+	return recordData;
 }
 
 char* Record::findRecordData(void)
 {
-	return data.record;
+	char* recordData = data.record;
+	ASSERT(isAllocated(recordData));
+	return recordData;
 }
 
+bool Record::isAllocated(void)
+{
+	return isAllocated(data.record);
+}
+
+bool Record::isAllocated(char* recordData)
+{
+	return (   recordData != NULL
+	        && recordData != recordIsChilled
+	        && recordData != recordIsALock
+	        && recordData != recordIsDeleted
+	        && recordData != recordDataIsFreed);
+}

=== modified file 'storage/falcon/Record.h'
--- a/storage/falcon/Record.h	2009-05-20 16:58:14 +0000
+++ b/storage/falcon/Record.h	2009-06-03 01:36:57 +0000
@@ -41,22 +41,25 @@ 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 recNormal          =  0;	// normal record
+//static const int recDeleted       =  1;	// obsolete - record has been deleted
+//static const int recChilled       =  2;	// obsolete - record data is temporarily stored in serial log
+//static const int recOnDisk        =  3;	// obsolete - record is on disk and must be read
+//static const int recLock          =  4;	// obsolete - 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;	// obsolete - 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;	// obsolete - 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;
+static const char* recordIsChilled   = (const char*) -1;
+static const char* recordIsALock     = (const char*) -2;
+static const char* recordIsDeleted   = (const char*) -3;
+static const char* recordDataIsFreed = (const char*) -4;
 
 //#define CHECK_RECORD_ACTIVITY
 #ifdef CHECK_RECORD_ACTIVITY
@@ -126,12 +129,14 @@ public:
 	virtual const char*	getEncodedRecord();
 	virtual char*		setRecordData(const UCHAR *dataIn, int dataLength, int *bytesReallocated);
 	virtual char*		getRecordData(void);
+	virtual char*		findRecordData(void);
 	virtual void		serialize(Serialize* stream);
 	virtual int			getSize(void);
 	virtual SyncObject* getSyncThaw(void);
 	virtual void		queueForDelete(void);
 	virtual bool		hasRecord();
 	virtual bool		isChilled();
+	virtual bool		isALock();
 	virtual bool		isDeleted();
 	virtual bool		isSuperceded();
 	virtual bool		isVersion();
@@ -152,16 +157,19 @@ public:
 	bool				getRecord (Stream *stream);
 	int					getAllocatedSize(void);
 	int					getEncodedSize();
+	char*				exchangeData(char* newRecordData);
 	void				deleteData(void);
 	void				deleteData(bool now);
+	void				deleteData(char* recordData, bool now);
 	void				chillData(void);
 	void				printRecord(const char* header);
-	void				validateData(void);
 	char*				allocRecordData(int length);
 	void				expungeRecord(void);
 	int					getDataMemUsage(void);
 	int					getMemUsage(void);
-	char*				findRecordData(void);
+	bool				isRecordDataFreed();
+	bool				isAllocated(void);
+	bool				isAllocated(char* recordData);
 
 	Record (Table *table, Format *recordFormat);
 	Record (Table *table, int32 recordNumber, Stream *stream);
@@ -175,9 +183,10 @@ protected:
 		char	*record;
 		}		data;
 
-	inline bool isAllocated(char* recordData)
+	inline bool hasNoData(char* recordData)
 		{
-		return ((recordData != NULL) && (recordData != recordIsChilled));
+		ASSERT(recordData != NULL);
+		return (recordData == recordIsALock || recordData == recordIsDeleted || recordData == recordDataIsFreed);
 		}
 
 public:

=== modified file 'storage/falcon/RecordScavenge.cpp'
--- a/storage/falcon/RecordScavenge.cpp	2009-04-08 15:36:49 +0000
+++ b/storage/falcon/RecordScavenge.cpp	2009-06-03 01:36:57 +0000
@@ -187,7 +187,7 @@ Record* RecordScavenge::inventoryRecord(
 				}
 			else if (committedBeforeAnyActiveTrans)
 				{
-				if (rec->state != recLock)
+				if (rec->isALock())
 					oldestVisibleRec = rec;
 				}
 			}

=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp	2009-05-20 04:46:31 +0000
+++ b/storage/falcon/RecordVersion.cpp	2009-06-03 01:36:57 +0000
@@ -39,13 +39,13 @@
 static const char THIS_FILE[]=__FILE__;
 #endif
 
-// USE_CAS_FOR_PRIOR_VERSION ma be temporary while we look for  
+// USE_CAS_FOR_PRIOR_VERSION may be temporary while we look for  
 // problems... or not if the performance hit is unoticeable.
 // In theory, Record::priorVersion can be updated without CAS 
 // because the code is organized so that only one thread can change 
 // this at a time, even though many threads can be traversing the 
 // prior record chain simultaneously on separate CPUs.  
-// The CycleManagermakes sure that any pointer read from a priorRecord 
+// The CycleManager makes sure that any pointer read from a priorRecord 
 // remains valid while the local copy of that pointer exists.
 // These CAS exchanges protect and identify places where multiple 
 // threads change this pointer at the same time, if possible.
@@ -132,7 +132,7 @@ RecordVersion::RecordVersion(Database* d
 
 RecordVersion::~RecordVersion()
 {
-	state = recDeleting;
+//	state = recDeleting;
 	Record* prior = priorVersion;
 #ifdef USE_CAS_FOR_PRIOR_VERSION
 	if (!COMPARE_EXCHANGE_POINTER(&priorVersion, prior, NULL))
@@ -181,20 +181,19 @@ Record* RecordVersion::fetchVersion(Tran
 	// Unless the record is at least as old as the transaction, it's not for us
 
 	TransactionState* recTransState = transactionState;
+	ASSERT(recTransState);
 
-	if (state != recLock)
+	if (!isALock())
 		{
 		if (IS_READ_COMMITTED(trans->isolationLevel))
 			{
-			int state = (recTransState) ? recTransState->state : 0;
-			
-			if (state == Committed || recTransState == trans->transactionState)
-				return (getRecordData()) ? this : NULL;
+			if (recTransState->state == Committed || recTransState == trans->transactionState)
+				return (hasRecord() ? this : NULL);
 			}
 		else if (recTransState->transactionId <= trans->transactionId)
 			{
 			if (trans->visible(recTransState, FOR_READING))
-				return (getRecordData()) ? this : NULL;
+				return (hasRecord()) ? this : NULL;
 			}
 		}
 
@@ -254,7 +253,7 @@ void RecordVersion::retire(void)
 	SET_THIS_RECORD_ACTIVE(false);
 	RECORD_HISTORY(this);
 
-	if (state == recDeleted)
+	if (isDeleted())
 		expungeRecord();  // Allow this record number to be reused
 
 	release();
@@ -299,8 +298,10 @@ void RecordVersion::scavengeSavepoint(Tr
 	// Set this record's priorVersion to point past the leaving record(s)
 
 	setPriorVersion(prior, rec);
+	UCHAR prevState = ptr->state;
 	ptr->state = recEndChain;
 	format->table->garbageCollect(prior, this, targetTransaction, false);
+	ptr->state = prevState;
 	prior->queueForDelete();
 }
 
@@ -394,6 +395,8 @@ uint64 RecordVersion::getVirtualOffset()
 	return (virtualOffset);
 }
 
+// Either return the recordData buffer that was thawed, or NULL
+
 char *RecordVersion::thaw()
 {
 	// We should try without this now that data.record is manipulated safely.
@@ -402,10 +405,14 @@ char *RecordVersion::thaw()
 	syncThaw.lock(Exclusive);
 
 	int bytesReallocated = 0;
-	
-	// Nothing to do if the record is no longer chilled
-	
+
+	// Nothing to do if the record is no longer or cannot be chilled.
+
 	char* recordData = data.record;
+
+	if (hasNoData(recordData))
+		return NULL;
+
 	if (recordData != recordIsChilled)
 		return recordData;
 
@@ -460,9 +467,24 @@ bool RecordVersion::isChilled(void)
 	return (data.record == recordIsChilled);
 }
 
+bool RecordVersion::isALock(void)
+{
+	return (data.record == recordIsALock);
+}
+
+void RecordVersion::setLock(void)
+{
+	data.record = (char *) recordIsALock;
+}
+
 bool RecordVersion::isDeleted(void)
 {
-	return (data.record == NULL && state == recDeleted);
+	return (data.record == recordIsDeleted);
+}
+
+void RecordVersion::setDeleted(void)
+{
+	data.record = (char *) recordIsDeleted;
 }
 
 void RecordVersion::print(void)
@@ -517,13 +539,13 @@ Transaction* RecordVersion::findTransact
 bool RecordVersion::hasRecord(void)
 {
 	char* recordData = data.record;
-	if (recordData == NULL)
+	if (hasNoData(recordData))
 		return false;
 
-	if (recordData == recordIsChilled)
+	if (isChilled())
 		{
 		recordData = thaw();
-		ASSERT(recordData != recordIsChilled);
+		ASSERT(!isChilled());
 		return (recordData != NULL);
 		}
 
@@ -534,6 +556,9 @@ char* RecordVersion::getRecordData()
 {
 	char* recordData = data.record;
 
+	if (hasNoData(recordData))
+		return NULL;
+
 	if (recordData == recordIsChilled)
 		{
 		recordData = thaw();
@@ -542,3 +567,16 @@ char* RecordVersion::getRecordData()
 
 	return recordData;
 }
+
+// The callers of this function want to see recordIsChilled 
+// instead of actually thawing the record like getRecordData does.
+
+char* RecordVersion::findRecordData(void)
+{
+	char* recordData = data.record;
+
+	if (hasNoData(recordData))
+		return NULL;
+
+	return recordData;
+}

=== modified file 'storage/falcon/RecordVersion.h'
--- a/storage/falcon/RecordVersion.h	2009-05-12 14:35:40 +0000
+++ b/storage/falcon/RecordVersion.h	2009-06-03 01:36:57 +0000
@@ -58,8 +58,10 @@ public:
 	virtual void		serialize(Serialize* stream);
 	virtual Transaction* findTransaction(void);
 	virtual char*		getRecordData();
+	virtual char*		findRecordData();
 	virtual bool		hasRecord();
 	virtual bool		isChilled();
+	virtual bool		isALock();
 	virtual bool		isDeleted();
 	virtual bool		isSuperceded();
 	virtual bool		isVersion();
@@ -67,6 +69,8 @@ public:
 	void				commit();
 	bool				committedBefore(TransId);
 	void				setTransactionState(TransactionState* newTransState);
+	void				setLock();
+	void				setDeleted();
 
 protected:
 	virtual ~RecordVersion();

=== modified file 'storage/falcon/SRLUpdateRecords.cpp'
--- a/storage/falcon/SRLUpdateRecords.cpp	2009-05-20 04:46:31 +0000
+++ b/storage/falcon/SRLUpdateRecords.cpp	2009-06-03 01:36:57 +0000
@@ -160,7 +160,7 @@ void SRLUpdateRecords::append(Transactio
 			{
 			// Skip lock records
 
-			if (record->state == recLock)
+			if (record->isALock())
 				continue;
 
 			// Skip chilled records, but advance the chillpoint
@@ -198,7 +198,7 @@ void SRLUpdateRecords::append(Transactio
 				{
 				// Record is already in serial log
 
-				if (chillRecords && record->state != recDeleted)
+				if (chillRecords && !record->isDeleted())
 					{
 					if (chill(transaction, record))
 						{
@@ -220,7 +220,7 @@ void SRLUpdateRecords::append(Transactio
 					continue;
 				}
 			else
-				ASSERT(record->state == recDeleted);
+				ASSERT(record->isDeleted());
 			
 			// Ensure record fits within current window
 
@@ -245,7 +245,7 @@ void SRLUpdateRecords::append(Transactio
 			putInt(record->recordNumber);
 			putStream(&stream);
 			
-			if (chillRecords && record->state != recDeleted)
+			if (chillRecords && !record->isDeleted())
 				{
 				if (chill(transaction, record))
 					chilledRecordsWindow++;

=== modified file 'storage/falcon/StorageDatabase.cpp'
--- a/storage/falcon/StorageDatabase.cpp	2009-04-16 11:25:48 +0000
+++ b/storage/falcon/StorageDatabase.cpp	2009-06-03 01:36:57 +0000
@@ -672,7 +672,7 @@ int StorageDatabase::deleteRow(StorageCo
 			
 		RECORD_HISTORY(candidate);
 
-		if (candidate->state == recLock)
+		if (candidate->isALock())
 			record = candidate->getPriorVersion();
 		else if (candidate->getTransactionState() == transaction->transactionState)
 			record = candidate;

=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-06-02 15:25:59 +0000
+++ b/storage/falcon/Table.cpp	2009-06-03 01:36:57 +0000
@@ -404,7 +404,7 @@ void Table::insert(Transaction *transact
 		insertIndexes(transaction, record);
 		updateInversion(record, transaction);
 		fireTriggers(transaction, PostInsert, NULL, record);
-		record->state = recData;
+		record->state = recNormal;
 		record->release(REC_HISTORY);
 		}
 	catch (...)
@@ -1058,7 +1058,7 @@ void Table::rollbackRecord(RecordVersion
 	SET_RECORD_ACTIVE(recordToRollback, false);
 
 	int priorState = recordToRollback->state;
-	recordToRollback->state = recRollback;
+//	recordToRollback->state = recRollback;
 
 	// Find the record that will become the current version.
 
@@ -1076,7 +1076,7 @@ void Table::rollbackRecord(RecordVersion
 		{
 		// If a transaction inserts a record and then deletes it, 
 		// the  deleted record version will have no priorRecord.
-		if (priorRecord == NULL && priorState == recDeleted)
+		if (priorRecord == NULL && recordToRollback->isDeleted())
 			{
 			ASSERT(!(priorRecord == NULL && recordToRollback->isDeleted()));
 			return; // This should not be an excuse to keep going.
@@ -1424,8 +1424,8 @@ void Table::update(Transaction * transac
 			if (prior)
 				prior->setSuperceded(false);
 
-			if (record->state == recLock)
-				record->deleteData();
+			ASSERT(!record->isALock());
+			record->deleteData();
 
 			SET_RECORD_ACTIVE(record, false);
 			record->queueForDelete();
@@ -1569,7 +1569,7 @@ void Table::deleteRecord(Transaction * t
 	RecordVersion *record;
 	bool wasLock = false;
 	
-	if (candidate->state == recLock && candidate->getTransactionState() == transaction->transactionState)
+	if (candidate->isALock() && candidate->getTransactionState() == transaction->transactionState)
 		{
 		if (candidate->getSavePointId() == transaction->curSavePointId)
 			{
@@ -1596,7 +1596,7 @@ void Table::deleteRecord(Transaction * t
 		candidate->release(REC_HISTORY);
 		}
 
-	record->state = recDeleted;
+	record->setDeleted();
 
 	fireTriggers(transaction, PreDelete, orgRecord, NULL);
 
@@ -2584,17 +2584,12 @@ bool Table::checkUniqueRecordVersion(int
 
 		if (!dup->hasRecord())
 			{
-			// If the record is a lock record, keep looking for a dup.
+			// A record without a data buffer is either a lock record or a deleted record.
 
-			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);
+			if (dup->isALock() || dup->isRecordDataFreed())
+				continue;	// Next record version.
 
+			ASSERT(dup->isDeleted());  // Deleted records require special actions.
 			switch (state)
 				{
 				case CommittedVisible:
@@ -3084,7 +3079,7 @@ uint Table::insert(Transaction *transact
 		addedToTransaction = true;
 
 		insertIndexes(transaction, record);
-		record->state = recData;
+		record->state = recNormal;
 		record->release(REC_HISTORY);
 		}
 	catch (...)
@@ -3126,47 +3121,45 @@ void Table::update(Transaction * transac
 		return;
 
 	RECORD_HISTORY(candidate);
-
 	checkAncestor(candidate, orgRecord);
-	Record *oldRecord = candidate;
-
-	if (candidate->getTransactionState() == transaction->transactionState)
-		{
-		if (candidate->state == recLock)
-			oldRecord = oldRecord->getPriorVersion();
-		}
-	else
-		oldRecord = candidate->fetchVersion(transaction);
-
-	if (!oldRecord)
-		{
-		ASSERT(false);
-		candidate->release(REC_HISTORY);
-		
-		return;
-		}
 
 	RecordVersion *record = NULL;
+	Record *oldRecord;
+	bool wasLock = false;
 	bool wasUpdated = false;
 
-	if (   candidate->state == recLock 
+	// Set the oldRecord, which we will attempt to update.
+
+	if (   candidate->isALock() 
 	    && candidate->getTransactionState() == transaction->transactionState)
 		{
 		if (candidate->getSavePointId() == transaction->curSavePointId)
 			{
-			record = (RecordVersion*) candidate;	// Use the lock record for the new version.
+			// Reuse the lock record for the new version.
+
+			record = (RecordVersion*) candidate;
+			oldRecord = candidate->getPriorVersion();
 			oldRecord->addRef(REC_HISTORY);
+			wasLock = true;
 			}
-		else
+		else	// Supercede a lock record with an updated RecordVersion.
 			oldRecord = candidate;
 		}
 	else
+		{
+		// The candidate is either 1) locked by another transaction, 
+		// 2) Updated by this transaction, or 3) candidate is committed.
+		// Find our visible record version.
+
+		oldRecord = candidate->fetchVersion(transaction);
+
 		if (candidate != oldRecord)
 			{
 			oldRecord->addRef(REC_HISTORY);
 			candidate->release(REC_HISTORY);
 			}
-		
+		}
+
 	try
 		{
 		// Find current record format and create new record version
@@ -3197,10 +3190,10 @@ void Table::update(Transaction * transac
 		END_FOR;
 
 		//updateInversion(record, transaction);
-		
-		if (record->state == recLock)
-			record->state = recData;
-		else
+
+		ASSERT(record->state == recNormal);
+
+		if (!wasLock)
 			{
 			validateAndInsert(transaction, record);
 			transaction->addRecord(record);
@@ -3240,8 +3233,8 @@ void Table::update(Transaction * transac
 			if (prior)
 				prior->setSuperceded(false);
 
-			if (record->state == recLock)
-				record->deleteData();
+			char* recordData = record->exchangeData((char*) (wasLock ? recordIsALock : recordDataIsFreed));
+			record->deleteData(recordData, false);
 
 			SET_RECORD_ACTIVE(record, false);
 			oldRecord->release(REC_HISTORY);
@@ -3429,7 +3422,7 @@ void Table::unlockRecord(int recordNumbe
 
 	if (record)
 		{
-		if (record->state == recLock)
+		if (record->isALock())
 			unlockRecord((RecordVersion*) record, verbMark);
 
 		record->release(REC_HISTORY);
@@ -3438,7 +3431,7 @@ void Table::unlockRecord(int recordNumbe
 
 void Table::unlockRecord(RecordVersion* record, int verbMark)
 {
-	if (record->state != recLock)
+	if (!record->isALock())
 		return;
 
 	// A lock record that has superceded=true is already unlocked
@@ -3489,14 +3482,14 @@ Record* Table::fetchForUpdate(Transactio
 
 	if (source->getTransactionState() == transaction->transactionState)
 		{
-		if (source->state == recDeleted)
+		if (source->isDeleted())
 			{
 			source->release(REC_HISTORY);
 			
 			return NULL;
 			}
 
-		if (source->state != recLock)
+		if (!source->isALock())
 			return source;
 
 		// The source record (base record) is a lockRecord.
@@ -3540,10 +3533,10 @@ Record* Table::fetchForUpdate(Transactio
 
 			case CommittedVisible:
 				{
-				if (source->state == recLock)
+				if (source->isALock())
 					break;  // need to re-fetch the base record
 
-				if (source->state == recDeleted)
+				if (source->isDeleted())
 					{
 					source->release(REC_HISTORY);
 
@@ -3562,7 +3555,7 @@ Record* Table::fetchForUpdate(Transactio
 							source->state, source->useCount);
 
 				RecordVersion *lockRecord = allocRecordVersion(NULL, transaction, source);
-				lockRecord->state = recLock;
+				lockRecord->setLock();
 
 				if (insertIntoTree(lockRecord, source, recordNumber))
 					{

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2009-06-02 15:25:59 +0000
+++ b/storage/falcon/Transaction.cpp	2009-06-03 01:36:57 +0000
@@ -249,7 +249,7 @@ void Transaction::commit()
 		RECORD_HISTORY(record);
 		Table * table = record->format->table;
 
-		if (!record->isSuperceded() && record->state != recLock)
+		if (!record->isSuperceded() && !record->isALock())
 			{
 			table->updateRecord (record);
 			
@@ -260,7 +260,7 @@ void Transaction::commit()
 		if (!record->getPriorVersion())
 			++table->cardinality;
 			
-		if (record->state == recDeleted && table->cardinality > 0)
+		if (record->isDeleted() && table->cardinality > 0)
 			--table->cardinality;
 		}
 		
@@ -403,7 +403,7 @@ void Transaction::rollback()
 		stack = record->nextInTrans;
 		record->nextInTrans = NULL;
 
-		if (record->state == recLock)
+		if (record->isALock())
 			record->format->table->unlockRecord(record, 0);
 		else
 			record->rollback(this);
@@ -602,9 +602,9 @@ void Transaction::addRecord(RecordVersio
 
 	hasUpdates = true;
 
-	if (record->state == recLock)
+	if (record->isALock())
 		hasLocks = true;
-	else if (record->state == recDeleted)
+	else if (record->isDeleted())
 		++deletedRecords;
 		
 	totalRecordData += record->getAllocatedSize();
@@ -622,7 +622,7 @@ void Transaction::addRecord(RecordVersio
 		// which may be part of an update or insert
 
 		UCHAR saveState = record->state;
-		if (record->state != recLock)
+		if (!record->isALock())
 			record->state = recNoChill;
 
 		chillRecords();
@@ -704,7 +704,7 @@ void Transaction::removeRecordNoLock(Rec
 	if (totalRecordData >= allocatedSize)
 		totalRecordData -= allocatedSize;
 
-	if (record->state == recDeleted && deletedRecords > 0)
+	if (record->isDeleted() && deletedRecords > 0)
 		--deletedRecords;
 	
 	record->release(REC_HISTORY);
@@ -794,7 +794,7 @@ bool Transaction::needToLock(Record* rec
 
 		if (visible(transState, FOR_WRITING))
 			{
-			if (candidate->state == recDeleted)
+			if (candidate->isDeleted())
 				{
 				if (transState->state == Committed)
 					return false; // Committed and deleted
@@ -1291,7 +1291,7 @@ void Transaction::rollbackSavepoint(int 
 			rec->nextInTrans = stack;
 			stack = rec;
 
-			if (rec->state == recDeleted)
+			if (rec->isDeleted())
 				--deletedRecords;
 			}
 
@@ -1353,7 +1353,7 @@ void Transaction::releaseRecordLocks(voi
 	syncRec.lock(Exclusive);
 
 	for (RecordVersion *record = firstRecord; record; record = record->nextInTrans)
-		if (record->state == recLock)
+		if (record->isALock())
 			{
 			record->format->table->unlockRecord(record, 0);
 
@@ -1384,9 +1384,9 @@ void Transaction::printBlocking(int leve
 	syncRec.lock(Shared);
 
 	for (record = firstRecord; record; record = record->nextInTrans)
-		if (record->state == recLock)
+		if (record->isALock())
 			++locks;
-		else if (!record->hasRecord())
+		else if (record->isDeleted())
 			++deletes;
 		else if (record->getPriorVersion())
 			++updates;
@@ -1412,9 +1412,9 @@ void Transaction::printBlocking(int leve
 		{
 		const char *what;
 
-		if (record->state == recLock)
+		if (record->isALock())
 			what = "locked";
-		else if (!record->hasRecord())
+		else if (record->isDeleted())
 			what = "deleted";
 		else if (record->getPriorVersion())
 			what = "updated";


Attachment: [text/bzr-bundle] bzr/kevin.lewis@sun.com-20090603013657-nq7gkncnwsbqq5lu.bundle
Thread
bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2715)Bug#43344Kevin Lewis3 Jun
  • Re: bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2715)Bug#43344Olav Sandstaa3 Jun
    • Re: bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2715)Bug#43344Kevin Lewis3 Jun