From: Date: July 9 2008 11:36pm Subject: RE: bzr commit into mysql-6.0-falcon branch (klewis:2740) Bug#37587 List-Archive: http://lists.mysql.com/commits/49370 Message-Id: <002f01c8e20b$e2b6af40$a8240dc0$@com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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=vaintroub@stripped