From d8566609e85810d01546060694ee2f00a53be2b3 Mon Sep 17 00:00:00 2001 From: Seungmin Lee Date: Tue, 10 Dec 2024 11:25:30 -0800 Subject: [PATCH] Support empty set Signed-off-by: Seungmin Lee --- src/config.c | 1 + src/intset.c | 3 -- src/rdb.c | 25 ++++++++-- src/server.h | 4 ++ src/t_set.c | 14 +++--- tests/support/server.tcl | 11 +++- tests/unit/type/set.tcl | 105 +++++++++++++++++++++++++++++++++++++++ valkey.conf | 25 ++++++++++ 8 files changed, 174 insertions(+), 14 deletions(-) diff --git a/src/config.c b/src/config.c index cc0f8d2dd8..ace97e3057 100644 --- a/src/config.c +++ b/src/config.c @@ -3206,6 +3206,7 @@ standardConfig static_configs[] = { createBoolConfig("cluster-slot-stats-enabled", NULL, MODIFIABLE_CONFIG, server.cluster_slot_stats_enabled, 0, NULL, NULL), createBoolConfig("hide-user-data-from-log", NULL, MODIFIABLE_CONFIG, server.hide_user_data_from_log, 1, NULL, NULL), createBoolConfig("import-mode", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, server.import_mode, 0, NULL, NULL), + createBoolConfig("allow-empty-set", NULL, MODIFIABLE_CONFIG, server.allow_empty_set, 0, NULL, NULL), /* String Configs */ createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL), diff --git a/src/intset.c b/src/intset.c index ddddae221d..6258f5b195 100644 --- a/src/intset.c +++ b/src/intset.c @@ -320,9 +320,6 @@ int intsetValidateIntegrity(const unsigned char *p, size_t size, int deep) { uint32_t count = intrev32ifbe(is->length); if (sizeof(*is) + count * record_size != size) return 0; - /* check that the set is not empty. */ - if (count == 0) return 0; - if (!deep) return 1; /* check that there are no dup or out of order records. */ diff --git a/src/rdb.c b/src/rdb.c index 5fb77a2897..08dcabcdad 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1900,7 +1900,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { } else if (rdbtype == RDB_TYPE_SET) { /* Read Set value */ if ((len = rdbLoadLen(rdb, NULL)) == RDB_LENERR) return NULL; - if (len == 0) goto emptykey; + if (len == 0 && !server.allow_empty_set) goto emptykey; /* Use a regular set when there are too many entries. */ size_t max_entries = server.set_max_intset_entries; @@ -2356,6 +2356,18 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { } o->type = OBJ_SET; o->encoding = OBJ_ENCODING_INTSET; + + if (intsetLen((intset *)encoded) == 0) { + if (!server.allow_empty_set) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } + + zfree(encoded); + o->ptr = intsetNew(); + } if (intsetLen(o->ptr) > server.set_max_intset_entries) setTypeConvert(o, OBJ_ENCODING_HASHTABLE); break; case RDB_TYPE_SET_LISTPACK: @@ -2371,10 +2383,15 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { o->encoding = OBJ_ENCODING_LISTPACK; if (setTypeSize(o) == 0) { + if (!server.allow_empty_set) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } + zfree(encoded); - o->ptr = NULL; - decrRefCount(o); - goto emptykey; + o->ptr = lpNew(0); } if (setTypeSize(o) > server.set_max_listpack_entries) setTypeConvert(o, OBJ_ENCODING_HASHTABLE); break; diff --git a/src/server.h b/src/server.h index dc4d2e8808..d10228ee73 100644 --- a/src/server.h +++ b/src/server.h @@ -1930,6 +1930,10 @@ struct valkeyServer { invocation of the event loop. */ unsigned int max_new_conns_per_cycle; /* The maximum number of tcp connections that will be accepted during each invocation of the event loop. */ + int allow_empty_set; /* Flag to control whether empty set is allowed in the database + 1 empty sets are preserved in database even after all elements are removed + 0 by default, key for the set is deleted when it becomes empty */ + /* AOF persistence */ int aof_enabled; /* AOF configuration */ int aof_state; /* AOF_(ON|OFF|WAIT_REWRITE) */ diff --git a/src/t_set.c b/src/t_set.c index 4279baf82f..9b5344b6b2 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -624,8 +624,10 @@ void sremCommand(client *c) { if (setTypeRemove(set, c->argv[j]->ptr)) { deleted++; if (setTypeSize(set) == 0) { - dbDelete(c->db, c->argv[1]); - keyremoved = 1; + if (!server.allow_empty_set) { + dbDelete(c->db, c->argv[1]); + keyremoved = 1; + } break; } } @@ -668,8 +670,8 @@ void smoveCommand(client *c) { } notifyKeyspaceEvent(NOTIFY_SET, "srem", c->argv[1], c->db->id); - /* Remove the src set from the database when empty */ - if (setTypeSize(srcset) == 0) { + /* Remove the src set from the database when empty and allow-empty-set is disabled */ + if (!server.allow_empty_set && setTypeSize(srcset) == 0) { dbDelete(c->db, c->argv[1]); notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id); } @@ -971,8 +973,8 @@ void spopCommand(client *c) { addReplyBulk(c, ele); decrRefCount(ele); - /* Delete the set if it's empty */ - if (setTypeSize(set) == 0) { + /* Delete the set if it's empty and allow-empty-set is disabled */ + if (!server.allow_empty_set && setTypeSize(set) == 0) { dbDelete(c->db, c->argv[1]); notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id); } diff --git a/tests/support/server.tcl b/tests/support/server.tcl index 7257339042..131a8b7356 100644 --- a/tests/support/server.tcl +++ b/tests/support/server.tcl @@ -769,7 +769,7 @@ proc start_multiple_servers {num options code} { uplevel 1 $code } -proc restart_server {level wait_ready rotate_logs {reconnect 1} {shutdown sigterm}} { +proc restart_server {level wait_ready rotate_logs {reconnect 1} {shutdown sigterm} {config_override {}}} { set srv [lindex $::servers end+$level] if {$shutdown ne {sigterm}} { catch {[dict get $srv "client"] shutdown $shutdown} @@ -798,6 +798,15 @@ proc restart_server {level wait_ready rotate_logs {reconnect 1} {shutdown sigter set config_file [dict get $srv "config_file"] + # Apply config updates if provided + if {[llength $config_override] > 0} { + set fd [open $config_file "a"] + foreach {key value} $config_override { + puts $fd "$key $value" + } + close $fd + } + set pid [spawn_server $config_file $stdout $stderr {}] # check that the server actually started diff --git a/tests/unit/type/set.tcl b/tests/unit/type/set.tcl index 1871ec9b4d..4fda1a376f 100644 --- a/tests/unit/type/set.tcl +++ b/tests/unit/type/set.tcl @@ -1168,6 +1168,111 @@ foreach type {single multiple single_multiple} { } } +start_server { + tags {"set external:skip"} + overrides { + "set-max-intset-entries" 1 + "allow-empty-set" "yes" + } +} { + proc save_load_rdb {{config_override {}}} { + r save + restart_server 0 true false true now $config_override + wait_done_loading r + } + proc create_emptyset {key members} { + r del $key + foreach member $members { r sadd $key $member } + foreach member $members { r srem $key $member } + } + proc verify_emptyset {key} { + assert_equal 0 [r scard $key] + assert_equal 1 [r exists $key] + } + + test {EMTPYSET save and reload empty listset from RDB} { + r flushdb + create_emptyset listset {a} + verify_emptyset listset + assert_equal listpack [r object encoding listset] + save_load_rdb + verify_emptyset listset + } + + test {EMTPYSET save and reload empty intset from RDB} { + r flushdb + create_emptyset intset {1} + verify_emptyset intset + assert_equal intset [r object encoding intset] + save_load_rdb + verify_emptyset intset + } + + test {EMTPYSET save and reload empty hashset from RDB} { + r flushdb + create_emptyset hashset {1 2} + verify_emptyset hashset + assert_equal hashtable [r object encoding hashset] + save_load_rdb + verify_emptyset hashset + } + + test {EMTPYSET smove to make empty set} { + r flushdb + assert_equal 1 [r sadd myset a] + assert_equal 1 [r sadd myset2 b] + assert_equal 1 [r smove myset myset2 a] + assert_equal 2 [r scard myset2] + verify_emptyset myset + save_load_rdb + verify_emptyset myset + } + + test {EMTPYSET spop to make empty set} { + r flushdb + assert_equal 1 [r sadd myset a] + assert_equal a [r spop myset] + verify_emptyset myset + save_load_rdb + verify_emptyset myset + } + + test {EMTPYSET sunion with empty set} { + r flushdb + create_emptyset myset {a} + verify_emptyset myset + assert_equal 1 [r sadd myset2 b] + assert_equal b [r sunion myset myset2] + save_load_rdb + verify_emptyset myset + } + + test {EMTPYSET skip loading empty sets when allow-empty-set is no} { + r flushdb + create_emptyset intset {1} + verify_emptyset intset + create_emptyset listset {a} + verify_emptyset listset + create_emptyset hashset {1 2} + verify_emptyset hashset + set config_override { + "allow-empty-set" "no" + } + save_load_rdb $config_override + wait_for_log_messages 0 {"*rdbLoadObject skipping empty key: intset*"} 0 1000 50 + wait_for_log_messages 0 {"*rdbLoadObject skipping empty key: listset*"} 0 1000 50 + wait_for_log_messages 0 {"*rdbLoadObject skipping empty key: hashset*"} 0 1000 50 + wait_for_log_messages 0 {"*Done loading RDB, keys loaded: 0, keys expired: 0, empty keys skipped: 3.*"} 0 1000 50 + + assert_equal 0 [r exists intset] + assert_equal 0 [r scard intset] + assert_equal 0 [r exists listset] + assert_equal 0 [r scard listset] + assert_equal 0 [r exists hashset] + assert_equal 0 [r scard hashset] + } +} + run_solo {set-large-memory} { start_server [list overrides [list save ""] ] { diff --git a/valkey.conf b/valkey.conf index e23aea39de..eed2193a34 100644 --- a/valkey.conf +++ b/valkey.conf @@ -496,6 +496,31 @@ locale-collate "" # # extended-redis-compatibility no +# This configuration controls whether Valkey allows the representation of empty sets in the database. +# By default, when the last element of a set is removed, the key associated with the set is deleted. +# Enabling this option allows Valkey to retain the key and store an empty set instead. +# +# This can be useful for scenarios where an empty set has semantic meaning in the application, +# such as indicating that a query was executed but returned no results. +# +# When this option is enabled: +# - Empty sets are preserved in the database, and their key will not be automatically deleted. +# - The `SCARD` command will return 0 for these keys. +# - These keys will remain in the database until explicitly deleted using the `DEL` command. +# +# Note: +# - This setting may result in additional memory usage due to the retention of keys for empty sets. +# - Ensure client applications handle empty sets correctly, as their presence differs from the absence of a key. +# - When loading RDB files, empty sets are skipped by default unless this option is enabled. +# This ensures backward compatibility with existing RDB files created in versions where empty +# sets were not explicitly supported. +# - Ensure that this setting is consistently enabled if your application relies on the presence of +# empty sets during RDB restores. +# +# Default: no +# +# allow-empty-set no + ################################ SNAPSHOTTING ################################ # Save the DB to disk.