List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:June 24 2009 1:18pm
Subject:Re: bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2739)
Bug#45665
View as plain text  
Kevin,

The patch looks fine and I think your changes make the code simpler to 
read by reading the data.record once and also by doing the explicit 
checks for the special recordversion/states directly instead of having 
the conversion of these hidden in Record::findRecordData().

OK to push.

Olav

Kevin Lewis wrote:
> #At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on
> revid:kevin.lewis@stripped
>
>  2739 Kevin Lewis	2009-06-23
>       Bug#45665 - Change the tests so that the Record::data.record is read once 
>       and tested multiple times instead of calling Record functions to test it
>       multiple times in Table::checkUniqueRecordVersion().  
>      @ storage/falcon/Record.cpp
>         Bug#45665 - findRecordData is simplified to an inline.
>         isAllocated(void)  and isRecordDataFreed() are not used.
>      @ storage/falcon/Record.h
>         Bug#45665 - findRecordData is simplified to an inline.
>         isAllocated(void)  and isRecordDataFreed() are not used.
>      @ storage/falcon/RecordVersion.cpp
>         Bug#45665 - findRecordData is simplified to an inline.
>      @ storage/falcon/RecordVersion.h
>         Bug#45665 - findRecordData is simplified to an inline.
>      @ storage/falcon/SRLUpdateRecords.cpp
>         Bug#45665 - Rename dataRecord to RecordData for consistency.
>         SRLUpdateRecords::thaw() uses Record::findRecordData() to get
>         Record::data.record.  But it needs to return null if the record
>         is not thawed.  Record::findRecordData() is simplified to return
>         exactly what is there so converting 'special' pointer values to
>         null is not done here.
>      @ storage/falcon/Table.cpp
>         Bug#45665 - Change the tests so that the Record::data.record is read once 
>         and tested multiple times instead of calling Record functions to test it
>         multiple times in Table::checkUniqueRecordVersion().
>      @ storage/falcon/Transaction.cpp
>         Bug#45665 - Use isRolledBack() in an assert at the bottom of
>         Transaction::getRelativeState().
>         At this point, the transaction can be no other.
>      @ storage/falcon/TransactionState.h
>         Use isRolledBack() in an assert at the bottom of
> Transaction::getRelativeState().
>
>     modified:
>       storage/falcon/Record.cpp
>       storage/falcon/Record.h
>       storage/falcon/RecordVersion.cpp
>       storage/falcon/RecordVersion.h
>       storage/falcon/SRLUpdateRecords.cpp
>       storage/falcon/Table.cpp
>       storage/falcon/Transaction.cpp
>       storage/falcon/TransactionState.h
> === modified file 'storage/falcon/Record.cpp'
> --- a/storage/falcon/Record.cpp	2009-06-03 18:15:32 +0000
> +++ b/storage/falcon/Record.cpp	2009-06-23 21:16:25 +0000
> @@ -1051,11 +1051,6 @@ bool Record::isDeleted(void)
>  	return false;		// Only RecordVersions are used to delete records!
>  }
>  
> -bool Record::isRecordDataFreed(void)
> -{
> -	return (data.record == recordDataIsFreed);
> -}
> -
>  void Record::print(void)
>  {
>  	printf("  %p\tId %d, enc %d, state %d, use %d, grp " I64FORMAT "\n",
> @@ -1247,18 +1242,6 @@ char* Record::getRecordData(void)
>  	return recordData;
>  }
>  
> -char* Record::findRecordData(void)
> -{
> -	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
>
> === modified file 'storage/falcon/Record.h'
> --- a/storage/falcon/Record.h	2009-06-10 15:05:08 +0000
> +++ b/storage/falcon/Record.h	2009-06-23 21:16:25 +0000
> @@ -130,7 +130,6 @@ 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);
> @@ -168,8 +167,6 @@ public:
>  	void				expungeRecord(void);
>  	int					getDataMemUsage(void);
>  	int					getMemUsage(void);
> -	bool				isRecordDataFreed();
> -	bool				isAllocated(void);
>  	bool				isAllocated(char* recordData);
>  
>  	Record (Table *table, Format *recordFormat);
> @@ -184,13 +181,19 @@ protected:
>  		char	*record;
>  		}		data;
>  
> +public:
> +
>  	inline bool hasNoData(char* recordData)
>  		{
>  		ASSERT(recordData != NULL);
>  		return (recordData == recordIsALock || recordData == recordIsDeleted || recordData
> == recordDataIsFreed);
>  		}
> +	inline char* findRecordData(void)
> +		{
> +		ASSERT(data.record != NULL);
> +		return data.record;
> +		}
>  
> -public:
>  	uint64		generation;
>  	Format		*format;
>  	volatile INTERLOCK_TYPE useCount;
>
> === modified file 'storage/falcon/RecordVersion.cpp'
> --- a/storage/falcon/RecordVersion.cpp	2009-06-04 12:55:34 +0000
> +++ b/storage/falcon/RecordVersion.cpp	2009-06-23 21:16:25 +0000
> @@ -570,16 +570,3 @@ 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-06-03 01:36:57 +0000
> +++ b/storage/falcon/RecordVersion.h	2009-06-23 21:16:25 +0000
> @@ -58,7 +58,6 @@ 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();
>
> === modified file 'storage/falcon/SRLUpdateRecords.cpp'
> --- a/storage/falcon/SRLUpdateRecords.cpp	2009-06-10 23:54:03 +0000
> +++ b/storage/falcon/SRLUpdateRecords.cpp	2009-06-23 21:16:25 +0000
> @@ -64,10 +64,10 @@ bool SRLUpdateRecords::chill(Transaction
>  char* SRLUpdateRecords::thaw(RecordVersion *record, int *bytesReallocated)
>  {
>  	// Nothing to do if record is no longer chilled
> -	
> -	char* dataRecord = record->findRecordData();
> -	if (dataRecord != recordIsChilled)
> -		return dataRecord ;
> +
> +	char* recordData = record->findRecordData();
> +	if (recordData != recordIsChilled)
> +		return recordData ;
>  
>  	// Find the window where the record is stored using the record offset, then
>  	// activate the window, reading from disk if necessary
> @@ -121,9 +121,12 @@ char* SRLUpdateRecords::thaw(RecordVersi
>  	// setRecordData() handles race conditions with an interlocked compare and
> exchange,
>  	// but check the state anyway.
>  
> -	dataRecord = record->findRecordData();
> -	if (dataRecord == recordIsChilled)
> -		dataRecord = record->setRecordData(control->input, dataLength,
> bytesReallocated);
> +	recordData = record->findRecordData();
> +	if ((recordData == recordIsALock || recordData == recordIsDeleted || recordData ==
> recordDataIsFreed))
> +		recordData = NULL;
> +
> +	else if (recordData == recordIsChilled)
> +		recordData = record->setRecordData(control->input, dataLength,
> bytesReallocated);
>  
>  
>  	if (*bytesReallocated > 0)
> @@ -134,7 +137,7 @@ char* SRLUpdateRecords::thaw(RecordVersi
>  			log->chilledRecords--;
>  		}
>  
> -	return dataRecord;
> +	return recordData;
>  }
>  
>  void SRLUpdateRecords::append(Transaction *transaction, RecordVersion *records, bool
> chillRecords)
>
> === modified file 'storage/falcon/Table.cpp'
> --- a/storage/falcon/Table.cpp	2009-06-12 18:17:19 +0000
> +++ b/storage/falcon/Table.cpp	2009-06-23 21:16:25 +0000
> @@ -29,6 +29,7 @@
>  #include "Dbb.h"
>  #include "PStatement.h"
>  #include "Transaction.h"
> +#include "TransactionState.h"
>  #include "Value.h"
>  #include "Format.h"
>  #include "RSet.h"
> @@ -2586,16 +2587,16 @@ bool Table::checkUniqueRecordVersion(int
>  
>  		state = transaction->getRelativeState(dup, DO_NOT_WAIT);
>  
> -		// Check for a deleted record or a record lock
> +		// Check for a record lock.  No need to thaw here.
>  
> -		if (!dup->hasRecord())
> -			{
> -			// A record without a data buffer is either a lock record or a deleted record.
> +		char *dataRecord = dup->findRecordData();
> +		if ( (dataRecord == recordIsALock) || (dataRecord == recordDataIsFreed))
> +			continue;	// Next record version.
>  
> -			if (dup->isALock() || dup->isRecordDataFreed())
> -				continue;	// Next record version.
> +		// Deleted records require special actions.
>  
> -			ASSERT(dup->isDeleted());  // Deleted records require special actions.
> +		if (dataRecord == recordIsDeleted)
> +			{
>  			switch (state)
>  				{
>  				case CommittedVisible:
>
> === modified file 'storage/falcon/Transaction.cpp'
> --- a/storage/falcon/Transaction.cpp	2009-06-03 18:15:32 +0000
> +++ b/storage/falcon/Transaction.cpp	2009-06-23 21:16:25 +0000
> @@ -912,7 +912,7 @@ State Transaction::getRelativeState(Tran
>  		return WasActive;			// caller will need to re-fetch
>  		}
>  
> -	if (transState->state == Committed)
> +	if (transState->isCommitted())
>  		{
>  		// Return CommittedVisible if the other trans has a lower TransId and 
>  		// it was committed when we started.
> @@ -923,6 +923,7 @@ State Transaction::getRelativeState(Tran
>  		return CommittedInvisible;
>  		}
>  
> +	ASSERT(transState->isRolledBack());
>  	return (State) transState->state;
>  }
>  
>
> === modified file 'storage/falcon/TransactionState.h'
> --- a/storage/falcon/TransactionState.h	2009-03-27 13:43:10 +0000
> +++ b/storage/falcon/TransactionState.h	2009-06-23 21:16:25 +0000
> @@ -70,6 +70,11 @@ public:
>  		return state == Committed;
>  		}
>  
> +	inline bool	isRolledBack()
> +		{
> +		return state == RolledBack;
> +		}
> +
>  
>  public:
>  	TransId			transactionId;       // used also as startEvent by dep.mgr.
>
>   
> ------------------------------------------------------------------------
>
>

Thread
bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2739)Bug#45665Kevin Lewis23 Jun
  • Re: bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2739)Bug#45665Olav Sandstaa24 Jun