MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Kevin Lewis Date:January 14 2009 10:34pm
Subject:bzr commit into mysql-6.0-falcon-team branch (klewis:2959) Bug#41831,
Bug#42080
View as plain text  
#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/

 2959 Kevin Lewis	2009-01-14
      Bug#42080, Bug#41831  In both of these bugs, the scavenger was pruning
      records that it should not have.  The record version chosen to start 
      pruning is returned from RecordScavenge::inventoryRecord().  This 
      function is improved so that only the oldest visible record is returned.
      In addition, Recordversion::committedbefore() is added to simplify 
      the code and read RecordVersion::transaction only once since it can be 
      set to null at any time.
modified:
  storage/falcon/Record.cpp
  storage/falcon/RecordScavenge.cpp
  storage/falcon/RecordVersion.cpp
  storage/falcon/RecordVersion.h

per-file messages:
  storage/falcon/Record.cpp
    This may be called by pruneRecords for the last record in a chain. Its OK
  storage/falcon/RecordScavenge.cpp
    Bug#42080, Bug#41831  In both of these bugs, the scavenger was pruning
    records that it should not have.  The record version chosen to start pruning is returned from RecordScavenge::inventoryRecord().  This function is improved so that only the oldest visible record is returned.
    In addition, Recordversion::committedbefore() is added to simplify 
    the code and read RecordVersion::transaction only once since it can be 
    set to null at any time.
  storage/falcon/RecordVersion.cpp
    Recordversion::committedbefore() is added to simplify the code and read RecordVersion::transaction only once since it can be set to null at any time.
  storage/falcon/RecordVersion.h
    Recordversion::committedbefore() is added to simplify the code and read RecordVersion::transaction only once since it can be set to null at any time.
=== modified file 'storage/falcon/Record.cpp'
--- a/storage/falcon/Record.cpp	2009-01-09 21:49:16 +0000
+++ b/storage/falcon/Record.cpp	2009-01-14 22:33:44 +0000
@@ -855,7 +855,6 @@ Record* Record::releaseNonRecursive(void
 
 Record* Record::clearPriorVersion(void)
 {
-	ASSERT(false);
 	return NULL;
 }
 

=== modified file 'storage/falcon/RecordScavenge.cpp'
--- a/storage/falcon/RecordScavenge.cpp	2009-01-10 16:03:04 +0000
+++ b/storage/falcon/RecordScavenge.cpp	2009-01-14 22:33:44 +0000
@@ -124,33 +124,60 @@ Record* RecordScavenge::inventoryRecord(
 
 		// Check if this record can be scavenged somehow
 
-		if (rec->isVersion())
+		if (!rec->isVersion())
+			{
+			// This Record object was read from a page on disk
+
+			if (rec == record)
+				scavengeType = CAN_BE_RETIRED;  // This is a base Record object 
+			else
+				{
+				// There must be some newer RecordVersions.
+
+				if (oldestVisibleRec)
+					scavengeType = CAN_BE_PRUNED;   // This is a Record object at the end of a chain.
+				else
+					oldestVisibleRec = rec;
+				}
+			}
+
+		else      // This is a RecordVersion object
 			{
 			RecordVersion * recVer = (RecordVersion *) rec;
 
-			bool committedBeforeAnyActiveTrans = false;
-			if (  !recVer->transaction
-				|| recVer->transaction->committedBefore(oldestActiveTransaction))
-				committedBeforeAnyActiveTrans = true;
+			// We assume here that Transaction::commitRecords is only called
+			// when there are no dependent transactions.  It means that if the 
+			// transaction pointer is null, and we do not know the commitId,
+			// Then we can be assured that the recVer was committed before
+			// any active transaction, including oldestActiveTransaction.
 
-			// This record may be retired if it is the base record AND
+			bool committedBeforeAnyActiveTrans = recVer->committedBefore(oldestActiveTransaction);
+			
+			// This recVer 'may' be retired if it is the base record AND
 			// it is currently not needed by any active transaction.
 
 			if (recVer == record && committedBeforeAnyActiveTrans)
 				scavengeType = CAN_BE_RETIRED;
 			
 			// Look for the oldest visible record version in this chain.
-			// If the transaction is null then there are no dependent transactions
-			// and this record version is visible to all.  If the transaction is not null,
-			// then check if it ended before the current oldest active trans started.
 
 			if (oldestVisibleRec)
 				{
-				if (recVer->useCount > 1)
-					// TBD - A prunable record has an extra use count!  Why? by who?
-					oldestVisibleRec = rec; // reset so that this recVer is not pruned.
+				// Younger transactions may commit before older transactions.
+				// If this older record is visible, then forget oldestVisibleRec
+
+				if (!committedBeforeAnyActiveTrans)
+					oldestVisibleRec = NULL;
 
-				scavengeType = CAN_BE_PRUNED;
+				else
+					{
+					// Do not prune records that have other pointers to them.
+
+					if (recVer->useCount != 1)
+						oldestVisibleRec = rec;   // Rreset this pointer.
+					else
+						scavengeType = CAN_BE_PRUNED;
+					}
 				}
 			else if (committedBeforeAnyActiveTrans)
 				{
@@ -158,10 +185,6 @@ Record* RecordScavenge::inventoryRecord(
 				oldestVisibleRec = rec;
 				}
 			}
-		else if (oldestVisibleRec)
-			scavengeType = CAN_BE_PRUNED;   // This is a Record object at the end of a chain.
-		else
-			scavengeType = CAN_BE_RETIRED;  // This is a base Record object 
 
 		// Add up the scavengeable space.
 
@@ -182,8 +205,8 @@ Record* RecordScavenge::inventoryRecord(
 				unScavengeableSpace += size;
 			}
 
-		// Only base records can be retired.  Add up all retireable records 
-		// in a array of relative ages from our baseGeneration.
+		// Add up all retireable records in a array of relative ages 
+		// from our baseGeneration. Only base records can be retired.
 
 		if (rec == record)
 			{

=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp	2009-01-13 08:39:22 +0000
+++ b/storage/falcon/RecordVersion.cpp	2009-01-14 22:33:44 +0000
@@ -180,6 +180,23 @@ void RecordVersion::commit()
 	transaction = NULL;
 }
 
+// Return true if this record has been committed before a certain transactionId
+
+bool RecordVersion::committedBefore(TransId transId)
+{
+	// The transaction pointer in this record can disapear at any time due to 
+	// another call to Transaction::commitRecords().  So read it locally
+
+	Transaction *transactionPtr = transaction;
+	if (transactionPtr)
+		return transactionPtr->committedBefore(transId);
+
+	// If the transaction Pointer is null, then this record is committed.
+	// All we have is the starting point for these transactions.
+	return (transactionId < transId);
+}
+
+
 bool RecordVersion::retire(RecordScavenge *recordScavenge)
 {
 	bool neededByAnyActiveTrans = true;
@@ -232,7 +249,8 @@ void RecordVersion::scavengeSavepoint(Tr
 
 	// Remove prior record versions assigned to the savepoint being released
 	
-	for (; rec && rec->getTransactionId() == targetTransactionId && rec->getSavePointId() >= oldestActiveSavePointId;
+	for (; (   rec && rec->getTransactionId() == targetTransactionId 
+		    && rec->getSavePointId() >= oldestActiveSavePointId);
 		  rec = rec->getPriorVersion())
 		{
 		ptr = rec;

=== modified file 'storage/falcon/RecordVersion.h'
--- a/storage/falcon/RecordVersion.h	2009-01-09 21:49:16 +0000
+++ b/storage/falcon/RecordVersion.h	2009-01-14 22:33:44 +0000
@@ -59,6 +59,7 @@ public:
 	virtual void		serialize(Serialize* stream);
 
 	void				commit();
+	bool				committedBefore(TransId);
 
 protected:
 	virtual ~RecordVersion();

Thread
bzr commit into mysql-6.0-falcon-team branch (klewis:2959) Bug#41831,Bug#42080Kevin Lewis15 Jan