diff --git a/src/api/bucket.h b/src/api/bucket.h index a2335fef8..69bbcee1c 100644 --- a/src/api/bucket.h +++ b/src/api/bucket.h @@ -152,6 +152,8 @@ Status Bucket::PlaceBlobs(std::vector &schemas, hermes::Blob blob = {}; blob.data = (u8 *)blobs[i].data(); blob.size = blobs[i].size() * sizeof(T); + LOG(INFO) << "Attaching blob '" << names[i] << "' to Bucket '" << name_ + << "'" << std::endl; // TODO(chogan): @errorhandling What about partial failure? result = PlaceBlob(&hermes_->context_, &hermes_->rpc_, schema, blob, names[i].c_str(), id_); diff --git a/src/buffer_pool.cc b/src/buffer_pool.cc index 682cb6564..88eb13c75 100644 --- a/src/buffer_pool.cc +++ b/src/buffer_pool.cc @@ -1602,12 +1602,6 @@ Status PlaceBlob(SharedMemoryContext *context, RpcContext *rpc, HERMES_END_TIMED_BLOCK(); if (buffer_ids.size()) { - MetadataManager *mdm = GetMetadataManagerFromContext(context); - char *bucket_name = ReverseGetFromStorage(mdm, bucket_id.as_int, - kMapType_Bucket); - LOG(INFO) << "Attaching blob " << std::string(name) << " to Bucket " - << bucket_name << std::endl; - HERMES_BEGIN_TIMED_BLOCK("WriteBlobToBuffers"); WriteBlobToBuffers(context, rpc, blob, buffer_ids); HERMES_END_TIMED_BLOCK(); diff --git a/src/communication_mpi.cc b/src/communication_mpi.cc index a1235e786..cedb29001 100644 --- a/src/communication_mpi.cc +++ b/src/communication_mpi.cc @@ -241,7 +241,7 @@ size_t InitCommunication(CommunicationContext *comm, Arena *arena, break; } default: { - assert(!"Invalid code path\n"); + HERMES_INVALID_CODE_PATH; break; } } diff --git a/src/config_parser.cc b/src/config_parser.cc index dbe57f9a2..5b4f961ce 100644 --- a/src/config_parser.cc +++ b/src/config_parser.cc @@ -806,7 +806,7 @@ void ParseTokens(TokenList *tokens, Config *config) { break; } default: { - assert(!"Invalid code path\n"); + HERMES_INVALID_CODE_PATH; break; } } diff --git a/src/hermes_types.h b/src/hermes_types.h index 013df589f..da85afdd9 100644 --- a/src/hermes_types.h +++ b/src/hermes_types.h @@ -50,6 +50,9 @@ constexpr char kPlaceInHierarchy[] = "PlaceInHierarchy"; #define HERMES_NOT_IMPLEMENTED_YET \ LOG(FATAL) << __func__ << " not implemented yet\n" +#define HERMES_INVALID_CODE_PATH \ + LOG(FATAL) << "Invalid code path." << std::endl + /** A TargetID uniquely identifies a buffering target within the system. */ union TargetID { struct { diff --git a/src/metadata_management.cc b/src/metadata_management.cc index 1423b2cb8..a2302b152 100644 --- a/src/metadata_management.cc +++ b/src/metadata_management.cc @@ -64,29 +64,6 @@ bool IsNullTargetId(TargetID id) { return result; } -TicketMutex *GetMapMutex(MetadataManager *mdm, MapType map_type) { - TicketMutex *mutex = 0; - switch (map_type) { - case kMapType_Bucket: { - mutex = &mdm->bucket_map_mutex; - break; - } - case kMapType_VBucket: { - mutex = &mdm->vbucket_map_mutex; - break; - } - case kMapType_Blob: { - mutex = &mdm->blob_map_mutex; - break; - } - default: { - assert(!"Invalid code path\n"); - } - } - - return mutex; -} - void LocalPut(MetadataManager *mdm, const char *key, u64 val, MapType map_type) { PutToStorage(mdm, key, val, map_type); @@ -529,10 +506,11 @@ void LocalDestroyBlobById(SharedMemoryContext *context, RpcContext *rpc, FreeBufferIdList(context, rpc, blob_id); MetadataManager *mdm = GetMetadataManagerFromContext(context); - char *blob_name = ReverseGetFromStorage(mdm, blob_id.as_int, kMapType_Blob); + std::string blob_name = ReverseGetFromStorage(mdm, blob_id.as_int, + kMapType_Blob); - if (blob_name) { - DeleteId(mdm, rpc, blob_name, kMapType_Blob); + if (blob_name.size() > 0) { + DeleteId(mdm, rpc, blob_name.c_str(), kMapType_Blob); } else { // TODO(chogan): @errorhandling DLOG(INFO) << "Expected to find blob_id " << blob_id.as_int diff --git a/src/metadata_management.h b/src/metadata_management.h index 2b8b94308..2c15ed824 100644 --- a/src/metadata_management.h +++ b/src/metadata_management.h @@ -88,12 +88,23 @@ struct MetadataManager { ptrdiff_t swap_filename_prefix_offset; ptrdiff_t swap_filename_suffix_offset; + // TODO(chogan): @optimization Should the TicketMutexes here be reader/writer + // locks? + + /** Lock for accessing `BucketInfo` structures located at + * `bucket_info_offset` */ TicketMutex bucket_mutex; + /** Lock for accessing `VBucketInfo` structures located at + * `vbucket_info_offset` */ TicketMutex vbucket_mutex; + /** Lock for accessing the `IdMap` located at `bucket_map_offset` */ TicketMutex bucket_map_mutex; + /** Lock for accessing the `IdMap` located at `vbucket_map_offset` */ TicketMutex vbucket_map_mutex; + /** Lock for accessing the `IdMap` located at `blob_map_offset` */ TicketMutex blob_map_mutex; + /** Lock for accessing `IdList`s and `ChunkedIdList`s */ TicketMutex id_mutex; size_t map_seed; diff --git a/src/metadata_storage.h b/src/metadata_storage.h index 0d99de839..119679311 100644 --- a/src/metadata_storage.h +++ b/src/metadata_storage.h @@ -17,7 +17,9 @@ u64 GetFromStorage(MetadataManager *mdm, const char *key, MapType map_type); /** * */ -char *ReverseGetFromStorage(MetadataManager *mdm, u64 id, MapType map_type); +std::string ReverseGetFromStorage(MetadataManager *mdm, u64 id, + MapType map_type); + /** * */ diff --git a/src/metadata_storage_stb_ds.cc b/src/metadata_storage_stb_ds.cc index 41059e720..e81973ab8 100644 --- a/src/metadata_storage_stb_ds.cc +++ b/src/metadata_storage_stb_ds.cc @@ -65,8 +65,41 @@ void CheckHeapOverlap(MetadataManager *mdm) { } } +TicketMutex *GetMapMutex(MetadataManager *mdm, MapType map_type) { + TicketMutex *mutex = 0; + switch (map_type) { + case kMapType_Bucket: { + mutex = &mdm->bucket_map_mutex; + break; + } + case kMapType_VBucket: { + mutex = &mdm->vbucket_map_mutex; + break; + } + case kMapType_Blob: { + mutex = &mdm->blob_map_mutex; + break; + } + default: { + HERMES_INVALID_CODE_PATH; + } + } + + return mutex; +} + +/** + * Get a pointer to an IdMap in shared memory. + * + * This function acquires a lock because it's pointing to a shared data + * structure. Make sure to call `ReleaseMap` when you're finished with the + * IdMap. + */ IdMap *GetMap(MetadataManager *mdm, MapType map_type) { IdMap *result = 0; + TicketMutex *mutex = GetMapMutex(mdm, map_type); + BeginTicketMutex(mutex); + switch (map_type) { case kMapType_Bucket: { result = GetBucketMap(mdm); @@ -81,12 +114,94 @@ IdMap *GetMap(MetadataManager *mdm, MapType map_type) { break; } default: - assert(!"Invalid code path.\n"); + HERMES_INVALID_CODE_PATH; + } + + return result; +} + +/** + * Releases the lock acquired by `GetMap`. + */ +void ReleaseMap(MetadataManager *mdm, MapType map_type) { + TicketMutex *mutex = 0; + switch (map_type) { + case kMapType_Bucket: { + mutex = &mdm->bucket_map_mutex; + break; + } + case kMapType_VBucket: { + mutex = &mdm->vbucket_map_mutex; + break; + } + case kMapType_Blob: { + mutex = &mdm->blob_map_mutex; + break; + } + default: { + HERMES_INVALID_CODE_PATH; + } } + EndTicketMutex(mutex); +} + +/** + * Return a pointer to the the internal array of IDs that the `id_list` + * represents. + * + * T must be an `IdList` or a `ChunkedIdList`. This call acquires a lock, and + * must be paired with a corresponding call to `ReleaseIdsPtr` to release the + * lock. + */ +template +u64 *GetIdsPtr(MetadataManager *mdm, T id_list) { + Heap *id_heap = GetIdHeap(mdm); + BeginTicketMutex(&mdm->id_mutex); + u64 *result = (u64 *)HeapOffsetToPtr(id_heap, id_list.head_offset); + + return result; +} + +/** + * Returns a copy of an embedded `IdList`. + * + * An `IdList` that consists of `BufferID`s contains an embedded `IdList` as the + * first element of the list. This is so the `BlobID` can find information about + * its buffers from a single offset. If you want a pointer to the `BufferID`s in + * an `IdList`, then you have to first retrieve the embedded `IdList` using this + * function, and then use the resulting `IdList` in `GetIdsPtr`. + */ +IdList GetEmbeddedIdList(MetadataManager *mdm, u32 offset) { + Heap *id_heap = GetIdHeap(mdm); + BeginTicketMutex(&mdm->id_mutex); + IdList *embedded_id_list = (IdList *)HeapOffsetToPtr(id_heap, offset); + IdList result = *embedded_id_list; + EndTicketMutex(&mdm->id_mutex); + + return result; +} + +/** + * Return a pointer to the internal array of `BufferID`s associated with the + * `blob_id`. + * + * Acquires a lock, and must be paired with a call to `ReleaseIdsPtr` to release + * the lock. The length of the array is returned in the `length` parameter. + */ +BufferID *GetBufferIdsPtrFromBlobId(MetadataManager *mdm, BlobID blob_id, + size_t &length) { + IdList id_list = GetEmbeddedIdList(mdm, blob_id.bits.buffer_ids_offset); + length = id_list.length; + BufferID *result = (BufferID *)GetIdsPtr(mdm, id_list); + return result; } +void ReleaseIdsPtr(MetadataManager *mdm) { + EndTicketMutex(&mdm->id_mutex); +} + /** Convert a key offset into the pointer where the string is stored. * * Even though our maps are char* -> u64, the keys are not actually pointers to @@ -103,47 +218,66 @@ static char *GetKey(MetadataManager *mdm, IdMap *map, u32 index) { return result; } -void AllocateOrGrowBlobIdList(MetadataManager *mdm, ChunkedIdList *blobs) { +template +void FreeIdList(MetadataManager *mdm, T id_list) { Heap *id_heap = GetIdHeap(mdm); BeginTicketMutex(&mdm->id_mutex); - if (blobs->capacity == 0) { - u8 *ids = HeapPushSize(id_heap, sizeof(BlobID) * kIdListChunkSize); - if (ids) { - blobs->capacity = kIdListChunkSize; - blobs->length = 0; - blobs->head_offset = GetHeapOffset(id_heap, (u8 *)ids); - } - } else { - // NOTE(chogan): grow capacity by kIdListChunkSize IDs - BlobID *ids = (BlobID *)HeapOffsetToPtr(id_heap, blobs->head_offset); - size_t new_capacity = blobs->capacity + kIdListChunkSize; - BlobID *new_ids = HeapPushArray(id_heap, new_capacity); - CopyIds((u64 *)new_ids, (u64 *)ids, blobs->length); - HeapFree(id_heap, ids); - - blobs->capacity += kIdListChunkSize; - blobs->head_offset = GetHeapOffset(id_heap, (u8 *)new_ids); - } + u8 *ptr = HeapOffsetToPtr(id_heap, id_list.head_offset); + HeapFree(id_heap, ptr); EndTicketMutex(&mdm->id_mutex); +} - CheckHeapOverlap(mdm); +void FreeEmbeddedIdList(MetadataManager *mdm, u32 offset) { + Heap *id_heap = GetIdHeap(mdm); + BeginTicketMutex(&mdm->id_mutex); + u8 *to_free = HeapOffsetToPtr(id_heap, offset); + HeapFree(id_heap, to_free); + EndTicketMutex(&mdm->id_mutex); } -void LocalAddBlobIdToBucket(MetadataManager *mdm, BucketID bucket_id, - BlobID blob_id) { +/** + * Assumes the caller has protected @p id_list with a lock. + */ +void AllocateOrGrowIdList(MetadataManager *mdm, ChunkedIdList *id_list) { Heap *id_heap = GetIdHeap(mdm); + // NOTE(chogan): grow capacity by kIdListChunkSize IDs + size_t new_capacity = id_list->capacity + kIdListChunkSize; + u64 *new_ids = HeapPushArray(id_heap, new_capacity); + + if (id_list->capacity != 0) { + // NOTE(chogan): Copy over old ids and then free them + u64 *ids = GetIdsPtr(mdm, *id_list); + CopyIds(new_ids, ids, id_list->length); + ReleaseIdsPtr(mdm); + FreeIdList(mdm, *id_list); + } else { + // NOTE(chogan): This is the initial allocation so initialize length + id_list->length = 0; + } - // TODO(chogan): Think about lock granularity - BeginTicketMutex(&mdm->bucket_mutex); - BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id); - ChunkedIdList *blobs = &info->blobs; + id_list->capacity = new_capacity; + id_list->head_offset = GetHeapOffset(id_heap, (u8 *)new_ids); +} - if (blobs->length >= blobs->capacity) { - AllocateOrGrowBlobIdList(mdm, blobs); +/** + * Assumes the caller has protected @p id_list with a lock. + */ +void AppendToChunkedIdList(MetadataManager *mdm, ChunkedIdList *id_list, + u64 id) { + if (id_list->length >= id_list->capacity) { + AllocateOrGrowIdList(mdm, id_list); } - BlobID *head = (BlobID *)HeapOffsetToPtr(id_heap, blobs->head_offset); - head[blobs->length++] = blob_id; + u64 *head = GetIdsPtr(mdm, *id_list); + head[id_list->length++] = id; + ReleaseIdsPtr(mdm); +} + +void LocalAddBlobIdToBucket(MetadataManager *mdm, BucketID bucket_id, + BlobID blob_id) { + BeginTicketMutex(&mdm->bucket_mutex); + BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id); + AppendToChunkedIdList(mdm, &info->blobs, blob_id.as_int); EndTicketMutex(&mdm->bucket_mutex); CheckHeapOverlap(mdm); @@ -151,41 +285,41 @@ void LocalAddBlobIdToBucket(MetadataManager *mdm, BucketID bucket_id, void LocalAddBlobIdToVBucket(MetadataManager *mdm, VBucketID vbucket_id, BlobID blob_id) { - Heap *id_heap = GetIdHeap(mdm); - - // TODO(chogan): Think about lock granularity BeginTicketMutex(&mdm->vbucket_mutex); VBucketInfo *info = LocalGetVBucketInfoById(mdm, vbucket_id); - ChunkedIdList *blobs = &info->blobs; - - if (blobs->length >= blobs->capacity) { - AllocateOrGrowBlobIdList(mdm, blobs); - } - - BlobID *head = (BlobID *)HeapOffsetToPtr(id_heap, blobs->head_offset); - head[blobs->length++] = blob_id; + AppendToChunkedIdList(mdm, &info->blobs, blob_id.as_int); EndTicketMutex(&mdm->vbucket_mutex); CheckHeapOverlap(mdm); } -IdList *AllocateIdList(MetadataManager *mdm, u32 length) { +IdList AllocateIdList(MetadataManager *mdm, u32 length) { static_assert(sizeof(IdList) == sizeof(u64)); Heap *id_heap = GetIdHeap(mdm); - // NOTE(chogan): Add 1 extra for the embedded BufferIdList - u64 *id_list_memory = HeapPushArray(id_heap, length + 1); - IdList *result = (IdList *)id_list_memory; - result->length = length; - result->head_offset = GetHeapOffset(id_heap, (u8 *)(result + 1)); + BeginTicketMutex(&mdm->id_mutex); + u64 *id_list_memory = HeapPushArray(id_heap, length); + IdList result = {}; + result.length = length; + result.head_offset = GetHeapOffset(id_heap, (u8 *)(id_list_memory)); + EndTicketMutex(&mdm->id_mutex); CheckHeapOverlap(mdm); return result; } -template -u64 *GetIdsPtr(MetadataManager *mdm, T id_list) { +u32 AllocateEmbeddedIdList(MetadataManager *mdm, u32 length) { + static_assert(sizeof(IdList) == sizeof(u64)); Heap *id_heap = GetIdHeap(mdm); - u64 *result = (u64 *)HeapOffsetToPtr(id_heap, id_list.head_offset); + BeginTicketMutex(&mdm->id_mutex); + // NOTE(chogan): Add 1 extra for the embedded IdList + u64 *id_list_memory = HeapPushArray(id_heap, length + 1); + IdList *embedded_id_list = (IdList *)id_list_memory; + embedded_id_list->length = length; + embedded_id_list->head_offset = + GetHeapOffset(id_heap, (u8 *)(embedded_id_list + 1)); + u32 result = GetHeapOffset(id_heap, (u8 *)embedded_id_list); + EndTicketMutex(&mdm->id_mutex); + CheckHeapOverlap(mdm); return result; } @@ -201,6 +335,7 @@ std::vector LocalGetBlobIds(SharedMemoryContext *context, for (u32 i = 0; i < num_blobs; ++i) { result[i] = blob_ids[i]; } + ReleaseIdsPtr(mdm); return result; } @@ -209,72 +344,69 @@ u32 LocalAllocateBufferIdList(MetadataManager *mdm, const std::vector &buffer_ids) { static_assert(sizeof(IdList) == sizeof(BufferID)); u32 length = (u32)buffer_ids.size(); - IdList *id_list = AllocateIdList(mdm, length); - CopyIds(GetIdsPtr(mdm, *id_list), (u64 *)buffer_ids.data(), length); + u32 id_list_offset = AllocateEmbeddedIdList(mdm, length); + IdList id_list = GetEmbeddedIdList(mdm, id_list_offset); + u64 *ids = (u64 *)GetIdsPtr(mdm, id_list); + CopyIds(ids, (u64 *)buffer_ids.data(), length); + ReleaseIdsPtr(mdm); - Heap *id_heap = GetIdHeap(mdm); - u32 result = GetHeapOffset(id_heap, (u8 *)id_list); + u32 result = id_list_offset; return result; } std::vector LocalGetBufferIdList(MetadataManager *mdm, BlobID blob_id) { - Heap *id_heap = GetIdHeap(mdm); - IdList *id_list = - (IdList *)HeapOffsetToPtr(id_heap, blob_id.bits.buffer_ids_offset); - BufferID *ids = (BufferID *)HeapOffsetToPtr(id_heap, id_list->head_offset); - std::vector result(id_list->length); - CopyIds((u64 *)result.data(), (u64 *)ids, id_list->length); + size_t length = 0; + BufferID *ids = GetBufferIdsPtrFromBlobId(mdm, blob_id, length); + std::vector result(length); + CopyIds((u64 *)result.data(), (u64 *)ids, length); + ReleaseIdsPtr(mdm); return result; } void LocalGetBufferIdList(Arena *arena, MetadataManager *mdm, BlobID blob_id, BufferIdArray *buffer_ids) { - Heap *id_heap = GetIdHeap(mdm); - IdList *id_list = - (IdList *)HeapOffsetToPtr(id_heap, blob_id.bits.buffer_ids_offset); - BufferID *ids = (BufferID *)HeapOffsetToPtr(id_heap, id_list->head_offset); - buffer_ids->ids = PushArray(arena, id_list->length); - buffer_ids->length = id_list->length; - CopyIds((u64 *)buffer_ids->ids, (u64 *)ids, id_list->length); + size_t length = 0; + BufferID *ids = GetBufferIdsPtrFromBlobId(mdm, blob_id, length); + buffer_ids->ids = PushArray(arena, length); + buffer_ids->length = length; + CopyIds((u64 *)buffer_ids->ids, (u64 *)ids, length); + ReleaseIdsPtr(mdm); } void LocalFreeBufferIdList(SharedMemoryContext *context, BlobID blob_id) { MetadataManager *mdm = GetMetadataManagerFromContext(context); - Heap *id_heap = GetIdHeap(mdm); - u8 *to_free = HeapOffsetToPtr(id_heap, blob_id.bits.buffer_ids_offset); - - HeapFree(id_heap, to_free); + FreeEmbeddedIdList(mdm, blob_id.bits.buffer_ids_offset); CheckHeapOverlap(mdm); } void LocalRemoveBlobFromBucketInfo(SharedMemoryContext *context, BucketID bucket_id, BlobID blob_id) { MetadataManager *mdm = GetMetadataManagerFromContext(context); + BeginTicketMutex(&mdm->bucket_mutex); BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id); ChunkedIdList *blobs = &info->blobs; - Heap *id_heap = GetIdHeap(mdm); - BeginTicketMutex(&mdm->bucket_mutex); - BlobID *blobs_arr = (BlobID *)HeapOffsetToPtr(id_heap, blobs->head_offset); + BlobID *blobs_arr = (BlobID *)GetIdsPtr(mdm, *blobs); for (u32 i = 0; i < blobs->length; ++i) { if (blobs_arr[i].as_int == blob_id.as_int) { blobs_arr[i] = blobs_arr[--blobs->length]; break; } } + ReleaseIdsPtr(mdm); EndTicketMutex(&mdm->bucket_mutex); } bool LocalContainsBlob(SharedMemoryContext *context, BucketID bucket_id, BlobID blob_id) { MetadataManager *mdm = GetMetadataManagerFromContext(context); + BeginTicketMutex(&mdm->bucket_mutex); BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id); ChunkedIdList *blobs = &info->blobs; - Heap *id_heap = GetIdHeap(mdm); - BlobID *blob_id_arr = (BlobID *)HeapOffsetToPtr(id_heap, blobs->head_offset); + BlobID *blob_id_arr = (BlobID *)GetIdsPtr(mdm, *blobs); bool result = false; for (u32 i = 0; i < blobs->length; ++i) { @@ -283,6 +415,8 @@ bool LocalContainsBlob(SharedMemoryContext *context, BucketID bucket_id, break; } } + ReleaseIdsPtr(mdm); + EndTicketMutex(&mdm->bucket_mutex); return result; } @@ -295,25 +429,32 @@ static inline bool HasAllocatedBlobs(BucketInfo *info) { bool LocalDestroyBucket(SharedMemoryContext *context, RpcContext *rpc, const char *bucket_name, BucketID bucket_id) { + bool destroyed = false; MetadataManager *mdm = GetMetadataManagerFromContext(context); + BeginTicketMutex(&mdm->bucket_mutex); BucketInfo *info = LocalGetBucketInfoById(mdm, bucket_id); - Heap *id_heap = GetIdHeap(mdm); - BlobID *blobs = (BlobID *)HeapOffsetToPtr(id_heap, info->blobs.head_offset); - bool destroyed = false; // TODO(chogan): @optimization Lock granularity can probably be relaxed if // this is slow - BeginTicketMutex(&mdm->bucket_mutex); int ref_count = info->ref_count.load(); if (ref_count == 1) { if (HasAllocatedBlobs(info)) { + // NOTE(chogan): Collecting the blobs to destroy first and destroying them + // later avoids locking complications + std::vector blobs_to_destroy; + blobs_to_destroy.reserve(info->blobs.length); + BlobID *blobs = (BlobID *)GetIdsPtr(mdm, info->blobs); for (u32 i = 0; i < info->blobs.length; ++i) { BlobID blob_id = *(blobs + i); - DestroyBlobById(context, rpc, blob_id); + blobs_to_destroy.push_back(blob_id); } + ReleaseIdsPtr(mdm); + for (auto blob_id : blobs_to_destroy) { + DestroyBlobById(context, rpc, blob_id); + } // Delete BlobId list - HeapFree(id_heap, blobs); + FreeIdList(mdm, info->blobs); } info->blobs.length = 0; @@ -343,13 +484,14 @@ bool LocalDestroyBucket(SharedMemoryContext *context, RpcContext *rpc, std::vector GetNodeTargets(SharedMemoryContext *context) { MetadataManager *mdm = GetMetadataManagerFromContext(context); - u64 *target_ids = GetIdsPtr(mdm, mdm->node_targets); u32 length = mdm->node_targets.length; std::vector result(length); + u64 *target_ids = GetIdsPtr(mdm, mdm->node_targets); for (u32 i = 0; i < length; ++i) { result[i].as_int = target_ids[i]; } + ReleaseIdsPtr(mdm); return result; } @@ -357,12 +499,9 @@ std::vector GetNodeTargets(SharedMemoryContext *context) { void PutToStorage(MetadataManager *mdm, const char *key, u64 val, MapType map_type) { Heap *heap = GetMapHeap(mdm); - TicketMutex *mutex = GetMapMutex(mdm, map_type); - - BeginTicketMutex(mutex); IdMap *map = GetMap(mdm, map_type); shput(map, key, val, heap); - EndTicketMutex(mutex); + ReleaseMap(mdm, map_type); // TODO(chogan): Maybe wrap this in a DEBUG only macro? CheckHeapOverlap(mdm); @@ -370,28 +509,30 @@ void PutToStorage(MetadataManager *mdm, const char *key, u64 val, u64 GetFromStorage(MetadataManager *mdm, const char *key, MapType map_type) { Heap *heap = GetMapHeap(mdm); - TicketMutex *mutex = GetMapMutex(mdm, map_type); - - BeginTicketMutex(mutex); IdMap *map = GetMap(mdm, map_type); u64 result = shget(map, key, heap); - EndTicketMutex(mutex); + ReleaseMap(mdm, map_type); return result; } -char *ReverseGetFromStorage(MetadataManager *mdm, u64 id, - MapType map_type) { +std::string ReverseGetFromStorage(MetadataManager *mdm, u64 id, + MapType map_type) { + std::string result; + size_t map_size = GetStoredMapSize(mdm, map_type); IdMap *map = GetMap(mdm, map_type); - char *result = 0; // TODO(chogan): @optimization This could be more efficient if necessary - for (size_t i = 0; i < GetStoredMapSize(mdm, map_type); ++i) { + for (size_t i = 0; i < map_size; ++i) { if (map[i].value == id) { - result = GetKey(mdm, map, i); + char *key = GetKey(mdm, map, i); + if (key) { + result = key; + } break; } } + ReleaseMap(mdm, map_type); return result; } @@ -399,12 +540,9 @@ char *ReverseGetFromStorage(MetadataManager *mdm, u64 id, void DeleteFromStorage(MetadataManager *mdm, const char *key, MapType map_type) { Heap *heap = GetMapHeap(mdm); - TicketMutex *mutex = GetMapMutex(mdm, map_type); - - BeginTicketMutex(mutex); IdMap *map = GetMap(mdm, map_type); shdel(map, key, heap); - EndTicketMutex(mutex); + ReleaseMap(mdm, map_type); // TODO(chogan): Maybe wrap this in a DEBUG only macro? CheckHeapOverlap(mdm); @@ -413,6 +551,7 @@ void DeleteFromStorage(MetadataManager *mdm, const char *key, size_t GetStoredMapSize(MetadataManager *mdm, MapType map_type) { IdMap *map = GetMap(mdm, map_type); size_t result = shlen(map); + ReleaseMap(mdm, map_type); return result; } @@ -480,13 +619,14 @@ void InitMetadataStorage(SharedMemoryContext *context, MetadataManager *mdm, mdm->id_heap_offset = GetOffsetFromMdm(mdm, id_heap); // NOTE(chogan): Local Targets default to one Target per Device - IdList *node_targets = AllocateIdList(mdm, config->num_devices); - TargetID *target_ids = (TargetID *)GetIdsPtr(mdm, *node_targets); - for (u32 i = 0; i < node_targets->length; ++i) { + IdList node_targets = AllocateIdList(mdm, config->num_devices); + TargetID *target_ids = (TargetID *)GetIdsPtr(mdm, node_targets); + for (u32 i = 0; i < node_targets.length; ++i) { Target *target = GetTarget(context, i); target_ids[i] = target->id; } - mdm->node_targets = *node_targets; + ReleaseIdsPtr(mdm); + mdm->node_targets = node_targets; // ID Maps diff --git a/test/bucket_test.cc b/test/bucket_test.cc index 264464f84..05251e2c6 100644 --- a/test/bucket_test.cc +++ b/test/bucket_test.cc @@ -113,7 +113,7 @@ void TestBucketPersist(std::shared_ptr hermes) { break; } default: { - Assert(!"Invalid code path\n."); + HERMES_INVALID_CODE_PATH; } } Assert(read_buffer[i] == expected);