diff --git a/src/data_structures/hashtable/mcmp/hashtable.h b/src/data_structures/hashtable/mcmp/hashtable.h index ca5946b71..f78e28540 100644 --- a/src/data_structures/hashtable/mcmp/hashtable.h +++ b/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; diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_delete.c b/src/data_structures/hashtable/mcmp/hashtable_op_delete.c index f47369adc..0af7880a6 100644 --- a/src/data_structures/hashtable/mcmp/hashtable_op_delete.c +++ b/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; diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_get_key.c b/src/data_structures/hashtable/mcmp/hashtable_op_get_key.c index 36bc75f64..3f83056af 100644 --- a/src/data_structures/hashtable/mcmp/hashtable_op_get_key.c +++ b/src/data_structures/hashtable/mcmp/hashtable_op_get_key.c @@ -12,12 +12,16 @@ #include #include #include +#include #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; } diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.c b/src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.c new file mode 100644 index 000000000..dd0c64467 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include + +#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); +} diff --git a/src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.h b/src/data_structures/hashtable/mcmp/hashtable_op_get_random_key.h new file mode 100644 index 000000000..b8223f5eb --- /dev/null +++ b/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 diff --git a/src/data_structures/hashtable/mcmp/hashtable_support_op_arch.c b/src/data_structures/hashtable/mcmp/hashtable_support_op_arch.c index a8a7f34dd..d0f09deeb 100644 --- a/src/data_structures/hashtable/mcmp/hashtable_support_op_arch.c +++ b/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, diff --git a/src/module/redis/command/module_redis_command_randomkey.c b/src/module/redis/command/module_redis_command_randomkey.c new file mode 100644 index 000000000..507f29a18 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include + +#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; +} diff --git a/src/storage/db/storage_db.c b/src/storage/db/storage_db.c index d2de85c81..de6f6b317 100644 --- a/src/storage/db/storage_db.c +++ b/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); } } } diff --git a/src/storage/db/storage_db.h b/src/storage/db/storage_db.h index aa770da2f..7d507b2a6 100644 --- a/src/storage/db/storage_db.h +++ b/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); diff --git a/tests/unit_tests/data_structures/hashtable/mpmc/fixtures-hashtable-mpmc.h b/tests/unit_tests/data_structures/hashtable/mpmc/fixtures-hashtable-mpmc.h index 64dc40bb1..8f7e9836b 100644 --- a/tests/unit_tests/data_structures/hashtable/mpmc/fixtures-hashtable-mpmc.h +++ b/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) \ diff --git a/tests/unit_tests/modules/redis/test-program-redis-commands.cpp b/tests/unit_tests/modules/redis/test-program-redis-commands.cpp index 98d577b90..d385006b7 100644 --- a/tests/unit_tests/modules/redis/test-program-redis-commands.cpp +++ b/tests/unit_tests/modules/redis/test-program-redis-commands.cpp @@ -3358,6 +3358,11 @@ TEST_CASE("program.c-redis-commands", "[program-redis-commands]") { std::vector{"FLUSHDB"}, "+OK\r\n")); + REQUIRE(send_recv_resp_command_text( + client_fd, + std::vector{"GET", "a_key"}, + "$-1\r\n")); + REQUIRE(send_recv_resp_command_text( client_fd, std::vector{"DBSIZE"}, @@ -3365,6 +3370,27 @@ TEST_CASE("program.c-redis-commands", "[program-redis-commands]") { } } + SECTION("Redis - command - RANDOMKEY") { + SECTION("Empty database") { + REQUIRE(send_recv_resp_command_text( + client_fd, + std::vector{"RANDOMKEY"}, + "$-1\r\n")); + } + + SECTION("One key") { + REQUIRE(send_recv_resp_command_text( + client_fd, + std::vector{"SET", "a_key", "b_value"}, + "+OK\r\n")); + + REQUIRE(send_recv_resp_command_text( + client_fd, + std::vector{"RANDOMKEY"}, + "$5\r\na_key\r\n")); + } + } + SECTION("Redis - command - PING") { SECTION("Without value") { REQUIRE(send_recv_resp_command_text(