bk commit into 6.0 tree (cpowers:1.2623) BUG#34778

[cpowers]

Below is the list of changes that have just been committed into a local
6.0 repository of cpowers.  When cpowers does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-03-30 02:24:29-05:00, cpowers@stripped +8 -0
  Bug#34778, "Possible memory leak running UPDATEs in tight loop"
  
  This test generates thousands of prior record versions for just a
  very few records. The record scavenger ignored the prior record chain
  because the base record was not a scavenge candidate.
  
  The scavenger now scavenges prior records that match the transaction and
  record generation criteria, even when the base record is not scavengable.

  storage/falcon/Database.cpp@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +2
-2
    Replace scavenge generation of -1 with the constant "UNDEFINED".

  storage/falcon/Record.cpp@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +1 -1
    Added LockType parameter to Record::scavenge.

  storage/falcon/Record.h@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +3 -1
    Added LockType parameter to Record::scavenge.

  storage/falcon/RecordLeaf.cpp@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +6
-5
    Consolidated record scavenge logic in RecordVersion::scavenge().
    Pass record leaf lock type to RecordVersion::scavenge().

  storage/falcon/RecordScavenge.h@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped
+1 -0
    Define the scavenge generation constant UNDEFINED.

  storage/falcon/RecordVersion.cpp@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped
+50 -14
    Consolidated record scavenging logic in RecordVersion::scavenge().
    Modified RecordVersion::scavenge() to scavenge prior record versions even
    when the base record is not a scavenge candidate.

  storage/falcon/RecordVersion.h@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +1
-1
    Added LockType parameter to scavenge.

  storage/falcon/Table.cpp@stripped, 2008-03-30 02:24:27-05:00, cpowers@stripped +3 -1
    Set record->active = false in Table::validateUpdate() for unused record versions.

diff -Nrup a/storage/falcon/Database.cpp b/storage/falcon/Database.cpp
--- a/storage/falcon/Database.cpp	2008-03-28 17:44:33 -05:00
+++ b/storage/falcon/Database.cpp	2008-03-30 02:24:27 -05:00
@@ -1836,14 +1836,14 @@ void Database::retireRecords(bool forced
 		}
 	else if ((total - lastRecordMemory) < recordScavengeThreshold / AGE_GROUPS)
 		{
-		recordScavenge.scavengeGeneration = -1;
+		recordScavenge.scavengeGeneration = UNDEFINED;
 		cleanupRecords (&recordScavenge);
 		
 		return;
 		}
 	else
 		{
-		recordScavenge.scavengeGeneration = -1;
+		recordScavenge.scavengeGeneration = UNDEFINED;
 		cleanupRecords (&recordScavenge);
 		}
 
diff -Nrup a/storage/falcon/Record.cpp b/storage/falcon/Record.cpp
--- a/storage/falcon/Record.cpp	2008-03-12 08:15:24 -05:00
+++ b/storage/falcon/Record.cpp	2008-03-30 02:24:27 -05:00
@@ -480,7 +480,7 @@ bool Record::isVersion()
 }
 
 
-bool Record::scavenge(RecordScavenge *recordScavenge)
+bool Record::scavenge(RecordScavenge *recordScavenge, LockType lockType)
 {
 	return true;
 }
diff -Nrup a/storage/falcon/Record.h b/storage/falcon/Record.h
--- a/storage/falcon/Record.h	2008-03-12 07:15:13 -05:00
+++ b/storage/falcon/Record.h	2008-03-30 02:24:27 -05:00
@@ -29,6 +29,8 @@
 
 #define CHECK_RECORD_ACTIVITY
 
+#include "SynchronizationObject.h"
+
 enum RecordEncoding {
 	noEncoding = 0,
 	traditional,
@@ -73,7 +75,7 @@ public:
 	virtual int		getSavePointId();
 	virtual void	setSuperceded (bool flag);
 	virtual Record* fetchVersion (Transaction *transaction);
-	virtual bool	scavenge(RecordScavenge *recordScavenge);
+	virtual bool	scavenge(RecordScavenge *recordScavenge, LockType lockType);
 	virtual void	scavenge(TransId targetTransactionId, int oldestActiveSavePointId);
 	virtual bool	isVersion();
 	virtual bool	isSuperceded();
diff -Nrup a/storage/falcon/RecordLeaf.cpp b/storage/falcon/RecordLeaf.cpp
--- a/storage/falcon/RecordLeaf.cpp	2008-03-11 10:16:31 -05:00
+++ b/storage/falcon/RecordLeaf.cpp	2008-03-30 02:24:27 -05:00
@@ -122,6 +122,7 @@ int RecordLeaf::retireRecords (Table *ta
 	sync.lock(Shared);
 	
 	// Get a shared lock to find at least one record to scavenge
+	// If scavengeGeneration == UNDEFINED then just count the records in the leaf.
 	
 	for (ptr = records, end = records + RECORD_SLOTS; ptr < end; ++ptr)
 		{
@@ -129,10 +130,11 @@ int RecordLeaf::retireRecords (Table *ta
 		
 		if (record)
 			{
-			if (record->isVersion())
+			if (recordScavenge->scavengeGeneration == UNDEFINED)
+				++count;
+			else if (record->isVersion())
 				{
-				if ((record->scavenge(recordScavenge)) &&
-				    ((!record->hasRecord()) || ((record->useCount == 1) &&
(record->generation <= recordScavenge->scavengeGeneration))))
+				if (record->scavenge(recordScavenge, Shared))
 				    break;
 				else
 					++count;
@@ -161,8 +163,7 @@ int RecordLeaf::retireRecords (Table *ta
 			{
 			if (record->isVersion())
 				{
-				if ((record->scavenge(recordScavenge)) &&
-				    ((!record->hasRecord()) || ((record->useCount == 1) &&
(record->generation <= recordScavenge->scavengeGeneration))))
+				if (record->scavenge(recordScavenge, Exclusive))
 					{
 					*ptr = NULL;
 					recordScavenge->spaceReclaimed += record->size;
diff -Nrup a/storage/falcon/RecordScavenge.h b/storage/falcon/RecordScavenge.h
--- a/storage/falcon/RecordScavenge.h	2007-09-25 00:14:42 -05:00
+++ b/storage/falcon/RecordScavenge.h	2008-03-30 02:24:27 -05:00
@@ -17,6 +17,7 @@
 #define _RECORD_SCAVENGE_H_
 
 static const int AGE_GROUPS = 20;
+static const int UNDEFINED = -1;
 
 class Database;
 class Record;
diff -Nrup a/storage/falcon/RecordVersion.cpp b/storage/falcon/RecordVersion.cpp
--- a/storage/falcon/RecordVersion.cpp	2008-03-12 07:15:13 -05:00
+++ b/storage/falcon/RecordVersion.cpp	2008-03-30 02:24:27 -05:00
@@ -172,27 +172,63 @@ void RecordVersion::commit()
 }
 
 // Scavenge record versions by the scavenger thread.  Return true if the
-// record is a scavenge candidate
+// record or any prior version of the record is a scavenge candidate.
 
-bool RecordVersion::scavenge(RecordScavenge *recordScavenge)
+bool RecordVersion::scavenge(RecordScavenge *recordScavenge, LockType lockType)
 {
-	if (useCount != 1)
-		return false;
+	// Scavenge criteria:
+	// 
+	// 1. Use count == 1 AND
+	// 2. Record Version is older than the record version that was visible
+	//    to the oldest active transaction AND
+	// 3. Either the record generation is older than the current generation
+	//    OR there is no record data associated with the record version.
+
+	if (	useCount == 1
+		&& !transaction
+		&& transactionId < recordScavenge->transactionId
+		&& (!hasRecord()
+			|| generation <= recordScavenge->scavengeGeneration))
+		{
+		
+		// Expunge all record versions prior to this
 
-	if (transaction || (transactionId >= recordScavenge->transactionId))
+		if (priorVersion && lockType == Exclusive)
+			format->table->expungeRecordVersions(this, recordScavenge);
+			
+		return true;
+		}
+	else
 		{
+		 // Signal Table::cleanupRecords() that there is work to do
+		 
 		format->table->activeVersions = true;
 
-		if (priorVersion)
-			priorVersion->scavenge(recordScavenge);
-
-		return false;
+		// Scavenge criteria not met for this record, so check prior versions.
+		
+		if (priorVersion && recordScavenge->scavengeGeneration != UNDEFINED)
+			{
+			
+			// Scavenge prior record versions only if we have an exclusive lock on
+			// the record leaf. Return 'false' because the base record is not scavengable. 
+			
+			if (lockType == Exclusive)
+				priorVersion->scavenge(recordScavenge, lockType);
+			else
+
+				// Scan the prior record versions and return 'true' if a scavenge
+				// candidate is found.
+				
+				for (Record *rec = priorVersion; rec; rec = rec->getPriorVersion())
+					if (	rec->useCount == 1
+						&& !rec->getTransaction()
+						&& rec->getTransactionId() < recordScavenge->transactionId
+						&& (!rec->hasRecord()
+							|| rec->generation <= recordScavenge->scavengeGeneration))
+						return true;
+			}
 		}
-
-	if (priorVersion)
-		format->table->expungeRecordVersions(this, recordScavenge);
-
-	return true;
+		return false;
 }
 
 // Scavenge record versions replaced within a savepoint.
diff -Nrup a/storage/falcon/RecordVersion.h b/storage/falcon/RecordVersion.h
--- a/storage/falcon/RecordVersion.h	2008-03-12 07:15:13 -05:00
+++ b/storage/falcon/RecordVersion.h	2008-03-30 02:24:27 -05:00
@@ -41,7 +41,7 @@ public:
 	virtual void		setSuperceded (bool flag);
 	virtual Record*		getPriorVersion();
 	virtual Record*		getGCPriorVersion(void);
-	virtual bool		scavenge(RecordScavenge *recordScavenge);
+	virtual bool		scavenge(RecordScavenge *recordScavenge, LockType lockType);
 	virtual void		scavenge(TransId targetTransactionId, int oldestActiveSavePoint);
 	virtual bool		isVersion();
 	virtual Record*		rollback(Transaction *transaction);
diff -Nrup a/storage/falcon/Table.cpp b/storage/falcon/Table.cpp
--- a/storage/falcon/Table.cpp	2008-03-24 22:54:48 -05:00
+++ b/storage/falcon/Table.cpp	2008-03-30 02:24:27 -05:00
@@ -3617,8 +3617,10 @@ bool Table::validateUpdate(int32 recordN
 		
 		if (!transaction || transaction->state == Committed)
 			{
+#ifdef CHECK_RECORD_ACTIVITY
+			record->active = false;
+#endif
 			record->release();
-			
 			return false;
 			}