Vlad, before you push this, we need to measure the performance cost in a
high concurrency test.
Hakan and/or Kelly.
Can one of you try out Vlad's patch here to see what affect it has on DBT2?
The new RecoveryObjects::syncArray of SyncObjects is a concern expressed by
Jim. That email went to dev_falcon, so I added it as a comment to the bug
report.
>-----Original Message-----
>From: Vladislav Vaintroub [mailto:vvaintroub@stripped]
>Sent: Friday, July 18, 2008 4:25 PM
>To: commits@stripped
>Subject: bzr commit into mysql-6.0-falcon branch (vvaintroub:2750)
Bug#35072
>
>#At file:///C:/bzr/mysql-6.0-falcon-team/
>
> 2750 Vladislav Vaintroub 2008-07-18
> Bug#35072: Crash Falcon crash in RecoveryObjects::findRecoveryObject
> Problem: The functiion crashed while traversing single linked list
> that contained a dangling pointer (deleted memory, filled with 0xe
>bytes).
> Access and modification of the hash table was not synchronized.
> When one thread adds a new entry to the hashtable while another
> is delets from the same hash bucket, hashtable can become corrupted
> (i.e still have pointer to the deleted object)
>
> Solution is to synchronize hashtable access. Here, a reader/writer
> lock is used, one per hash bucket.
>modified:
> storage/falcon/RecoveryObjects.cpp
> storage/falcon/RecoveryObjects.h
>
>per-file messages:
> storage/falcon/RecoveryObjects.cpp
> Add synchronization to for hashtable access
> storage/falcon/RecoveryObjects.h
> Add synchronization for hashtable access
>=== modified file 'storage/falcon/RecoveryObjects.cpp'
>--- a/storage/falcon/RecoveryObjects.cpp 2007-09-20 15:44:25 +0000
>+++ b/storage/falcon/RecoveryObjects.cpp 2008-07-18 21:24:52 +0000
>@@ -23,6 +23,8 @@
> #include "RecoveryObjects.h"
> #include "RecoveryPage.h"
> #include "SerialLog.h"
>+#include "SyncObject.h"
>+#include "Sync.h"
>
> #ifdef _DEBUG
> #undef THIS_FILE
>@@ -95,17 +97,24 @@ bool RecoveryObjects::isObjectActive(int
> return object->pass1Count == object->currentCount;
> }
>
>-RecoveryPage* RecoveryObjects::findRecoveryObject(int objectNumber, int
>tableSpaceId)
>-{
>- int slot = objectNumber % RPG_HASH_SIZE;
>
>- for (RecoveryPage *object = recoveryObjects[slot]; object; object =
>object->collision)
>+static inline RecoveryPage * findInHashBucket(RecoveryPage *head, int
>objectNumber, int tableSpaceId)
>+{
>+ for (RecoveryPage *object = head ; object; object =
object->collision)
> if (object->objectNumber == objectNumber &&
object->tableSpaceId
>== tableSpaceId)
> return object;
>-
> return NULL;
> }
>
>+
>+RecoveryPage* RecoveryObjects::findRecoveryObject(int objectNumber, int
>tableSpaceId)
>+{
>+ int slot = objectNumber % RPG_HASH_SIZE;
>+ Sync sync(&syncArray[slot], "RecoveryObjects::findRecoveryObject");
>+ sync.lock(Shared);
>+ return findInHashBucket(recoveryObjects[slot], objectNumber,
>tableSpaceId);
>+}
>+
> void RecoveryObjects::setActive(int objectNumber, int tableSpaceId)
> {
> RecoveryPage *object = findRecoveryObject(objectNumber,
tableSpaceId);
>@@ -129,10 +138,25 @@ RecoveryPage* RecoveryObjects::getRecove
> int slot = objectNumber % RPG_HASH_SIZE;
> RecoveryPage *object;
>
>- for (object = recoveryObjects[slot]; object; object = object-
>>collision)
>- if (object->objectNumber == objectNumber &&
object->tableSpaceId
>== tableSpaceId)
>- return object;
>+ Sync sync(&syncArray[slot], "RecoveryObjects::getRecoveryObject");
>+ sync.lock(Shared);
>+ object = findInHashBucket(recoveryObjects[slot], objectNumber,
>tableSpaceId);
>+
>+ if(object)
>+ return object;
>+
>+ // Object not found, insert (need exlusive lock for this)
>+ sync.unlock();
>+ sync.lock(Exclusive);
>+
>+ // We need to traverse the collision list once again. Another thread
>+ // may have inserted the entry while current thread was waiting
>+ // for exclusive lock.
>+ object = findInHashBucket(recoveryObjects[slot], objectNumber,
>tableSpaceId);
>+ if (object)
>+ return object;
>
>+ // Add object to the start of the collision list
> object = new RecoveryPage(objectNumber, tableSpaceId);
> object->collision = recoveryObjects[slot];
> recoveryObjects[slot] = object;
>@@ -143,7 +167,9 @@ RecoveryPage* RecoveryObjects::getRecove
> void RecoveryObjects::deleteObject(int objectNumber, int tableSpaceId)
> {
> int slot = objectNumber % RPG_HASH_SIZE;
>-
>+ Sync sync(&syncArray[slot], "RecoveryObjects::deleteObject");
>+ sync.lock(Exclusive);
>+
> for (RecoveryPage **ptr = recoveryObjects + slot, *object; (object =
>*ptr); ptr = &object->collision)
> if (object->objectNumber == objectNumber &&
object->tableSpaceId
>== tableSpaceId)
> {
>
>=== modified file 'storage/falcon/RecoveryObjects.h'
>--- a/storage/falcon/RecoveryObjects.h 2007-09-20 15:44:25 +0000
>+++ b/storage/falcon/RecoveryObjects.h 2008-07-18 21:24:52 +0000
>@@ -26,6 +26,7 @@
>
> static const int RPG_HASH_SIZE = 101;
>
>+#include "SyncObject.h"
> class RecoveryPage;
> class SerialLog;
>
>@@ -48,6 +49,7 @@ public:
>
> SerialLog *serialLog;
> RecoveryPage *recoveryObjects[RPG_HASH_SIZE];
>+ SyncObject syncArray[RPG_HASH_SIZE];
> };
>
> #endif //
>!defined(AFX_RECOVERYOBJECTS_H__00C7CE5F_3C33_435C_9521_9C274CAB0581__INCLU
D
>ED_)
>
>
>--
>MySQL Code Commits Mailing List
>For list archives: http://lists.mysql.com/commits
>To unsubscribe: http://lists.mysql.com/commits?unsub=1
