Skip to content

Commit

Permalink
Support empty set
Browse files Browse the repository at this point in the history
Signed-off-by: Seungmin Lee <sungming@amazon.com>
  • Loading branch information
Seungmin Lee committed Dec 16, 2024
1 parent ad24220 commit d856660
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
3 changes: 0 additions & 3 deletions src/intset.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
25 changes: 21 additions & 4 deletions src/rdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand All @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand Down
14 changes: 8 additions & 6 deletions src/t_set.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 10 additions & 1 deletion tests/support/server.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
105 changes: 105 additions & 0 deletions tests/unit/type/set.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""] ] {

Expand Down
25 changes: 25 additions & 0 deletions valkey.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit d856660

Please sign in to comment.