bzr commit into mysql-6.0-falcon-team branch (cpowers:2845) Bug#39711 Bug#39795 Bug#39796

[Christopher Powers]

#At file:///home/cpowers/work/dev/dev-07/mysql/

 2845 Christopher Powers	2008-10-01
      Bug#39711 "Running falcon_bug_34351_C shows increasing memory usage"
      Bug#39795 "Falcon: Online add index does not support index with non-null columns"
      Bug#39796 "Falcon: Reference count decrement not atomic"
modified:
  storage/falcon/DeferredIndex.cpp
  storage/falcon/DeferredIndex.h
  storage/falcon/DeferredIndexWalker.cpp
  storage/falcon/Index.cpp
  storage/falcon/Transaction.cpp
  storage/falcon/Transaction.h
  storage/falcon/WalkDeferred.cpp
  storage/falcon/ha_falcon.cpp

per-file messages:
  storage/falcon/DeferredIndex.cpp
    Renamed ::releaseRef() to ::release() to be consistent with other classes
    Corrected refcount atomicity in ::release()
  storage/falcon/DeferredIndex.h
    Renamed ::releaseRef() to ::release() to be consistent with other classes
  storage/falcon/DeferredIndexWalker.cpp
    Renamed DeferredIndex::releaseRef() to ::release() to be consistent with other classes
  storage/falcon/Index.cpp
    Added deferredIndex->release() to Index::detachDeferredIndex()
    Note that this is the correct location for release(), but it is temporarily disabled
until the DeferredIndex refcount analysis is complete.
  storage/falcon/Transaction.cpp
    Disabled DeferredIndex::addRef() in Transaction::add(DefererredIndex*) to avoid
DeferredIndex memory leak. Note that this is an intermediate solution.
    Renamed DeferredIndex::releaseRef() to ::release() to be consistent with other classes
    Corrected refcount in ::release() to ensure atomicity
  storage/falcon/Transaction.h
    Declared Transaction::release() as void
  storage/falcon/WalkDeferred.cpp
    Renamed DeferredIndex::releaseRef() to ::release() to be consistent with other classes
    Added comment to WalkDefered constructor indicating where DeferredIndex::addRef()
should be located once refcount analysis is complete.
  storage/falcon/ha_falcon.cpp
    Remove nullable column and primary key restrictions for online index creation in
StorageInterface::check_if_supported_alter()
=== modified file 'storage/falcon/DeferredIndex.cpp'
--- a/storage/falcon/DeferredIndex.cpp	2008-09-10 19:51:03 +0000
+++ b/storage/falcon/DeferredIndex.cpp	2008-10-02 00:09:30 +0000
@@ -840,7 +840,7 @@ void DeferredIndex::detachTransaction(vo
 	else
 		sync.unlock();
 
-	//releaseRef();
+	//release();
 }
 
 void DeferredIndex::chill(Dbb *dbb)
@@ -882,13 +882,9 @@ void DeferredIndex::addRef()
 	INTERLOCKED_INCREMENT (useCount);
 }
 
-void DeferredIndex::releaseRef()
+void DeferredIndex::release()
 {
-	ASSERT(useCount > 0);
-	
-	INTERLOCKED_DECREMENT(useCount);
-
-	if (useCount == 0)
+	if (INTERLOCKED_DECREMENT(useCount) == 0)
 		delete this;
 }
 

=== modified file 'storage/falcon/DeferredIndex.h'
--- a/storage/falcon/DeferredIndex.h	2008-07-25 18:07:24 +0000
+++ b/storage/falcon/DeferredIndex.h	2008-10-02 00:09:30 +0000
@@ -125,7 +125,7 @@ public:
 	uint64			virtualOffsetAtEnd;
 	SerialLogWindow	*window;
 	void			addRef();
-	void			releaseRef();
+	void			release();
 };
 
 #endif

=== modified file 'storage/falcon/DeferredIndexWalker.cpp'
--- a/storage/falcon/DeferredIndexWalker.cpp	2008-09-10 19:51:03 +0000
+++ b/storage/falcon/DeferredIndexWalker.cpp	2008-10-02 00:09:30 +0000
@@ -96,7 +96,7 @@ void DeferredIndexWalker::initialize(Def
 DeferredIndexWalker::~DeferredIndexWalker(void)
 {
 	if (deferredIndex)
-		deferredIndex->releaseRef();
+		deferredIndex->release();
 }
 
 DINode* DeferredIndexWalker::next(void)

=== modified file 'storage/falcon/Index.cpp'
--- a/storage/falcon/Index.cpp	2008-08-19 03:33:01 +0000
+++ b/storage/falcon/Index.cpp	2008-10-02 00:09:30 +0000
@@ -514,7 +514,7 @@ IndexWalker* Index::positionIndex(IndexK
 			if (transaction->visible(deferredIndex->transaction,
deferredIndex->transactionId, FOR_WRITING))
 				{
 				deferredIndex->addRef();
-
+				
 				if (!indexWalker)
 					{
 					indexWalker = new IndexWalker(this, transaction, searchFlags);
@@ -843,6 +843,7 @@ void Index::detachDeferredIndex(Deferred
 	Sync sync(&deferredIndexes.syncObject, "Index::detachDeferredIndex(1)");
 	sync.lock(Exclusive);
 	deferredIndexes.remove(deferredIndex);
+//	deferredIndex->release(); // temp disabled for Bug#39711, still causes crash
 	sync.unlock();
 
 	if (   (database->configuration->useDeferredIndexHash)
@@ -861,7 +862,6 @@ void Index::detachDeferredIndex(Deferred
 			removeFromDIHash(uniqueNode);
 			}
 		}
-
 }
 
 int Index::getRootPage(void)

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2008-09-10 19:51:03 +0000
+++ b/storage/falcon/Transaction.cpp	2008-10-02 00:09:30 +0000
@@ -1076,14 +1076,10 @@ void Transaction::addRef()
 	INTERLOCKED_INCREMENT(useCount);
 }
 
-int Transaction::release()
+void Transaction::release()
 {
-	int count = INTERLOCKED_DECREMENT(useCount);
-
-	if (count == 0)
+	if (INTERLOCKED_DECREMENT(useCount) == 0)
 		delete this;
-
-	return count;
 }
 
 int Transaction::createSavepoint()
@@ -1317,7 +1313,7 @@ void Transaction::add(DeferredIndex* def
 	Sync sync(&syncDeferredIndexes, "Transaction::add");
 	sync.lock(Exclusive);
 
-	deferredIndex->addRef();
+//	deferredIndex->addRef(); // temp disable for Bug#39711 memory leak (see
Index::detachDeferredIndex)
 	deferredIndex->nextInTransaction = deferredIndexes;
 	deferredIndexes = deferredIndex;
 	deferredIndexCount++;
@@ -1504,7 +1500,7 @@ void Transaction::releaseDeferredIndexes
 		ASSERT(deferredIndex->transaction == this);
 		deferredIndexes = deferredIndex->nextInTransaction;
 		deferredIndex->detachTransaction();
-		deferredIndex->releaseRef();
+		deferredIndex->release();
 		deferredIndexCount--;
 		}
 }
@@ -1520,7 +1516,7 @@ void Transaction::releaseDeferredIndexes
 			{
 			*ptr = deferredIndex->nextInTransaction;
 			deferredIndex->detachTransaction();
-			deferredIndex->releaseRef();
+			deferredIndex->release();
 			--deferredIndexCount;
 			}
 		else

=== modified file 'storage/falcon/Transaction.h'
--- a/storage/falcon/Transaction.h	2008-09-10 04:02:07 +0000
+++ b/storage/falcon/Transaction.h	2008-10-02 00:09:30 +0000
@@ -99,7 +99,7 @@ public:
 	void		prepare(int xidLength, const UCHAR *xid);
 	void		rollback();
 	void		commit();
-	int			release();
+	void		release();
 	void		addRef();
 	void		waitForTransaction();
 	bool		waitForTransaction (TransId transId);

=== modified file 'storage/falcon/WalkDeferred.cpp'
--- a/storage/falcon/WalkDeferred.cpp	2008-07-25 18:07:24 +0000
+++ b/storage/falcon/WalkDeferred.cpp	2008-10-02 00:09:30 +0000
@@ -19,6 +19,7 @@
 WalkDeferred::WalkDeferred(DeferredIndex *deferredIdx, Transaction *transaction, int
flags, IndexKey *lower, IndexKey *upper) 
 	: IndexWalker(deferredIdx->index, transaction, flags)
 {
+	//deferredIdx->addRef();  // assume addref by caller
 	deferredIndex = deferredIdx;
 	walker.initialize(deferredIndex, lower, flags);
 	node = NULL;
@@ -26,7 +27,7 @@ WalkDeferred::WalkDeferred(DeferredIndex
 
 WalkDeferred::~WalkDeferred(void)
 {
-	deferredIndex->releaseRef();
+	deferredIndex->release();
 }
 
 Record* WalkDeferred::getNext(bool lockForUpdate)

=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp	2008-09-16 17:58:49 +0000
+++ b/storage/falcon/ha_falcon.cpp	2008-10-02 00:09:30 +0000
@@ -2186,34 +2186,6 @@ int StorageInterface::check_if_supported
 	
 	if (alter_flags->is_set(HA_ADD_INDEX) || alter_flags->is_set(HA_ADD_UNIQUE_INDEX))
 		{
-		for (unsigned int n = 0; n < altered_table->s->keys; n++)
-			{
-			if (n != altered_table->s->primary_key)
-				{
-				KEY *key = altered_table->key_info + n;
-				KEY *tableEnd = table->key_info + table->s->keys;
-				KEY *tableKey;
-				
-				// Determine if this is a new index
-
-				for (tableKey = table->key_info; tableKey < tableEnd; tableKey++)
-					if (!strcmp(tableKey->name, key->name))
-						break;
-				
-				// Verify that each part is nullable
-				
-				if (tableKey >= tableEnd)
-					for (uint p = 0; p < key->key_parts; p++)
-						{
-						KEY_PART_INFO *keyPart = key->key_part + p;
-						if (keyPart && !keyPart->field->real_maybe_null())
-							{
-							DBUG_PRINT("info",("Online add index columns must be nullable"));
-							DBUG_RETURN(HA_ALTER_NOT_SUPPORTED);
-							}
-						}
-				}
-			}
 		}
 		
 	if (alter_flags->is_set(HA_DROP_INDEX) ||
alter_flags->is_set(HA_DROP_UNIQUE_INDEX))
@@ -2307,20 +2279,17 @@ int StorageInterface::addIndex(THD* thd,
 
 	for (unsigned int n = 0; n < alteredTable->s->keys; n++)
 		{
-		if (n != alteredTable->s->primary_key)
-			{
-			KEY *key = alteredTable->key_info + n;
-			KEY *tableEnd = table->key_info + table->s->keys;
-			KEY *tableKey;
+		KEY *key = alteredTable->key_info + n;
+		KEY *tableEnd = table->key_info + table->s->keys;
+		KEY *tableKey;
 			
-			for (tableKey = table->key_info; tableKey < tableEnd; tableKey++)
-				if (!strcmp(tableKey->name, key->name))
-					break;
+		for (tableKey = table->key_info; tableKey < tableEnd; tableKey++)
+			if (!strcmp(tableKey->name, key->name))
+				break;
 					
-			if (tableKey >= tableEnd)
-				if ((ret = createIndex(schemaName, tableName, alteredTable, n)))
-					break;
-			}
+		if (tableKey >= tableEnd)
+			if ((ret = createIndex(schemaName, tableName, alteredTable, n)))
+				break;
 		}
 		
 	// The server indexes may have been reordered, so remap to the Falcon indexes
@@ -2349,20 +2318,17 @@ int StorageInterface::dropIndex(THD* thd
 	
 	for (unsigned int n = 0; n < table->s->keys; n++)
 		{
-		if (n != table->s->primary_key)
-				{
-			KEY *key = table->key_info + n;
-			KEY *alterEnd = alteredTable->key_info + alteredTable->s->keys;
-			KEY *alterKey;
+		KEY *key = table->key_info + n;
+		KEY *alterEnd = alteredTable->key_info + alteredTable->s->keys;
+		KEY *alterKey;
 			
-			for (alterKey = alteredTable->key_info; alterKey < alterEnd; alterKey++)
-				if (!strcmp(alterKey->name, key->name))
-					break;
-
-			if (alterKey >= alterEnd)
-				if ((ret = dropIndex(schemaName, tableName, table, n)))
-					break;
-				}
+		for (alterKey = alteredTable->key_info; alterKey < alterEnd; alterKey++)
+			if (!strcmp(alterKey->name, key->name))
+				break;
+
+		if (alterKey >= alterEnd)
+			if ((ret = dropIndex(schemaName, tableName, table, n)))
+				break;
 		}
 	
 	// The server indexes have been reordered, so remap to the Falcon indexes