List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:March 23 2009 2:57pm
Subject:bzr commit into mysql-6.0-falcon-team branch (olav:3069) Bug#41357
View as plain text  
#At file:///home/os136802/mysql/develop/repo/falcon-bug41357/ based on revid:hky@stripped

 3069 Olav Sandstaa	2009-03-23
      Fix for Bug#41357 falcon.falcon_bug_34351_C fails with assertion IS_CONSISTENT_READ
      and potentially other related bugs.
      
      This assert was hit due to Transaction::getRelativeState() returning a
      wrong value. The cause was that the RecordVersion object's transaction
      pointer was refering to a deleted Transaction object.
      
      This patch fixes this problem by:
      
      1. Introduces a new class called TransactionState. The following
         member variables in the Transaction class has been moved to this
         new class:
       
          transactionId;       // used also as startEvent by dep.mgr.
      	commitId;            // used as commitEvent by dep.mgr.
      	state;
      	syncIsActive;
      	waitingFor;          // Used for deadlock detection
      
         Compared to the Transaction object which can be purged when its commit
         id is older than the transaction id of the oldest active
         transaction, the new TransactionState object will be available as
         long as there exits record versions refering to it. Reference
         counting is used for ensuring that transaction state object are not
         deleted while there are other object needing to acces itt.
      
      2. Rewrites Transaction::getRelativeState(), Transaction::visible(),
         Transaction::needToLock() and Transaction::waitForTransaction()
         only use the transaction state object instead of accessing
         potentially deleted transaction objects.
     @ storage/falcon/Connection.cpp
        Access Transaction's state using member function instead of direct access to member variable
     @ storage/falcon/Database.cpp
        Access Transaction's state using member function instead of direct access to member variable
     @ storage/falcon/Makefile.am
        Added new files: TransactionState.h and .cpp
     @ storage/falcon/Record.cpp
        Added method for retrieving transaction state.
     @ storage/falcon/Record.h
        Added method for retrieving transaction state.
     @ storage/falcon/RecordVersion.cpp
        Added transaction state object.
     @ storage/falcon/RecordVersion.h
        Added transaction state object.
     @ storage/falcon/Table.cpp
        Use transaction state instead of transaction pointer when calling Transaction::getReletiveState
     @ storage/falcon/Transaction.cpp
        Moved transaction state from Transaction class to TransactionState class and converted main methods to use
        the TransactionState object instead of Transaction object when handling relationship to other transactions.
     @ storage/falcon/Transaction.h
        Moved transaction state from Transaction class to TransactionState class.
     @ storage/falcon/TransactionManager.cpp
        Introduce TransactionState in separate object.
     @ storage/falcon/TransactionState.cpp
        The transaction state for a Transaction. The members have been moved from the Transaction class.
     @ storage/falcon/TransactionState.h
        The transaction state for a Transaction. The members have been moved from the Transaction class.

    added:
      storage/falcon/TransactionState.cpp
      storage/falcon/TransactionState.h
    modified:
      storage/falcon/Connection.cpp
      storage/falcon/Database.cpp
      storage/falcon/Makefile.am
      storage/falcon/Record.cpp
      storage/falcon/Record.h
      storage/falcon/RecordVersion.cpp
      storage/falcon/RecordVersion.h
      storage/falcon/Table.cpp
      storage/falcon/Transaction.cpp
      storage/falcon/Transaction.h
      storage/falcon/TransactionManager.cpp
=== modified file 'storage/falcon/Connection.cpp'
--- a/storage/falcon/Connection.cpp	2009-02-23 22:42:36 +0000
+++ b/storage/falcon/Connection.cpp	2009-03-23 14:57:48 +0000
@@ -1718,7 +1718,7 @@ void Connection::detachDatabase()
 
 	try
 		{
-		if (transaction && transaction->state == Active)
+		if (transaction && transaction->getState() == Active)
 			transaction->rollback();
 		}
 	catch (SQLException &exception)

=== modified file 'storage/falcon/Database.cpp'
--- a/storage/falcon/Database.cpp	2009-03-09 12:12:14 +0000
+++ b/storage/falcon/Database.cpp	2009-03-23 14:57:48 +0000
@@ -1611,7 +1611,7 @@ void Database::shutdown()
 	
 	if (systemConnection && 
 		systemConnection->transaction && 
-		systemConnection->transaction->state == Active)
+		systemConnection->transaction->getState() == Active)
 		systemConnection->commit();
 		
 	if (repositoryManager)

=== modified file 'storage/falcon/Makefile.am'
--- a/storage/falcon/Makefile.am	2009-03-10 12:37:26 +0000
+++ b/storage/falcon/Makefile.am	2009-03-23 14:57:48 +0000
@@ -205,6 +205,7 @@ falcon_headers= Agent.h Alias.h Applicat
 		TimeStamp.h \
 		Transaction.h \
 		TransactionManager.h \
+		TransactionState.h \
 		Trigger.h \
 		TriggerRecord.h \
 		Types.h Unicode.h UnTable.h User.h UserRole.h \
@@ -389,6 +390,7 @@ falcon_sources= Agent.cpp Alias.cpp \
 		TimeStamp.cpp \
 		Transaction.cpp \
 		TransactionManager.cpp \
+		TransactionState.cpp \
 		Trigger.cpp \
 		Unicode.cpp UnTable.cpp User.cpp \
 		UserRole.cpp Validation.cpp Value.cpp ValueEx.cpp Values.cpp \

=== modified file 'storage/falcon/Record.cpp'
--- a/storage/falcon/Record.cpp	2009-03-07 01:37:19 +0000
+++ b/storage/falcon/Record.cpp	2009-03-23 14:57:48 +0000
@@ -536,6 +536,11 @@ Transaction* Record::getTransaction()
 	return NULL;
 }
 
+TransactionState* Record::getTransactionState() const
+{
+	return NULL;
+}
+
 bool Record::isSuperceded()
 {
 	return false;

=== modified file 'storage/falcon/Record.h'
--- a/storage/falcon/Record.h	2009-02-26 20:04:31 +0000
+++ b/storage/falcon/Record.h	2009-03-23 14:57:48 +0000
@@ -89,6 +89,7 @@ static const int recEndChain = 11;		// e
 class Format;
 class Table;
 class Transaction;
+class TransactionState;
 class Value;
 class Stream;
 class Database;
@@ -104,6 +105,7 @@ class Record
 {
 public:
 	virtual Transaction* getTransaction();
+	virtual TransactionState* getTransactionState() const;
 	virtual TransId	getTransactionId();
 	virtual int		getSavePointId();
 	virtual void	setSuperceded (bool flag);

=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp	2009-03-03 07:09:29 +0000
+++ b/storage/falcon/RecordVersion.cpp	2009-03-23 14:57:48 +0000
@@ -23,6 +23,7 @@
 #include "Configuration.h"
 #include "RecordVersion.h"
 #include "Transaction.h"
+#include "TransactionState.h"
 #include "TransactionManager.h"
 #include "Table.h"
 #include "Connection.h"
@@ -47,10 +48,16 @@ RecordVersion::RecordVersion(Table *tbl,
 {
 	virtualOffset = 0;
 	transaction   = trans;
+	transState    = trans->transactionState;
 	transactionId = transaction->transactionId;
 	savePointId   = transaction->curSavePointId;
 	superceded    = false;
 
+	// Add a use count on the transaction state to ensure it lives as long 
+	// as the record version object
+
+	transState->addRef();
+
 	if ((priorVersion = oldVersion))
 		{
 		priorVersion->addRef();
@@ -66,7 +73,7 @@ RecordVersion::RecordVersion(Table *tbl,
 		recordNumber = -1;
 }
 
-RecordVersion::RecordVersion(Database* database, Serialize *stream) : Record(database, stream)
+RecordVersion::RecordVersion(Database* database, Serialize *stream) : Record(database, stream), transState(NULL)
 {
 	// Reconstitute a record version and recursively restore all
 	// prior versions from 'stream'
@@ -104,6 +111,11 @@ RecordVersion::~RecordVersion()
 	
 	while (prior)
 		prior = prior->releaseNonRecursive();
+
+	// Release the use count on the transaction state object
+
+	if (transState)
+		transState->release();
 }
 
 // Release the priorRecord reference without doing it recursively.
@@ -143,7 +155,7 @@ Record* RecordVersion::fetchVersionRecur
 		{
 		if (IS_READ_COMMITTED(trans->isolationLevel))
 			{
-			int state = (recTransaction) ? recTransaction->state : 0;
+			int state = (recTransaction) ? recTransaction->transactionState->state : 0;
 			
 			if (!transaction || state == Committed || recTransaction == trans)
 				return (getRecordData()) ? this : NULL;
@@ -295,6 +307,11 @@ Transaction* RecordVersion::getTransacti
 	return transaction;
 }
 
+TransactionState* RecordVersion::getTransactionState() const
+{
+	return transState;
+}
+
 bool RecordVersion::isSuperceded()
 {
 	return superceded;

=== modified file 'storage/falcon/RecordVersion.h'
--- a/storage/falcon/RecordVersion.h	2009-01-14 22:33:44 +0000
+++ b/storage/falcon/RecordVersion.h	2009-03-23 14:57:48 +0000
@@ -27,6 +27,7 @@
 #include "Record.h"
 
 class Transaction;
+class TransactionState;
 class SyncObject;
 
 class RecordVersion : public Record  
@@ -37,6 +38,7 @@ public:
 
 	virtual bool		isSuperceded();
 	virtual Transaction* getTransaction();
+	virtual TransactionState* getTransactionState() const;
 	virtual TransId		getTransactionId();
 	virtual int			getSavePointId();
 	virtual void		setSuperceded (bool flag);
@@ -73,6 +75,9 @@ public:
 	TransId			transactionId;
 	int				savePointId;
 	bool			superceded;
+
+private:
+	TransactionState *transState;
 };
 
 #endif // !defined(AFX_RECORDVERSION_H__84FD1965_A97F_11D2_AB5C_0000C01D2301__INCLUDED_)

=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-03-09 12:12:14 +0000
+++ b/storage/falcon/Table.cpp	2009-03-23 14:57:48 +0000
@@ -2661,7 +2661,7 @@ bool Table::checkUniqueRecordVersion(int
 				syncPrior.unlock(); // release lock before wait
 				syncUnique->unlock(); // release lock before wait
 
-				state = transaction->getRelativeState(activeTransaction,
+				state = transaction->getRelativeState(activeTransaction->transactionState,
 						activeTransaction->transactionId, WAIT_IF_ACTIVE);
 
 				if (state != Deadlock)
@@ -3768,7 +3768,7 @@ bool Table::validateUpdate(int32 recordN
 		
 		Transaction *transaction = record->getTransaction();
 		
-		if (!transaction || transaction->state == Committed)
+		if (!transaction || transaction->getState() == Committed)
 			{
 			SET_RECORD_ACTIVE(record, false);
 			record->release(REC_HISTORY);

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2009-03-05 09:20:35 +0000
+++ b/storage/falcon/Transaction.cpp	2009-03-23 14:57:48 +0000
@@ -20,6 +20,7 @@
 #include <memory.h>
 #include "Engine.h"
 #include "Transaction.h"
+#include "TransactionState.h"
 #include "Configuration.h"
 #include "Database.h"
 #include "Dbb.h"
@@ -89,12 +90,12 @@ Transaction::Transaction(Connection *cnc
 	freeSavePoints = NULL;
 	useCount = 1;
 	syncObject.setName("Transaction::syncObject");
-	syncIsActive.setName("Transaction::syncActive");
 	syncDeferredIndexes.setName("Transaction::syncDeferredIndexes");
 	syncRecords.setName("Transaction::syncRecords");
 	syncSavepoints.setName("Transaction::syncSavepoints");
 	firstRecord = NULL;
 	lastRecord = NULL;
+	transactionState = new TransactionState();
 	initialize(cnct, seq);
 }
 
@@ -112,14 +113,14 @@ void Transaction::initialize(Connection*
 	transactionManager = database->transactionManager;
 	systemTransaction = database->systemConnection == connection;
 	transactionId = seq;
-	commitId = 0;
+	transactionState->transactionId = seq;
+	transactionState->commitId = 0;
 	chillPoint = &firstRecord;
 	commitTriggers = false;
 	hasUpdates = false;
 	hasLocks = false;
 	writePending = true;
 	pendingPageWrites = false;
-	waitingFor = NULL;
 	curSavePointId = 0;
 	deferredIndexes = NULL;
 	backloggedRecords = NULL;
@@ -143,7 +144,7 @@ void Transaction::initialize(Connection*
 	
 	if (seq == 0)
 		{
-		state = Available;
+		transactionState->state = Available;
 		systemTransaction = false;
 		writePending = false;
 
@@ -159,21 +160,21 @@ void Transaction::initialize(Connection*
 	startTime = database->deltaTime;
 	blockingRecord = NULL;
 	thread = Thread::getThread("Transaction::initialize");
-	syncIsActive.lock(NULL, Exclusive);
-	state = Active;
+	transactionState->syncIsActive.lock(NULL, Exclusive);
+	transactionState->state = Active;
 }
 
 Transaction::~Transaction()
 {
 	Tdelete++;
 
-	if (state == Active)
+	if (transactionState->state == Active)
 		{
 		Log::debug("Deleting apparently active transaction %d\n", transactionId);
 		ASSERT(false);
 		
-		if (syncIsActive.ourExclusiveLock())
-			syncIsActive.unlock();
+		if (transactionState->syncIsActive.ourExclusiveLock())
+			transactionState->syncIsActive.unlock();
 		}
 
 	if (inList)
@@ -196,6 +197,9 @@ Transaction::~Transaction()
 	
 	if (deferredIndexes)
 		releaseDeferredIndexes();
+
+	if (transactionState)
+		transactionState->release();
 }
 
 void Transaction::commit()
@@ -217,7 +221,7 @@ void Transaction::commit()
 	Log::log(LogXARecovery, "%d: Commit %sTransaction %d\n", 
 		database->deltaTime, (systemTransaction ? "System " : ""),  transactionId);
 
-	if (state == Active)
+	if (transactionState->state == Active)
 		{
 		Sync sync(&syncDeferredIndexes, "Transaction::commit(1)");
 		sync.lock(Shared);
@@ -279,11 +283,11 @@ void Transaction::commit()
 
 	// Set the commit transition id for this transaction
 
-	commitId = INTERLOCKED_INCREMENT(transactionManager->transactionSequence);
+	transactionState->commitId = INTERLOCKED_INCREMENT(transactionManager->transactionSequence);
 
 	transactionManager->activeTransactions.remove(this);
 	transactionManager->committedTransactions.append(this);
-	state = Committed;
+	transactionState->state = Committed;
 
 	// This is one of the few points where we have an exclusive lock on both the
 	// active and committed transaction list. Although this has nothing to do
@@ -298,7 +302,7 @@ void Transaction::commit()
 	syncCommitted.unlock();
 	syncActiveTransactions.unlock();
 	
-	syncIsActive.unlock(); // signal waiting transactions
+	transactionState->syncIsActive.unlock(); // signal waiting transactions
 	
 	// signal a gopher to start processing this transaction
 
@@ -350,10 +354,11 @@ void Transaction::commitNoUpdates(void)
 	
 	connection = NULL;
 	transactionId = 0;
+	transactionState->transactionId = 0;
 	writePending = false;
-	state = Available;
+	transactionState->state = Available;
 	syncActiveTransactions.unlock();
-	syncIsActive.unlock();
+	transactionState->syncIsActive.unlock();
 	release();
 }
 
@@ -445,8 +450,8 @@ void Transaction::rollback()
 	inList = false;
 	transactionManager->activeTransactions.remove(this);
 	syncActiveTransactions.unlock();
-	state = RolledBack;
-	syncIsActive.unlock();
+	transactionState->state = RolledBack;
+	transactionState->syncIsActive.unlock();
 
 	// Finish the SerialLogTransaction and signal a gopher
 
@@ -463,7 +468,7 @@ void Transaction::rollback()
 
 void Transaction::prepare(int xidLen, const UCHAR *xidPtr)
 {
-	if (state != Active)
+	if (transactionState->state != Active)
 		throw SQLEXCEPTION (RUNTIME_ERROR, "transaction is not active");
 
 	Log::log(LogXARecovery, "Prepare transaction %d: xidLen = %d\n", transactionId, xidLen);
@@ -478,7 +483,7 @@ void Transaction::prepare(int xidLen, co
 		}
 		
 	database->pageWriter->waitForWrites(this);
-	state = Limbo;
+	transactionState->state = Limbo;
 
 	// Flush a prepare record to the serial log
 
@@ -591,8 +596,9 @@ void Transaction::thaw(DeferredIndex * d
 void Transaction::addRecord(RecordVersion * record)
 {
 	ASSERT(record->recordNumber >= 0);
+
 	hasUpdates = true;
-	
+
 	if (record->state == recLock)
 		hasLocks = true;
 	else if (record->state == recDeleted)
@@ -715,19 +721,34 @@ bool Transaction::visible(Transaction * 
 	if (!transaction)
 		return true;
 
+    return visible(transaction->transactionState, forWhat);
+}
+
+
+/***
+@brief		Determine if changes by another transaction are visible to this.
+@details	This function is called for Consistent-Read transactions to determine
+			if the sent trans was committed before this transaction started.  If not,
+			it is invisible to this transaction.
+***/
+
+bool Transaction::visible(const TransactionState* transState, int forWhat) const
+{
+	ASSERT(transState != NULL);
+
 	// If we're the transaction in question, consider us committed
 
-	if (transId == transactionId)
+	if (transactionState == transState)
 		return true;
 
 	// If we're the system transaction, just use the state of the other transaction
 
 	if (database->systemConnection->transaction == this)
-		return transaction->state == Committed;
+		return transState->state == Committed;
 
 	// If the other transaction is not yet committed, the trans is not visible.
 
-	if (transaction->state != Committed)
+	if (transState->state != Committed)
 		return false;
 
 	// The other transaction is committed.  
@@ -744,12 +765,13 @@ bool Transaction::visible(Transaction * 
 	// If the other transaction committed after we started then it is not
 	// be visible to us
 
-	if (transaction->commitId > transactionId)
+	if (transState->commitId > transactionId)
 		return false;
 
 	return true;
 }
 
+
 /***
 @brief		Determine if there is a need to lock this record for update.
 ***/
@@ -765,12 +787,12 @@ bool Transaction::needToLock(Record* rec
 		 candidate != NULL;
 		 candidate = candidate->getPriorVersion())
 		{
-		Transaction *transaction = candidate->getTransaction();
-		TransId transId = candidate->getTransactionId();
+		TransactionState* transState = candidate->getTransactionState();
+		ASSERT(transState != NULL);
 
-		if (visible(transaction, transId, FOR_WRITING))
+		if (visible(transState, FOR_WRITING))
 			if (candidate->state == recDeleted)
-				if (!transaction || transaction->state == Committed)
+				if (transState->state == Committed)
 					return false; // Committed and deleted
 				else
 					return true; // Just in case this rolls back.
@@ -832,24 +854,30 @@ State Transaction::getRelativeState(Reco
 		}
 
 	blockingRecord = record;
-	State state = getRelativeState(record->getTransaction(), record->getTransactionId(), flags);
+	State state = getRelativeState(record->getTransactionState(), record->getTransactionId(), flags);
 	blockingRecord = NULL;
 
 	return state;
 }
 
+
 /***
 @brief		Get the relative state between this transaction and another.
 ***/
 
-State Transaction::getRelativeState(Transaction *transaction, TransId transId, uint32 flags)
+State Transaction::getRelativeState(TransactionState* transState, TransId transId, uint32 flags)
 {
+	// Note: The structure of this method is still based on the
+	// original getRelativeState() - should be carefully rewritten
+
 	if (transactionId == transId)
 		return Us;
 
-	// A record may still have the transId even after the trans itself has been deleted.
-	
-	if (!transaction)
+	// The following if test replaces the original if (!transaction)
+	// This could be improved by combining it with the last if test
+	// at the end of this function.
+
+	if (transState->state == Committed && transState->commitId < transactionId)
 		{
 		// All calls to getRelativeState are for the purpose of writing.
 		// So only ConsistentRead can get CommittedInvisible.
@@ -869,13 +897,13 @@ State Transaction::getRelativeState(Tran
 		return CommittedVisible;
 		}
 
-	if (transaction->isActive())
+	if (transState->isActive())
 		{
 		if (flags & DO_NOT_WAIT)
 			return Active;
 
 		bool isDeadlock;
-		waitForTransaction(transaction, 0 , &isDeadlock);
+		waitForTransaction(transState, 0 , &isDeadlock);
 
 		if (isDeadlock)
 			return Deadlock;
@@ -883,20 +911,21 @@ State Transaction::getRelativeState(Tran
 		return WasActive;			// caller will need to re-fetch
 		}
 
-	if (transaction->state == Committed)
+	if (transState->state == Committed)
 		{
 		// Return CommittedVisible if the other trans has a lower TransId and 
 		// it was committed when we started.
 		
-		if (visible (transaction, transId, FOR_WRITING))
+		if (visible (transState, FOR_WRITING))
 			return CommittedVisible;
 
 		return CommittedInvisible;
 		}
 
-	return (State) transaction->state;
+	return (State) transState->state;
 }
 
+
 void Transaction::dropTable(Table* table)
 {
 	releaseDeferredIndexes(table);
@@ -942,7 +971,7 @@ bool Transaction::hasRecords(Table* tabl
 void Transaction::writeComplete(void)
 {
 	ASSERT(writePending);
-	ASSERT(state == Committed);
+	ASSERT(transactionState->state == Committed);
 	releaseDeferredIndexes();
 
 //	Log::log(LogXARecovery, "%d: WriteComplete %sTransaction %d\n", 
@@ -959,6 +988,7 @@ bool Transaction::waitForTransaction(Tra
 
 }
 
+
 // Wait for transaction, unless it would lead to deadlock.
 // Returns the state of transation.
 //
@@ -971,50 +1001,66 @@ bool Transaction::waitForTransaction(Tra
 // use inline assembly or intrinsics to generate memory barrier instead of 
 // volatile. 
 
-State Transaction::waitForTransaction(Transaction *transaction, TransId transId,
+State Transaction::waitForTransaction(TransactionState* transState, TransId transId,
 										bool *deadlock)
 {
+	ASSERT(transState != NULL);
+
 	*deadlock = false;
 	State state;
 
-	if(transaction)
-		transaction->addRef();
+	// Increase the use count on the transaction state object to ensure
+	// the object the waitingFor pointer refers to does not get deleted
+
+	if(transState)
+		transState->addRef();
 
 	Sync syncActiveTransactions(&transactionManager->activeTransactions.syncObject,
 		"Transaction::waitForTransaction(1)");
 	syncActiveTransactions.lock(Shared);
 
-	if (!transaction)
+	// If a transaction state object is not given, locate it by searching
+	// for the transaction in the active list
+
+	if (!transState)
 		{
+		Transaction* transaction; 
+
 		// transaction parameter is not given, find transaction using its ID.
 		for (transaction = transactionManager->activeTransactions.first; transaction;
 			 transaction = transaction->next)
 			{
 			if (transaction->transactionId == transId)
 				{
-				transaction->addRef();
 				break;
 				}
 			}
+
+		// If the transaction is not found in the active list it is committed
+
+		if (!transaction)
+			return Committed;
+
+		transState = transaction->transactionState;
+		transState->addRef();
 		}
 
-	if (!transaction)
-		return Committed;
+	ASSERT(transState != NULL);
 
-	if (transaction->state == Available || transaction->state == Committed)
+	if (transState->state == Available || transState->state == Committed)
 	{
-		state = (State)transaction->state;
-		transaction->release();
+		state = (State)transState->state;
+		transState->release();
 		return state;
 	}
 
 
-	if (!COMPARE_EXCHANGE_POINTER(&waitingFor, NULL, transaction))
+	if (!COMPARE_EXCHANGE_POINTER(&transactionState->waitingFor, NULL, transState))
 		FATAL("waitingFor was not NULL");
 
-	volatile Transaction *trans;
-	for (trans = transaction->waitingFor; trans; trans = trans->waitingFor)
-		if (trans == this)
+	volatile TransactionState *trans;
+	for (trans = transState->waitingFor; trans; trans = trans->waitingFor)
+		if (trans == transactionState)
 			{
 			*deadlock = true;
 			break;
@@ -1025,40 +1071,24 @@ State Transaction::waitForTransaction(Tr
 		try
 			{
 			syncActiveTransactions.unlock();
-			transaction->waitForTransaction();
+			transState->waitForTransaction();
 			}
 		catch(...)
 			{
-			if (!COMPARE_EXCHANGE_POINTER(&waitingFor, transaction, NULL))
-				FATAL("waitingFor was not %p",transaction);
+			if (!COMPARE_EXCHANGE_POINTER(&transactionState->waitingFor, transState, NULL))
+				FATAL("waitingFor was not %p", transState);
 			throw;
 			}
 		}
 
-	if (!COMPARE_EXCHANGE_POINTER(&waitingFor, transaction, NULL))
-		FATAL("waitingFor was not %p",transaction);
+	if (!COMPARE_EXCHANGE_POINTER(&transactionState->waitingFor, transState, NULL))
+		FATAL("waitingFor was not %p", transState);
 
-	state = (State)transaction->state;
-	transaction->release();
+	state = (State)transState->state;
 
-	return state;
-}
+	transState->release();
 
-void Transaction::waitForTransaction()
-{
-	/***
-	Thread *exclusiveThread = syncActive.getExclusiveThread();
-	
-	if (exclusiveThread)
-		{
-		char buffer[1024];
-		connection->getCurrentStatement(buffer, sizeof(buffer));
-		Log::debug("Blocking on %d: %s\n", exclusiveThread->threadId, buffer);
-		}
-	***/
-	
-	Sync sync(&syncIsActive, "Transaction::waitForTransaction(2)");
-	sync.lock(Shared, falcon_lock_wait_timeout * 1000);
+	return state;
 }
 
 void Transaction::addRef()
@@ -1339,7 +1369,7 @@ void Transaction::releaseRecordLocks(voi
 void Transaction::print(void)
 {
 	Log::debug("  %p Id %d, state %d, updates %d, wrtPend %d, records %d\n",
-			this, transactionId, state, hasUpdates, writePending, 
+			this, transactionId, transactionState->state, hasUpdates, writePending, 
 			firstRecord != NULL);
 }
 
@@ -1411,10 +1441,10 @@ void Transaction::getInfo(InfoTable* inf
 	// Need to decide if we want to include the startEvent and endEvent
 	// in this table.
 
-	if (!(state == Available))
+	if (!(transactionState->state == Available))
 		{
 		int n = 0;
-		infoTable->putString(n++, stateNames[state]);
+		infoTable->putString(n++, stateNames[transactionState->state]);
 		infoTable->putInt(n++, mySqlThreadId);
 		infoTable->putInt(n++, transactionId);
 		infoTable->putInt(n++, hasUpdates);
@@ -1422,7 +1452,7 @@ void Transaction::getInfo(InfoTable* inf
 		infoTable->putInt(n++, 0);  // Number of dependencies, will be removed
 		infoTable->putInt(n++, 0); //  was oldestActive);
 		infoTable->putInt(n++, firstRecord != NULL);
-		infoTable->putInt(n++, (waitingFor) ? waitingFor->transactionId : 0);
+		infoTable->putInt(n++, (transactionState->waitingFor) ? transactionState->waitingFor->transactionId : 0);
 		
 		char buffer[512];
 		
@@ -1557,5 +1587,5 @@ void Transaction::validateRecords(void)
 
 bool Transaction::committedBefore(TransId transactionId)
 {
-	return (commitId && commitId < transactionId);
+	return (transactionState->commitId && transactionState->commitId < transactionId);
 }

=== modified file 'storage/falcon/Transaction.h'
--- a/storage/falcon/Transaction.h	2009-03-07 01:37:19 +0000
+++ b/storage/falcon/Transaction.h	2009-03-23 14:57:48 +0000
@@ -27,6 +27,7 @@
 #include "SyncObject.h"
 #include "SerialLog.h"
 #include "SavePoint.h"
+#include "TransactionState.h"
 
 static const int NO_TRANSACTION = 0;
 
@@ -44,27 +45,6 @@ class InfoTable;
 class Thread;
 class TransactionManager;
 
-// Transaction States
-
-enum State {
-	Active,					// 0
-	Limbo,					// 1
-	Committed,				// 2
-	RolledBack,				// 3
-
-	// The following are 'relative states'.  See getRelativeState()
-
-	Us,						// 4
-	CommittedVisible,		// 5
-	CommittedInvisible,		// 6
-	WasActive,				// 7
-	Deadlock,				// 8
-	
-	// And the remaining are for transactions pending reuse
-	
-	Available,				// 9
-	Initializing			// 10
-	};
 
 struct TransState {
 	Transaction	*transaction;
@@ -87,12 +67,13 @@ public:
 	Transaction(Connection *connection, TransId seq);
 
 	State		getRelativeState(Record* record, uint32 flags);
-	State		getRelativeState (Transaction *transaction, TransId transId, uint32 flags);
+	State		getRelativeState (TransactionState* ts, TransId transId, uint32 flags);
 	void		removeRecordNoLock (RecordVersion *record);
 	void		removeRecord(RecordVersion *record);
 	void		removeRecord (RecordVersion *record, RecordVersion **ptr);
 	void		commitRecords();
 	bool		visible (Transaction *transaction, TransId transId, int forWhat);
+	bool		visible(const TransactionState* transState, int forWhat) const;
 	bool		needToLock(Record* record);
 	void		addRecord (RecordVersion *record);
 	void		prepare(int xidLength, const UCHAR *xid);
@@ -100,9 +81,9 @@ public:
 	void		commit();
 	void		release();
 	void		addRef();
-	void		waitForTransaction();
+	//void		waitForTransaction();
 	bool		waitForTransaction (TransId transId);
-	State		waitForTransaction (Transaction *transaction, TransId transId, bool *deadlock);
+	State		waitForTransaction (TransactionState* ts, TransId transId, bool *deadlock);
 	void		dropTable(Table* table);
 	void		truncateTable(Table* table);
 	bool		hasRecords(Table* table);
@@ -134,19 +115,22 @@ public:
 
 	inline bool isActive()
 		{
-		return state == Active || state == Limbo;
+		return transactionState->state == Active || transactionState->state == Limbo;
+		}
+
+	State getState() const
+		{
+		return static_cast<State>(transactionState->state);
 		}
 
 	Connection		*connection;
 	Database		*database;
 	TransactionManager	*transactionManager;
 	TransId			transactionId;  // used also as startEvent by dep.mgr.
-	TransId			commitId;       // used as commitEvent by dep.mgr.
 	TransId			blockedBy;
 	int				curSavePointId;
 	Transaction		*next;			// next in database
 	Transaction		*prior;			// next in database
-	volatile	Transaction		*waitingFor;
 	SavePoint		*savePoints;
 	SavePoint		*freeSavePoints;
 	SavePoint		localSavePoints[LOCAL_SAVE_POINTS];
@@ -167,7 +151,6 @@ public:
 	bool			pendingPageWrites;
 	bool			hasLocks;
 	SyncObject		syncObject;
-	SyncObject		syncIsActive;		// locked while transaction is active
 	SyncObject		syncDeferredIndexes;
 	SyncObject		syncRecords;
 	SyncObject		syncSavepoints;
@@ -183,8 +166,8 @@ public:
 	uint32			deletedRecords;		// active deleted records (exclusive of backllogged ones)
 	RecordVersion	**chillPoint;		// points to a pointer to the first non-chilled record
 	int				scanIndexCount;
+	TransactionState* transactionState;
 
-	volatile INTERLOCK_TYPE	state;
 	volatile INTERLOCK_TYPE	useCount;
 	volatile INTERLOCK_TYPE	inList;
 

=== modified file 'storage/falcon/TransactionManager.cpp'
--- a/storage/falcon/TransactionManager.cpp	2009-03-18 13:31:34 +0000
+++ b/storage/falcon/TransactionManager.cpp	2009-03-23 14:57:48 +0000
@@ -19,6 +19,7 @@
 #include "Engine.h"
 #include "TransactionManager.h"
 #include "Transaction.h"
+#include "TransactionState.h"
 #include "Sync.h"
 #include "Interlock.h"
 #include "SQLError.h"
@@ -40,6 +41,11 @@ static const char THIS_FILE[]=__FILE__;
 volatile int Talloc = 0;  // Temp. will be removed. Used for tracing
 volatile int Tdelete = 0; // new and delete of trans objects.
 
+// These are for debugging to trac number of object allocations
+extern volatile INTERLOCK_TYPE TSalloc;
+extern volatile INTERLOCK_TYPE TSdelete;
+
+
 //////////////////////////////////////////////////////////////////////
 // Construction/Destruction
 //////////////////////////////////////////////////////////////////////
@@ -54,7 +60,7 @@ TransactionManager::TransactionManager(D
 	priorCommitted = 0;
 	priorRolledBack = 0;
 	rolledBackTransaction = new Transaction(database->systemConnection, 0);
-	rolledBackTransaction->state = RolledBack;
+	rolledBackTransaction->transactionState->state = RolledBack;
 	rolledBackTransaction->inList = false;
 	syncObject.setName("TransactionManager::syncObject");
 	activeTransactions.syncObject.setName("TransactionManager::activeTransactions");
@@ -68,7 +74,7 @@ TransactionManager::~TransactionManager(
 	for (Transaction *transaction; (transaction = activeTransactions.first);)
 		{
 		transaction->inList = false;
-		transaction->state = Committed;
+		transaction->transactionState->state = Committed;
 		activeTransactions.first = transaction->next;
 		transaction->release();
 		}
@@ -113,8 +119,8 @@ Transaction* TransactionManager::startTr
 	Transaction *transaction;
 
 	for(transaction = activeTransactions.first; transaction; transaction = transaction->next)
-		if (transaction->state == Available)
-			if (COMPARE_EXCHANGE(&transaction->state, Available, Initializing))
+		if (transaction->transactionState->state == Available)
+			if (COMPARE_EXCHANGE(&transaction->transactionState->state, Available, Initializing))
 				{
 				transaction->initialize(connection, INTERLOCKED_INCREMENT(transactionSequence));
 				return transaction;
@@ -214,7 +220,7 @@ void TransactionManager::commitByXid(int
 		again = false;
 		
 		for (Transaction *transaction = activeTransactions.first; transaction; transaction = transaction->next)
-			if (transaction->state == Limbo && transaction->isXidEqual(xidLength, xid))
+			if (transaction->getState() == Limbo && transaction->isXidEqual(xidLength, xid))
 				{
 				sync.unlock();
 				transaction->commit();
@@ -235,7 +241,7 @@ void TransactionManager::rollbackByXid(i
 		again = false;
 		
 		for (Transaction *transaction = activeTransactions.first; transaction; transaction = transaction->next)
-			if (transaction->state == Limbo && transaction->isXidEqual(xidLength, xid))
+			if (transaction->getState() == Limbo && transaction->isXidEqual(xidLength, xid))
 				{
 				sync.unlock();
 				transaction->rollback();
@@ -324,8 +330,8 @@ void TransactionManager::purgeTransactio
 	Transaction* transaction = committedTransactions.first;
 
 	while ((transaction != NULL) &&
-		   (transaction->state == Committed) &&
-		   (transaction->commitId <= oldestActive) &&
+		   (transaction->getState() == Committed) &&
+		   (transaction->transactionState->commitId <= oldestActive) &&
 		   !transaction->writePending)
 		{
 		transaction->commitRecords();
@@ -365,10 +371,10 @@ void TransactionManager::getSummaryInfo(
 	
 	for (transaction = activeTransactions.first; transaction; transaction = transaction->next)
 		{
-		if (transaction->state == Active)
+		if (transaction->getState() == Active)
 			++numberActive;
 			
-		if (transaction->state == Committed)
+		if (transaction->getState() == Committed)
 			++numberPendingCommit;
 		}
 	syncActive.unlock();
@@ -398,13 +404,13 @@ void TransactionManager::reportStatistic
 	time_t maxTime = 0;
 	
 	for (transaction = activeTransactions.first; transaction; transaction = transaction->next)
-		if (transaction->state == Active)
+		if (transaction->getState() == Active)
 			{
 			++active;
 			time_t ageTime = database->deltaTime - transaction->startTime;
 			maxTime = MAX(ageTime, maxTime);
 			}
-		else if (transaction->state == Available)
+		else if (transaction->getState() == Available)
 			{
 			++available;
 			}
@@ -455,7 +461,7 @@ Transaction* TransactionManager::findTra
 
 void TransactionManager::removeTransaction(Transaction* transaction)
 {
-	if (transaction->state == Committed)
+	if (transaction->getState() == Committed)
 		{
 		Sync sync(&committedTransactions.syncObject, "TransactionManager::removeTransaction(1)");
 		sync.lock(Exclusive);
@@ -488,7 +494,7 @@ void TransactionManager::printBlockage(v
 	sync.lock (Shared);
 
 	for (Transaction *trans = activeTransactions.first; trans; trans = trans->next)
-		if (trans->state == Active && !trans->waitingFor)
+		if (trans->getState() == Active && !trans->transactionState->waitingFor)
 			trans->printBlocking(0);
 
 	Synchronize::freezeSystem();
@@ -497,6 +503,6 @@ void TransactionManager::printBlockage(v
 void TransactionManager::printBlocking(Transaction* transaction, int level)
 {
 	for (Transaction *trans = activeTransactions.first; trans; trans = trans->next)
-		if (trans->state == Active && trans->waitingFor == transaction)
+		if (trans->getState() == Active && trans->transactionState->waitingFor == transaction->transactionState)
 			trans->printBlocking(level);
 }

=== added file 'storage/falcon/TransactionState.cpp'
--- a/storage/falcon/TransactionState.cpp	1970-01-01 00:00:00 +0000
+++ b/storage/falcon/TransactionState.cpp	2009-03-23 14:57:48 +0000
@@ -0,0 +1,48 @@
+/* Copyright (C) 2009 Sun Microsystems, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+
+#include "TransactionState.h"
+#include "Interlock.h"
+#include "Sync.h"
+
+extern uint		falcon_lock_wait_timeout;
+
+TransactionState::TransactionState() : commitId(0), waitingFor(NULL), useCount(1)
+{
+	syncIsActive.setName("TransactionState::syncIaActive");
+}
+
+
+void TransactionState::waitForTransaction()
+{
+	Sync sync(&syncIsActive, "TransactionState::waitForTransaction");
+	sync.lock(Shared, falcon_lock_wait_timeout * 1000);
+}
+
+
+void TransactionState::addRef()
+{
+	INTERLOCKED_INCREMENT(useCount);
+}
+
+
+void TransactionState::release()
+{
+	ASSERT(useCount > 0);
+
+	if (INTERLOCKED_DECREMENT(useCount) == 0)
+		delete this;
+}

=== added file 'storage/falcon/TransactionState.h'
--- a/storage/falcon/TransactionState.h	1970-01-01 00:00:00 +0000
+++ b/storage/falcon/TransactionState.h	2009-03-23 14:57:48 +0000
@@ -0,0 +1,79 @@
+/* Copyright (C) 2009 Sun Microsystems, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+#if !defined(TRANSACTION_STATE_H)
+#define TRANSACTION_STATE_H
+
+#include "Engine.h"
+#include "SyncObject.h"
+
+// Transaction States
+
+enum State {
+	Active,					// 0
+	Limbo,					// 1
+	Committed,				// 2
+	RolledBack,				// 3
+
+	// The following are 'relative states'.  See getRelativeState()
+
+	Us,						// 4
+	CommittedVisible,		// 5
+	CommittedInvisible,		// 6
+	WasActive,				// 7
+	Deadlock,				// 8
+	
+	// And the remaining are for transactions pending reuse
+	
+	Available,				// 9
+	Initializing			// 10
+	};
+
+
+// TransactionState stores the main state information for a Transaction.
+// The reason for having this as a separate class instead of having it
+// stored in the Transaction object is that we want to be able to purge
+// transaction objects earlier than when record versions are scavanged. 
+// To be able to do that we let the TransactionState object live as long
+// as there is RecordVersions refering to it while we purge the Transaction 
+// objects earlier. 
+
+class TransactionState
+{
+public:
+	TransactionState();
+
+	void waitForTransaction();
+
+	bool isActive() const
+	    {
+		return state == Active || state == Limbo;
+	    }
+
+	void addRef();
+	void release();
+
+public:
+	TransId			transactionId;       // used also as startEvent by dep.mgr.
+	TransId			commitId;            // used as commitEvent by dep.mgr.
+	volatile		INTERLOCK_TYPE state;
+	SyncObject 		syncIsActive;
+	volatile TransactionState* waitingFor; // Used for deadlock detection
+
+private:
+	volatile INTERLOCK_TYPE useCount;
+};
+
+#endif


Attachment: [text/bzr-bundle] bzr/olav@sun.com-20090323145748-2rb50us7u6egwl34.bundle
Thread
bzr commit into mysql-6.0-falcon-team branch (olav:3069) Bug#41357Olav Sandstaa23 Mar