bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2709) Bug#44946

[Kevin Lewis]

#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on revid:kevin.lewis@stripped

 2709 Kevin Lewis	2009-05-20
      Bug#44946 Code review changes;
      It is not necessary to us CAS to update the Record::highWater.  Explained in comments.
      Added other comments and compiler warning fixes.

    modified:
      storage/falcon/Record.cpp
      storage/falcon/Record.h
      storage/falcon/Table.cpp
      storage/falcon/Transaction.cpp
=== modified file 'storage/falcon/Record.cpp'
--- a/storage/falcon/Record.cpp	2009-05-20 04:46:31 +0000
+++ b/storage/falcon/Record.cpp	2009-05-20 16:58:14 +0000
@@ -658,6 +658,8 @@ int Record::getEncodedSize()
 		}
 }
 
+// Get an encoded value from the record.
+
 void Record::getEncodedValue(char* recordData, int fieldId, Value *value)
 {
 	switch (encoding)
@@ -669,8 +671,17 @@ void Record::getEncodedValue(char* recor
 
 			if (highWater < index)
 				{
-				INTERLOCK_TYPE prevHighWater, myHighWater = highWater;
-				const UCHAR *p = (UCHAR*) recordData + vector[highWater];
+				// The vector offset for the field we want has not yet been determined.
+				// So find the offsets for each field and put it in the vector.
+				// It is possible for this function and loop to be called concurrently 
+				// by multiple threads.  But each thread is writting the same values
+				// into the vector.  If they are targeting different highValue marks,
+				// it is possible for a higher value to be overwritten with a lower value.
+				// Both are correct.  If that happens, the highWater will just be 
+				// re-calculated later.  This is better than a CAS for every highWater write.
+
+				short myHighWater = highWater;
+				const UCHAR *p = (UCHAR*) recordData + vector[myHighWater];
 
 				while (myHighWater < index)
 					{
@@ -679,9 +690,7 @@ void Record::getEncodedValue(char* recor
 					p = q;
 					}
 
-				while (myHighWater > (prevHighWater = highWater))
-					if (COMPARE_EXCHANGE(&highWater, prevHighWater, myHighWater))
-						break;
+				highWater = myHighWater;
 				}
 
 			if (vector[index] > size - getSize())
@@ -744,6 +753,9 @@ char* Record::setEncodedRecord(Stream *s
 
 	if (interlocked)
 		{
+		// Do not adjust the size.  The interlocked calls are only done during a thaw. 
+		// When a record is chilled, the size variable is left the same.
+
 		ASSERT(size == getSize() + totalLength);
 
 		// If data.record has changed since allocating the new buffer, then free the new buffer
@@ -756,6 +768,9 @@ char* Record::setEncodedRecord(Stream *s
 		}
 	else
 		{
+		// Adjust 'size' only for non-interlocked calls.  These are new records.
+		// The allocated buffer size has not yet been added in.
+
 		size += totalLength;
 		data.record = dataBuffer;
 		}

=== modified file 'storage/falcon/Record.h'
--- a/storage/falcon/Record.h	2009-05-20 04:46:31 +0000
+++ b/storage/falcon/Record.h	2009-05-20 16:58:14 +0000
@@ -186,7 +186,7 @@ public:
 	volatile INTERLOCK_TYPE useCount;
 	int			recordNumber;
 	int			size;
-	volatile INTERLOCK_TYPE highWater;
+	short		highWater;
 	UCHAR		encoding;
 	UCHAR		state;
 

=== modified file 'storage/falcon/Table.cpp'
--- a/storage/falcon/Table.cpp	2009-05-12 14:35:40 +0000
+++ b/storage/falcon/Table.cpp	2009-05-20 16:58:14 +0000
@@ -3535,7 +3535,17 @@ Record* Table::fetchForUpdate(Transactio
 					}
 
 				if (!source->hasRecord())
-					FATAL("Committed non-deleted record has no data buffer.\n");
+					{
+					if (source->isVersion())
+						{
+						RecordVersion *rv = (RecordVersion *) source;
+						FATAL("Committed non-deleted record has no data buffer. state=%d, useCount=%d, virtualOffset=" I64FORMAT "\n", 
+							rv->state, rv->useCount, rv->virtualOffset);
+						}
+					else
+						FATAL("Committed non-deleted record has no data buffer. state=%d, useCount=%d" I64FORMAT "\n", 
+							source->state, source->useCount);
+					}
 
 				// Lock the record
 

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2009-05-20 04:46:31 +0000
+++ b/storage/falcon/Transaction.cpp	2009-05-20 16:58:14 +0000
@@ -1608,7 +1608,5 @@ void Transaction::thawAll(void)
 	syncRec.lock(Shared);
 
 	for (RecordVersion *record = firstRecord; record && record->nextInTrans; record = record->nextInTrans)
-		{
-		char* recordData = record->getRecordData();
-		}
+		record->getRecordData();
 }


Attachment: [text/bzr-bundle] bzr/[e-mail]