RE: bzr commit into mysql-6.0-falcon branch (klewis:2740) Bug#37587

[Vladislav Vaintroub]

Hi Kevin,
from what I  can understand, the  implements a kind of special purpose
read/write lock, instead of SyncObject I used previously.
That's fine (probably, I have not figured out flaws yet), except for this
The variable access/modification of wantToSerializeGophers  and
serializeGophers should be interlocked, as many threads are involved.

Vlad

> -----Original Message-----
> From: Kevin Lewis [mailto:klewis@stripped]
> Sent: Wednesday, July 09, 2008 9:24 PM
> To: commits@stripped
> Subject: bzr commit into mysql-6.0-falcon branch (klewis:2740)
> Bug#37587
> 
> #At file:///C:/Work/bzr/Merge/mysql-6.0-falcon/
> 
>  2740 Kevin Lewis	2008-07-09
>       Bug#37587 - Try a new method of serializing gopher threads
>       for DropTableSpace.  Instead of holding a lock during the
>       action.  Set a lock variable while holding syncPending
> exclusively
>       and do a loop of release-sleep(10)-lock until the other gopher
>       threads are fully done with previous transactions.
> 
>       Also, reduce the possibility of a deadlock by not holding
>       TableSpaceManager::syncObject while calling
>       SRLDropTableSpace::append(), which locks
>       SerialLog::pending.syncObject.
> modified:
>   storage/falcon/Gopher.cpp
>   storage/falcon/Gopher.h
>   storage/falcon/SerialLog.cpp
>   storage/falcon/SerialLog.h
>   storage/falcon/TableSpaceManager.cpp
> 
> per-file messages:
>   storage/falcon/Gopher.cpp
>     Bug#37587 - Try a new method of serializing gopher threads
>     for DropTableSpace.  Instead of holding a lock during the
>     action.  Set a lock variable while holding syncPending exclusively
>     and do a loop of release-sleep(10)-lock until the other gopher
>     threads are fully done with previous transactions.
>     * rename sync to syncPending
>     * Introduce setConcurrency and releaseConcurrency
>     * Delete SyncObject SerialLog::serializeGophers
>     * Add and use these integers;
>         SerialLog::wantToSerializeGophers
>         SerialLog::serializeGophers
>   storage/falcon/Gopher.h
>     Bug#37587 - Try a new method of serializing gopher threads
>     for DropTableSpace.
>     * Introduce setConcurrency and releaseConcurrency
>   storage/falcon/SerialLog.cpp
>     Bug#37587 - Try a new method of serializing gopher threads
>     for DropTableSpace.
>     * Add and use these integers;
>         SerialLog::wantToSerializeGophers
>         SerialLog::serializeGophers
>   storage/falcon/SerialLog.h
>     Bug#37587 - Try a new method of serializing gopher threads
>     for DropTableSpace.
>     * Delete SyncObject SerialLog::serializeGophers
>     * Add and use these integers;
>         SerialLog::wantToSerializeGophers
>         SerialLog::serializeGophers
>   storage/falcon/TableSpaceManager.cpp
>     Bug#37587 - Try a new method of serializing gopher threads
>     for DropTableSpace.
>     *  Reduce the possibility of a deadlock by not holding
>        TableSpaceManager::syncObject while calling
>        SRLDropTableSpace::append(), which locks
>        SerialLog::pending.syncObject.
> === modified file 'storage/falcon/Gopher.cpp'
> --- a/storage/falcon/Gopher.cpp	2008-06-30 16:51:55 +0000
> +++ b/storage/falcon/Gopher.cpp	2008-07-09 19:23:23 +0000
> @@ -43,8 +43,8 @@ void Gopher::gopherThread(void)
>  	deadMan.lock(Shared);
>  	workerThread = Thread::getThread("Gopher::gopherThread");
>  	active = true;
> -	Sync sync (&log->pending.syncObject, "Gopher::gopherThread
> pending");
> -	sync.lock(Exclusive);
> +	Sync syncPending (&log->pending.syncObject, "Gopher::gopherThread
> pending");
> +	syncPending.lock(Exclusive);
> 
>  	while (!workerThread->shutdownInProgress && !log->finishing)
>  		{
> @@ -53,29 +53,25 @@ void Gopher::gopherThread(void)
>  			if (log->blocking)
>  				log->unblockUpdates();
> 
> -			sync.unlock();
> +			syncPending.unlock();
>  			active = false;
>  			workerThread->sleep();
>  			active = true;
> -			sync.lock(Exclusive);
> +			syncPending.lock(Exclusive);
> 
>  			continue;
>  			}
> 
>  		SerialLogTransaction *transaction = log->pending.first;
>  		log->pending.remove(transaction);
> -		sync.unlock();
> 
> -		Sync serializeGophers(&log->syncSerializeGophers,
> "Gopher::gopherThread(4)");
> -		if (transaction->allowConcurrentGophers)
> -			serializeGophers.lock(Shared);
> -		else
> -			serializeGophers.lock(Exclusive);
> +		setConcurrency(&syncPending, transaction-
> >allowConcurrentGophers);
> +		syncPending.unlock();
> 
>  		transaction->doAction();
> 
> -		sync.lock(Exclusive);
> -		serializeGophers.unlock();
> +		syncPending.lock(Exclusive);
> +		releaseConcurrency(&syncPending, transaction-
> >allowConcurrentGophers);
> 
>  		log->inactions.append(transaction);
> 
> @@ -87,6 +83,60 @@ void Gopher::gopherThread(void)
>  	workerThread = NULL;
>  }
> 
> +void Gopher::setConcurrency(Sync *syncPending, bool
> allowConcurrentGophers)
> +{
> +	// Assume that syncPending is locked exclusively.
> +
> +	if (allowConcurrentGophers)
> +		{
> +		while (log->serializeGophers < 0)
> +			{
> +			syncPending->unlock();
> +			workerThread->sleep(10);
> +			syncPending->lock(Exclusive);
> +			}
> +
> +		log->serializeGophers++;
> +		}
> +	else
> +		{
> +		log->wantToSerializeGophers++;
> +		while (log->serializeGophers)
> +			{
> +			syncPending->unlock();
> +			workerThread->sleep(10);
> +			syncPending->lock(Exclusive);
> +			}
> +
> +		log->serializeGophers = -1;
> +		}
> +}
> +
> +void Gopher::releaseConcurrency(Sync *syncPending, bool
> allowConcurrentGophers)
> +{
> +	if (allowConcurrentGophers)
> +		{
> +		ASSERT(log->serializeGophers > 0);
> +		log->serializeGophers--;
> +		}
> +	else
> +		{
> +		ASSERT(log->serializeGophers == -1);
> +		log->wantToSerializeGophers--;
> +		log->serializeGophers = 0;
> +		}
> +
> +	// If there is another thread that needs to serialize the
> gophers,
> +	// wait here until it is done.
> +
> +	while (log->wantToSerializeGophers)
> +		{
> +		syncPending->unlock();
> +		workerThread->sleep(10);
> +		syncPending->lock(Exclusive);
> +		}
> +}
> +
>  void Gopher::start(void)
>  {
>  	log->database->threads->start("SerialLog::start", gopherThread,
> this);
> 
> === modified file 'storage/falcon/Gopher.h'
> --- a/storage/falcon/Gopher.h	2007-10-25 18:10:34 +0000
> +++ b/storage/falcon/Gopher.h	2008-07-09 19:23:23 +0000
> @@ -28,10 +28,12 @@ public:
>  	~Gopher(void);
> 
>  	void		gopherThread(void);
> +	void		setConcurrency(Sync *syncPending, bool
> allowConcurrentGophers);
> +	void		releaseConcurrency(Sync *syncPending, bool
> allowConcurrentGophers);
>  	void		start(void);
>  	void		shutdown(void);
>  	void		wakeup(void);
> -
> +
>  	static void gopherThread(void* arg);
> 
>  	SerialLog	*log;
> 
> === modified file 'storage/falcon/SerialLog.cpp'
> --- a/storage/falcon/SerialLog.cpp	2008-06-30 16:58:45 +0000
> +++ b/storage/falcon/SerialLog.cpp	2008-07-09 19:23:23 +0000
> @@ -127,6 +127,8 @@ SerialLog::SerialLog(Database *db, JStri
>  	syncUpdateStall.setName("SerialLog::syncUpdateStall");
>  	pending.syncObject.setName("SerialLog::pending transactions");
>  	gophers = NULL;
> +	wantToSerializeGophers = 0;
> +	serializeGophers = 0;
> 
>  	for (uint n = 0; n < falcon_gopher_threads; ++n)
>  		{
> 
> === modified file 'storage/falcon/SerialLog.h'
> --- a/storage/falcon/SerialLog.h	2008-06-08 22:12:35 +0000
> +++ b/storage/falcon/SerialLog.h	2008-07-09 19:23:23 +0000
> @@ -188,7 +188,6 @@ public:
>  	SyncObject			syncSections;
>  	SyncObject			syncIndexes;
>  	SyncObject			syncGopher;
> -	SyncObject			syncSerializeGophers;
>  	SyncObject			syncUpdateStall;
>  	Stack				buffers;
>  	UCHAR				*bufferSpace;
> @@ -222,7 +221,9 @@ public:
>  	int32				traceRecord;
>  	uint32				chilledRecords;
>  	uint64				chilledBytes;
> -
> +	int32				wantToSerializeGophers;
> +	int32				serializeGophers;
> +
>  	TableSpaceInfo		*tableSpaces[SLT_HASH_SIZE];
>  	TableSpaceInfo		*tableSpaceInfo;
>  	SerialLogTransaction		*earliest;
> 
> === modified file 'storage/falcon/TableSpaceManager.cpp'
> --- a/storage/falcon/TableSpaceManager.cpp	2008-06-17 17:41:54
> +0000
> +++ b/storage/falcon/TableSpaceManager.cpp	2008-07-09 19:23:23
> +0000
> @@ -326,11 +326,8 @@ void TableSpaceManager::dropTableSpace(T
>  	statement->executeUpdate();
>  	Transaction *transaction = database->getSystemTransaction();
>  	transaction->hasUpdates = true;
> -	pendingDrops++;
> -	database->serialLog->logControl-
> >dropTableSpace.append(tableSpace, transaction);
> 
>  	syncDDL.unlock();
> -	database->commitSystemTransaction();
> 
>  	int slot = tableSpace->name.hash(TS_HASH_SIZE);
> 
> @@ -342,7 +339,12 @@ void TableSpaceManager::dropTableSpace(T
>  			break;
>  			}
> 
> +	pendingDrops++;
>  	syncObj.unlock();
> +
> +	database->serialLog->logControl-
> >dropTableSpace.append(tableSpace, transaction);
> +	database->commitSystemTransaction();
> +
>  	tableSpace->active = false;
>  }
> 
> 
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1