Implement randomkey command (#202)

This PR implements the RANDOMKEY command.

It refactors the hashtable op get key function to return a copy of the
key from the hashtable and to ensure that the copy is actually valid
implementing a 2-phase verification.

The PR also includes the tests for the new command and a minor
improvement to the DBSIZE command

src/data_structures/hashtable/mcmp/hashtable.h

@@ -22,6 +22,13 @@ extern "C" {
 #define HASHTABLE_KEY_INLINE_MAX_LENGTH                 22
 #endif
 
+#define HASHTABLE_TO_CHUNK_INDEX(bucket_index) \
+    ((bucket_index) / HASHTABLE_MCMP_HALF_HASHES_CHUNK_SLOTS_COUNT)
+#define HASHTABLE_TO_CHUNK_SLOT_INDEX(bucket_index) \
+    ((bucket_index) % HASHTABLE_MCMP_HALF_HASHES_CHUNK_SLOTS_COUNT)
+#define HASHTABLE_TO_BUCKET_INDEX(chunk_index, chunk_slot_index) \
+    (((chunk_index) * HASHTABLE_MCMP_HALF_HASHES_CHUNK_SLOTS_COUNT) + (chunk_slot_index))
+
 typedef uint8_t hashtable_key_value_flags_t;
 typedef uint64_t hashtable_hash_t;
 typedef uint32_t hashtable_hash_half_t;

src/data_structures/hashtable/mcmp/hashtable_op_delete.c

@@ -119,11 +119,11 @@ bool hashtable_mcmp_op_delete(
                 // scenario in which it might happen is that the code in the get operation has already checked the flags
                 // and therefore is now comparing the key.
                 // Under very heavy load (64 cores, 128 hw threads, 2048 threads operating on the hashtable) it has never
-                // caused any though.
-                // It's not a problem though if the slab allocator using hugepages is enabled (as it should), the slot in
-                // the slab allocator will just get marked as reusable and the worst case scenario is that it will be picked
-                // up and filled or zero-ed immediately and the key comparison being carried out in get will fail, but this
-                // is an acceptable scenario because the bucket is being deleted.
+                // caused any problem though.
+                // When the custom memory allocator is enabled (as it should) it's not a concern though, the slot in the
+                // slab allocator will just get marked as reusable and the worst case scenario is that it will be picked
+                // up and filled or zero-ed immediately and the key comparison being carried out in get will fail, the
+                // scenario because the bucket is being deleted.
                 slab_allocator_mem_free(key_value->external_key.data);
                 key_value->external_key.data = NULL;
                 key_value->external_key.size = 0;

src/data_structures/hashtable/mcmp/hashtable_op_get_key.c

@@ -12,12 +12,16 @@
 #include <stdatomic.h>
 #include <string.h>
 #include <numa.h>
+#include <assert.h>
 
 #include "misc.h"
 #include "exttypes.h"
+#include "log/log.h"
 #include "memory_fences.h"
 #include "spinlock.h"
-#include "log/log.h"
+#include "data_structures/double_linked_list/double_linked_list.h"
+#include "data_structures/queue_mpmc/queue_mpmc.h"
+#include "slab_allocator.h"
 
 #include "hashtable.h"
 
@@ -26,9 +30,16 @@ bool hashtable_mcmp_op_get_key(
         hashtable_bucket_index_t bucket_index,
         hashtable_key_data_t **key,
         hashtable_key_size_t *key_size) {
+    char *source_key = NULL;
+    size_t source_key_size = 0;
+    hashtable_chunk_index_t chunk_index = HASHTABLE_TO_CHUNK_INDEX(bucket_index);
+    hashtable_chunk_slot_index_t chunk_slot_index = HASHTABLE_TO_CHUNK_SLOT_INDEX(bucket_index);
+
     MEMORY_FENCE_LOAD();
 
     hashtable_data_volatile_t* hashtable_data = hashtable->ht_current;
+    hashtable_slot_id_volatile_t slot_id =
+            hashtable_data->half_hashes_chunk[chunk_index].half_hashes[chunk_slot_index].slot_id;
     hashtable_key_value_volatile_t *key_value = &hashtable_data->keys_values[bucket_index];
 
     if (
@@ -38,19 +49,58 @@ bool hashtable_mcmp_op_get_key(
     }
 
 #if HASHTABLE_FLAG_ALLOW_KEY_INLINE==1
-    // The key may potentially change if the item is first deleted and then recreated, if it's inline it
-    // doesn't really matter because the key will mismatch and the execution will continue but if the key is
-    // stored externally and the allocated memory is freed it may crash.
     if (HASHTABLE_KEY_VALUE_HAS_FLAG(key_value->flags, HASHTABLE_KEY_VALUE_FLAG_KEY_INLINE)) {
-        *key = key_value->inline_key.data;
-        *key_size = key_value->inline_key.size;
+        source_key = key_value->inline_key.data;
+        source_key_size = key_value->inline_key.size;
     } else {
 #endif
-        *key = key_value->external_key.data;
-        *key_size = key_value->external_key.size;
+        source_key = key_value->external_key.data;
+        source_key_size = key_value->external_key.size;
 #if HASHTABLE_FLAG_ALLOW_KEY_INLINE==1
     }
 #endif
 
-    return true;
+    assert(source_key != NULL);
+    assert(source_key_size > 0);
+
+    *key = slab_allocator_mem_alloc(source_key_size);
+    memcpy(*key, source_key, source_key_size);
+    *key_size = source_key_size;
+
+    // Validate that the hash and the key length in the bucket haven't changed or that the bucket has been deleted in
+    // the meantime
+    MEMORY_FENCE_LOAD();
+
+    bool key_deleted_or_different = false;
+    if (unlikely(slot_id != hashtable->ht_current->half_hashes_chunk[chunk_index].half_hashes[chunk_slot_index].slot_id)) {
+        key_deleted_or_different = true;
+    }
+
+    if (unlikely(!key_deleted_or_different &&
+        (HASHTABLE_KEY_VALUE_IS_EMPTY(key_value->flags) ||
+         HASHTABLE_KEY_VALUE_HAS_FLAG(key_value->flags, HASHTABLE_KEY_VALUE_FLAG_DELETED)))) {
+        key_deleted_or_different = true;
+    }
+
+#if HASHTABLE_FLAG_ALLOW_KEY_INLINE == 1
+    if (!HASHTABLE_KEY_VALUE_HAS_FLAG(key_value->flags, HASHTABLE_KEY_VALUE_FLAG_KEY_INLINE)) {
+#endif
+    if (unlikely(!key_deleted_or_different && key_value->external_key.size != source_key_size)) {
+        key_deleted_or_different = true;
+    }
+#if HASHTABLE_FLAG_ALLOW_KEY_INLINE == 1
+    } else {
+        if (unlikely(!key_deleted_or_different && key_value->inline_key.size != source_key_size)) {
+            return NULL;
+        }
+    }
+#endif
+
+    if (unlikely(key_deleted_or_different)) {
+        slab_allocator_mem_free(key);
+        *key = NULL;
+        *key_size = 0;
+    }
+
+    return *key != NULL;
 }

src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.c

@@ -0,0 +1,41 @@
+/**
+ * Copyright (C) 2018-2022 Daniele Salvatore Albano
+ * All rights reserved.
+ *
+ * This software may be modified and distributed under the terms
+ * of the BSD license.  See the LICENSE file for details.
+ **/
+
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdatomic.h>
+#include <assert.h>
+#include <string.h>
+
+#include "random.h"
+#include "misc.h"
+#include "exttypes.h"
+#include "memory_fences.h"
+#include "spinlock.h"
+#include "data_structures/double_linked_list/double_linked_list.h"
+#include "data_structures/queue_mpmc/queue_mpmc.h"
+#include "slab_allocator.h"
+#include "data_structures/hashtable/mcmp/hashtable.h"
+#include "data_structures/hashtable/mcmp/hashtable_data.h"
+#include "data_structures/hashtable/mcmp/hashtable_op_get_key.h"
+
+#include "hashtable_op_get_random_key.h"
+
+bool hashtable_mcmp_op_get_random_key_try(
+        hashtable_t *hashtable,
+        char **key,
+        hashtable_key_size_t *key_size) {
+    uint64_t random_value = random_generate();
+
+    return hashtable_mcmp_op_get_key(
+            hashtable,
+            random_value % hashtable->ht_current->buckets_count_real,
+            key,
+            key_size);
+}

src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.h

@@ -0,0 +1,17 @@
+#ifndef CACHEGRAND_HASHTABLE_OP_GET_RANDOM_KEY_H
+#define CACHEGRAND_HASHTABLE_OP_GET_RANDOM_KEY_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+bool hashtable_mcmp_op_get_random_key_try(
+        hashtable_t *hashtable,
+        char **key,
+        hashtable_key_size_t *key_size);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif //CACHEGRAND_HASHTABLE_OP_GET_RANDOM_KEY_H

src/data_structures/hashtable/mcmp/hashtable_support_op_arch.c

@@ -378,7 +378,6 @@ bool CONCAT(hashtable_mcmp_support_op_search_key_or_create_new, CACHEGRAND_HASHT
 #endif
                     LOG_DI(">>> key fetched, comparing");
 
-
                     if (key_size != found_key_compare_size || strncmp(
                             key,
                             (const char*)found_key,

src/module/redis/command/module_redis_command_randomkey.c

@@ -0,0 +1,62 @@
+/**
+ * Copyright (C) 2018-2022 Daniele Salvatore Albano
+ * All rights reserved.
+ *
+ * This software may be modified and distributed under the terms
+ * of the BSD license.  See the LICENSE file for details.
+ **/
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+#include <strings.h>
+#include <arpa/inet.h>
+#include <assert.h>
+
+#include "misc.h"
+#include "exttypes.h"
+#include "log/log.h"
+#include "clock.h"
+#include "spinlock.h"
+#include "data_structures/small_circular_queue/small_circular_queue.h"
+#include "data_structures/double_linked_list/double_linked_list.h"
+#include "data_structures/queue_mpmc/queue_mpmc.h"
+#include "slab_allocator.h"
+#include "data_structures/hashtable/mcmp/hashtable.h"
+#include "data_structures/hashtable/mcmp/hashtable_op_get.h"
+#include "data_structures/hashtable/spsc/hashtable_spsc.h"
+#include "protocol/redis/protocol_redis.h"
+#include "protocol/redis/protocol_redis_reader.h"
+#include "protocol/redis/protocol_redis_writer.h"
+#include "module/module.h"
+#include "network/io/network_io_common.h"
+#include "config.h"
+#include "fiber.h"
+#include "network/channel/network_channel.h"
+#include "storage/io/storage_io_common.h"
+#include "storage/channel/storage_channel.h"
+#include "storage/db/storage_db.h"
+#include "module/redis/module_redis.h"
+#include "module/redis/module_redis_connection.h"
+#include "module/redis/module_redis_command.h"
+#include "network/network.h"
+#include "worker/worker_stats.h"
+#include "worker/worker_context.h"
+
+#define TAG "module_redis_command_randomkey"
+
+MODULE_REDIS_COMMAND_FUNCPTR_COMMAND_END(randomkey) {
+    bool return_res;
+    hashtable_key_size_t key_size = 0;
+    char *key = storage_db_op_random_key(connection_context->db, &key_size);
+
+    if (likely(key)) {
+        return_res = module_redis_connection_send_string(connection_context, key, key_size);
+        slab_allocator_mem_free(key);
+    } else {
+        return_res = module_redis_connection_send_string_null(connection_context);
+    }
+
+    return return_res;
+}

src/storage/db/storage_db.c

@@ -33,6 +33,7 @@
 #include "data_structures/hashtable/mcmp/hashtable_op_delete.h"
 #include "data_structures/hashtable/mcmp/hashtable_op_iter.h"
 #include "data_structures/hashtable/mcmp/hashtable_op_rmw.h"
+#include "data_structures/hashtable/mcmp/hashtable_op_get_random_key.h"
 #include "data_structures/hashtable/mcmp/hashtable_thread_counters.h"
 #include "data_structures/queue_mpmc/queue_mpmc.h"
 #include "slab_allocator.h"
@@ -1307,6 +1308,19 @@ int64_t storage_db_op_get_size(
     return size;
 }
 
+char *storage_db_op_random_key(
+        storage_db_t *db,
+        hashtable_key_size_t *key_size) {
+    char *key = NULL;
+
+    while(storage_db_op_get_size(db) > 0 &&
+        !hashtable_mcmp_op_get_random_key_try(db->hashtable, &key, key_size)) {
+        // do nothing
+    }
+
+    return key;
+}
+
 bool storage_db_op_flush_sync(
         storage_db_t *db) {
     // As the resizing has to be taken into account but not yet implemented, the assert will catch if the resizing is
@@ -1329,6 +1343,7 @@ bool storage_db_op_flush_sync(
             // The bucket might have been deleted in the meantime so get_key has to return true
             if (hashtable_mcmp_op_get_key(db->hashtable, bucket_index, &key, &key_size)) {
                 storage_db_op_delete(db, key, key_size);
+                slab_allocator_mem_free(key);
             }
         }
     }

src/storage/db/storage_db.h

@@ -341,6 +341,10 @@ bool storage_db_op_delete(
         char *key,
         size_t key_length);
 
+char *storage_db_op_random_key(
+        storage_db_t *db,
+        hashtable_key_size_t *key_size);
+
 int64_t storage_db_op_get_size(
         storage_db_t *db);
 

tests/unit_tests/data_structures/hashtable/mpmc/fixtures-hashtable-mpmc.h

@@ -119,11 +119,6 @@ hashtable_hash_quarter_t test_key_long_1_hash_quarter = test_key_long_1_hash_hal
     HASHTABLE_FREE(); \
 }
 
-#define HASHTABLE_TO_CHUNK_INDEX(bucket_index) \
-    (int)(bucket_index / HASHTABLE_MCMP_HALF_HASHES_CHUNK_SLOTS_COUNT)
-#define HASHTABLE_TO_BUCKET_INDEX(chunk_index, chunk_slot_index) \
-    (chunk_index * HASHTABLE_MCMP_HALF_HASHES_CHUNK_SLOTS_COUNT) + chunk_slot_index
-
 #define HASHTABLE_HALF_HASHES_CHUNK(chunk_index) \
     hashtable->ht_current->half_hashes_chunk[chunk_index]
 #define HASHTABLE_KEYS_VALUES(chunk_index, chunk_slot_index) \

tests/unit_tests/modules/redis/test-program-redis-commands.cpp

@@ -3358,13 +3358,39 @@ TEST_CASE("program.c-redis-commands", "[program-redis-commands]") {
                     std::vector<std::string>{"FLUSHDB"},
                     "+OK\r\n"));
 
+            REQUIRE(send_recv_resp_command_text(
+                    client_fd,
+                    std::vector<std::string>{"GET", "a_key"},
+                    "$-1\r\n"));
+
             REQUIRE(send_recv_resp_command_text(
                     client_fd,
                     std::vector<std::string>{"DBSIZE"},
                     ":0\r\n"));
         }
     }
 
+    SECTION("Redis - command - RANDOMKEY") {
+        SECTION("Empty database") {
+            REQUIRE(send_recv_resp_command_text(
+                    client_fd,
+                    std::vector<std::string>{"RANDOMKEY"},
+                    "$-1\r\n"));
+        }
+
+        SECTION("One key") {
+            REQUIRE(send_recv_resp_command_text(
+                    client_fd,
+                    std::vector<std::string>{"SET", "a_key", "b_value"},
+                    "+OK\r\n"));
+
+            REQUIRE(send_recv_resp_command_text(
+                    client_fd,
+                    std::vector<std::string>{"RANDOMKEY"},
+                    "$5\r\na_key\r\n"));
+        }
+    }
+
     SECTION("Redis - command - PING") {
         SECTION("Without value") {
             REQUIRE(send_recv_resp_command_text(