RE: bzr commit into mysql-6.0-falcon:mysql-6.0-falcon branch (vvaintroub:2685) Bug#36396

[Kevin Lewis]

Vlad,

I do not like the 10 second wait inside TableSpaceManager::createTableSpace
for a drop to complete before creating a new one.  Is there a more
deterministic way of doing this?

>-----Original Message-----
>From: Vladislav Vaintroub [mailto:vvaintroub@stripped]
>Sent: Sunday, June 08, 2008 6:13 PM
>To: commits@stripped
>Subject: bzr commit into mysql-6.0-falcon:mysql-6.0-falcon branch
>(vvaintroub:2685) Bug#36396
>
>#At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-falcon/
>
> 2685 Vladislav Vaintroub	2008-06-09
>      Bug#36396 - Assertion in IO::pread (on closed tablespace file)
>      The problem was that gopher  processing "drop tablespace"
>      was faster then the gopher procesing preceeding "drop table".
>      As a result, tablespace file was closed and removed, while
>      "DROP TABLE" still tried to access it.
>
>      Solution is to synchronize gopher threads in this case, i.e
>      all while transaction containing DROP TABLESPACE is  being
>      processed, all other gophers wait until it completes.
>      Additionally, a race condition during  CREATE TABLESPACE has
>      been observed and fixed in this patch. It can happen that
>      DROP TABLESPACE was completed, but the file is still there
>      because commit record was not yet processed by a gopher thread.
>      This resulted into infrequent errors on CREATE with "file already
>      exists" message.
>
>      The solution to this problem is to track number of pending
>      DROP TABLESPACE commands.During CREATE TABLESPACE, if the file
>      exists and there are pending DROPs, wait for a short time (10 sec)
>      in a loop  for file to be deleted.
>
>      No testcase is provided with this patch. Had no luck writing
>      mysql-test-run script reproducing these race conditions.
>modified:
>  storage/falcon/Gopher.cpp
>  storage/falcon/SRLDropTableSpace.cpp
>  storage/falcon/SerialLog.h
>  storage/falcon/SerialLogTransaction.cpp
>  storage/falcon/SerialLogTransaction.h
>  storage/falcon/TableSpaceManager.cpp
>  storage/falcon/TableSpaceManager.h
>
>=== modified file 'storage/falcon/Gopher.cpp'
>--- a/storage/falcon/Gopher.cpp	2008-03-25 03:55:03 +0000
>+++ b/storage/falcon/Gopher.cpp	2008-06-08 22:12:35 +0000
>@@ -64,11 +64,22 @@ void Gopher::gopherThread(void)
>
> 		SerialLogTransaction *transaction = log->pending.first;
> 		log->pending.remove(transaction);
>+
>+
>+		Sync serializeGophers(&log->syncSerializeGophers,
>"Gopher::gopherThread(4)");
>+
>+		if (transaction->allowConcurrentGophers)
>+			serializeGophers.lock(Shared);
>+		else
>+			serializeGophers.lock(Exclusive);
>+
> 		sync.unlock();
>
> 		transaction->doAction();
>
> 		sync.lock(Exclusive);
>+		serializeGophers.unlock();
>+
> 		log->inactions.append(transaction);
>
> 		if (log->pending.count > log->maxTransactions &&
!log->blocking)
>
>=== modified file 'storage/falcon/SRLDropTableSpace.cpp'
>--- a/storage/falcon/SRLDropTableSpace.cpp	2007-11-14 23:24:47 +0000
>+++ b/storage/falcon/SRLDropTableSpace.cpp	2008-06-08 22:12:35 +0000
>@@ -24,6 +24,7 @@
> #include "SRLVersion.h"
> #include "SerialLogControl.h"
> #include "Transaction.h"
>+#include "SerialLogTransaction.h"
>
> //////////////////////////////////////////////////////////////////////
> // Construction/Destruction
>@@ -42,7 +43,12 @@ SRLDropTableSpace::~SRLDropTableSpace()
> void SRLDropTableSpace::append(TableSpace *tableSpace, Transaction
>*transaction)
> {
> 	START_RECORD(srlDropTableSpace, "SRLDropTableSpace::append");
>-	log->getTransaction(transaction->transactionId);
>+
>+	SerialLogTransaction* serialLogTransaction
>+		= log->getTransaction(transaction->transactionId);
>+
>+	serialLogTransaction->allowConcurrentGophers = false;
>+
> 	putInt(tableSpace->tableSpaceId);
> 	putInt(transaction->transactionId);
> }
>
>=== modified file 'storage/falcon/SerialLog.h'
>--- a/storage/falcon/SerialLog.h	2008-03-11 16:15:47 +0000
>+++ b/storage/falcon/SerialLog.h	2008-06-08 22:12:35 +0000
>@@ -188,6 +188,7 @@ public:
> 	SyncObject			syncSections;
> 	SyncObject			syncIndexes;
> 	SyncObject			syncGopher;
>+	SyncObject			syncSerializeGophers;
> 	SyncObject			syncUpdateStall;
> 	Stack				buffers;
> 	UCHAR				*bufferSpace;
>
>=== modified file 'storage/falcon/SerialLogTransaction.cpp'
>--- a/storage/falcon/SerialLogTransaction.cpp	2008-02-14 21:45:27 +0000
>+++ b/storage/falcon/SerialLogTransaction.cpp	2008-06-08 22:12:35 +0000
>@@ -46,6 +46,7 @@ SerialLogTransaction::SerialLogTransacti
> 	window = NULL;
> 	finished = false;
> 	ordered = false;
>+	allowConcurrentGophers = true;
> 	transaction = NULL;
> 	rolledBackSavepoints = NULL;
> 	blockNumber = maxBlockNumber = minBlockNumber = physicalBlockNumber
=
>0;
>
>=== modified file 'storage/falcon/SerialLogTransaction.h'
>--- a/storage/falcon/SerialLogTransaction.h	2008-03-11 16:15:47 +0000
>+++ b/storage/falcon/SerialLogTransaction.h	2008-06-08 22:12:35 +0000
>@@ -71,6 +71,7 @@ public:
> 	int						xidLength;
> 	UCHAR					*xid;
> 	bool					finished;
>+	bool					allowConcurrentGophers;
>
> 	SerialLog				*log;
> 	SerialLogTransaction	*next;
>
>=== modified file 'storage/falcon/TableSpaceManager.cpp'
>--- a/storage/falcon/TableSpaceManager.cpp	2008-05-06 23:37:54 +0000
>+++ b/storage/falcon/TableSpaceManager.cpp	2008-06-08 22:12:35 +0000
>@@ -41,6 +41,7 @@
> #include "SRLDropTableSpace.h"
> #include "Log.h"
> #include "InfoTable.h"
>+#include "Thread.h"
>
> #ifdef _DEBUG
> #undef THIS_FILE
>@@ -57,6 +58,7 @@ TableSpaceManager::TableSpaceManager(Dat
> 	memset(nameHash, 0, sizeof(nameHash));
> 	memset(idHash, 0, sizeof(nameHash));
> 	tableSpaces = NULL;
>+	pendingDrops = 0;
> }
>
> TableSpaceManager::~TableSpaceManager()
>@@ -168,10 +170,32 @@ TableSpace* TableSpaceManager::createTab
>
> 	TableSpace *tableSpace = new TableSpace(database, name, id,
fileName,
>type, tsInit);
>
>-	if (!repository && tableSpace->dbb->doesFileExist(fileName))
>+	if (!repository)
> 		{
>-		delete tableSpace;
>-		throw SQLError(TABLESPACE_DATAFILE_EXIST_ERROR, "table space
>file name \"%s\" already exists\n", fileName);
>+		bool fileExists;
>+
>+		// Check if table space file already exists.
>+		// Take into account, that tablespace might have been
already
>dropped
>+		// by another transaction, yet file can still be present on
the
>disk,
>+		// if log record is not yet fully committed by the gopher
>thread).
>+		// So we'll wait for a few seconds if there are pending
drops
>and
>+		// tablespace file exists.
>+
>+		for (int i=0; i < 10; i++)
>+			{
>+			fileExists =
tableSpace->dbb->doesFileExist(fileName);
>+
>+			if (fileExists && pendingDrops > 0)
>+
>
Thread::getThread("TableSpaceManager::createTableSpace")->sleep(1000);
>+			else
>+				break;
>+			}
>+
>+		if (fileExists)
>+			{
>+			delete tableSpace;
>+			throw SQLError(TABLESPACE_DATAFILE_EXIST_ERROR,
"table
>space file name \"%s\" already exists\n", fileName);
>+			}
> 		}
>
> 	try
>@@ -302,6 +326,7 @@ void TableSpaceManager::dropTableSpace(T
> 	statement->executeUpdate();
> 	Transaction *transaction = database->getSystemTransaction();
> 	transaction->hasUpdates = true;
>+	pendingDrops++;
> 	database->serialLog->logControl->dropTableSpace.append(tableSpace,
>transaction);
>
> 	syncDDL.unlock();
>@@ -386,6 +411,9 @@ void TableSpaceManager::expungeTableSpac
> 	sync.unlock();
> 	tableSpace->dropTableSpace();
> 	delete tableSpace;
>+
>+	sync.lock(Exclusive);
>+	if(pendingDrops >0) pendingDrops--;
> }
>
> void TableSpaceManager::reportWrites(void)
>
>=== modified file 'storage/falcon/TableSpaceManager.h'
>--- a/storage/falcon/TableSpaceManager.h	2008-04-05 22:09:17 +0000
>+++ b/storage/falcon/TableSpaceManager.h	2008-06-08 22:12:35 +0000
>@@ -69,6 +69,7 @@ public:
> 	TableSpace	*idHash[TS_HASH_SIZE];
> 	SyncObject	syncObject;
> 	void postRecovery(void);
>+	int pendingDrops;
> };
>
> #endif //
>!defined(AFX_TABLESPACEMANAGER_H__BD1D39F6_2201_4136_899C_7CB106E99B8C__INC
L
>UDED_)
>
>
>--
>MySQL Code Commits Mailing List
>For list archives: http://lists.mysql.com/commits
>To unsubscribe:    http://lists.mysql.com/commits?unsub=1