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

[Kevin Lewis]

>The variable access/modification of wantToSerializeGophers  and
>serializeGophers should be interlocked, as many threads are involved.

No only one thread is involved.  It is only accessed while one of the gopher
threads has an exclusive lock on SerialLog::pending.syncObject.



>-----Original Message-----
>From: Vladislav Vaintroub [mailto:vaintroub@stripped]
>Sent: Wednesday, July 09, 2008 4:37 PM
>To: 'Kevin Lewis'; commits@stripped
>Subject: RE: bzr commit into mysql-6.0-falcon branch (klewis:2740)
Bug#37587
>
>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