Fix deadlock issue with aggressive locking

HDFGroup · May 25, 2022 · c05819a · c05819a
1 parent 4085f05
commit c05819a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 4 deletions.
diff --git a/benchmarks/borg_bench.cc b/benchmarks/borg_bench.cc
@@ -75,7 +75,7 @@ int main(int argc, char *argv[]) {
       vbkt.Attach(&trait);
     }
 
-    const size_t kBlobSize = KILOBYTES(4);
+    const size_t kBlobSize = KILOBYTES(32);
     hapi::Blob blob(kBlobSize);
     std::iota(blob.begin(), blob.end(), 0);
 

diff --git a/src/buffer_organizer.cc b/src/buffer_organizer.cc
@@ -191,6 +191,10 @@ void BoMove(SharedMemoryContext *context, RpcContext *rpc,
           << std::endl;
   MetadataManager *mdm = GetMetadataManagerFromContext(context);
 
+  // TODO(chogan): This locking is too aggressive but I don't know how else to
+  // solve the deadlock that results when the following block of code is running
+  // after LocalDestroyBlobByName holds MetadataManager::bucket_mutex
+  BeginTicketMutex(&mdm->bucket_mutex);
   if (LocalLockBlob(context, blob_id)) {
     auto warning_string = [](BufferID id) {
       std::ostringstream ss;
@@ -240,6 +244,8 @@ void BoMove(SharedMemoryContext *context, RpcContext *rpc,
     }
 
     if (replacement_ids.size() > 0) {
+      // TODO(chogan): Only need to allocate a new BufferIdList if
+      // replacement.size > replaced.size
       std::vector<BufferID> buffer_ids = LocalGetBufferIdList(mdm, blob_id);
       using BufferIdSet = std::unordered_set<BufferID, BufferIdHash>;
       BufferIdSet new_buffer_ids(buffer_ids.begin(), buffer_ids.end());
@@ -293,6 +299,7 @@ void BoMove(SharedMemoryContext *context, RpcContext *rpc,
   } else {
     LOG(WARNING) << "Couldn't lock BlobID " << blob_id.as_int << "\n";
   }
+  EndTicketMutex(&mdm->bucket_mutex);
 }
 
 void LocalOrganizeBlob(SharedMemoryContext *context, RpcContext *rpc,

diff --git a/src/metadata_storage_stb_ds.cc b/src/metadata_storage_stb_ds.cc
@@ -419,14 +419,15 @@ i64 GetIndexOfId(MetadataManager *mdm, ChunkedIdList *id_list, u64 id) {
   return result;
 }
 
+/** Assumes MetadataManager::bucket_mutex is held by the caller. */
 void LocalReplaceBlobIdInBucket(SharedMemoryContext *context,
                                 BucketID bucket_id, BlobID old_blob_id,
                                 BlobID new_blob_id) {
   MetadataManager *mdm = GetMetadataManagerFromContext(context);
-  BeginTicketMutex(&mdm->bucket_mutex);
+  // BeginTicketMutex(&mdm->bucket_mutex);
   BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id);
 
-  if (info) {
+  if (info && info->active) {
     ChunkedIdList *blobs = &info->blobs;
 
     BlobID *blobs_arr = (BlobID *)GetIdsPtr(mdm, *blobs);
@@ -439,7 +440,7 @@ void LocalReplaceBlobIdInBucket(SharedMemoryContext *context,
     ReleaseIdsPtr(mdm);
   }
 
-  EndTicketMutex(&mdm->bucket_mutex);
+  // EndTicketMutex(&mdm->bucket_mutex);
 }
 
 void LocalAddBlobIdToBucket(MetadataManager *mdm, BucketID bucket_id,