Bug/ensure database number match when snapshotting (#441)

This pull request includes changes to the `hashtable_mcmp_op_get_by_index` function in multiple files to improve the handling of the `database_number` parameter. The changes ensure that the `database_number` is correctly passed and updated within the function calls. Improvements to `hashtable_mcmp_op_get_by_index` function: * [`src/data_structures/hashtable/mcmp/hashtable_op_get.c`](diffhunk://#diff-86e21ce364585344eeb916a45af6eed9ea7e2af80fbff3ff4f83857986757dc0L103-R105): Modified the `hashtable_mcmp_op_get_by_index` function to pass `database_number` as a pointer and update it within the function. [[1]](diffhunk://#diff-86e21ce364585344eeb916a45af6eed9ea7e2af80fbff3ff4f83857986757dc0L103-R105) [[2]](diffhunk://#diff-86e21ce364585344eeb916a45af6eed9ea7e2af80fbff3ff4f83857986757dc0L149-R151) * [`src/data_structures/hashtable/mcmp/hashtable_op_get.h`](diffhunk://#diff-8e1333f9331a063c2477010144b1c4d810d7d405e09439e643ddf90b80cc39e0L18-R20): Updated the function signature of `hashtable_mcmp_op_get_by_index` to reflect the change in `database_number` parameter type. Updates to function calls: * [`src/storage/db/storage_db.c`](diffhunk://#diff-7f9e44ce2f00979e67eb946f380a24688073a061281f7ad59792bbb4aeaf0feeR1635-R1644): Adjusted the call to `hashtable_mcmp_op_get_by_index` to pass `database_number` as a pointer and added a check to ensure the entry is from the correct database before deletion. * [`src/storage/db/storage_db_snapshot.c`](diffhunk://#diff-905ca66f717e9cf869d02cda91a0ae206f7b467cd6d8d78eeaeb04db0d95e1ccL967-R969): Modified the call to `hashtable_mcmp_op_get_by_index` to pass `database_number` as a pointer.
danielealbano · Dec 29, 2024 · db4891a · db4891a
1 parent b1b51a9
commit db4891a
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 9 deletions.
diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_get.c b/src/data_structures/hashtable/mcmp/hashtable_op_get.c
@@ -100,9 +100,9 @@ bool hashtable_mcmp_op_get(
 
 bool hashtable_mcmp_op_get_by_index(
         hashtable_t *hashtable,
-        hashtable_database_number_t database_number,
         transaction_t *transaction,
         hashtable_bucket_index_t bucket_index,
+        hashtable_database_number_t *database_number,
         hashtable_value_data_t *current_value) {
     hashtable_half_hashes_chunk_volatile_t* half_hashes_chunk;
     hashtable_chunk_index_t chunk_index;
@@ -146,12 +146,9 @@ bool hashtable_mcmp_op_get_by_index(
 
         key_value = &hashtable_data->keys_values[bucket_index];
 
-        if (unlikely(key_value->database_number != database_number)) {
-            return false;
-        }
-
         if (current_value != NULL) {
             *current_value = key_value->data;
+            *database_number = key_value->database_number;
         }
 
         return true;

diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_get.h b/src/data_structures/hashtable/mcmp/hashtable_op_get.h
@@ -15,9 +15,9 @@ bool hashtable_mcmp_op_get(
 
 bool hashtable_mcmp_op_get_by_index(
         hashtable_t *hashtable,
-        hashtable_database_number_t database_number,
         transaction_t *transaction,
         hashtable_bucket_index_t bucket_index,
+        hashtable_database_number_t *database_number,
         hashtable_value_data_t *current_value);
 
 bool hashtable_mcmp_op_get_by_index_all_databases(

diff --git a/src/storage/db/storage_db.c b/src/storage/db/storage_db.c
@@ -1632,13 +1632,16 @@ bool storage_db_op_flush_sync(
         transaction_acquire(&transaction);
 
         storage_db_entry_index_t *entry_index = NULL;
+        storage_db_database_number_t entry_database_number;
         if (hashtable_mcmp_op_get_by_index(
                 db->hashtable,
-                database_number,
                 &transaction,
                 bucket_index,
+                &entry_database_number,
                 (void *) &entry_index)) {
-            if (entry_index->created_time_ms <= deletion_start_ms) {
+            // Ensure that the entry is from the correct database and that it's older than the deletion start time, to
+            // avoid deleting entries that have been created after the flush has started
+            if (database_number == entry_database_number && entry_index->created_time_ms <= deletion_start_ms) {
                 storage_db_op_delete_by_index(
                         db,
                         database_number,

diff --git a/src/storage/db/storage_db_snapshot.c b/src/storage/db/storage_db_snapshot.c
@@ -964,9 +964,9 @@ bool storage_db_snapshot_rdb_process_block(
         storage_db_entry_index_t *entry_index = NULL;
         if (!hashtable_mcmp_op_get_by_index(
                 db->hashtable,
-                database_number,
                 &transaction,
                 bucket_index,
+                &database_number,
                 (void *) &entry_index)) {
             goto loop_end;
         }