Skip to content

Commit cc788b0

Browse files
committed
Changes and fixes for the new DELIFEQ command
DELIFEQ was added in valkey-io#1975 but had some issues. Do some refactoring to reduce the number of lines of code, write commands need to increment dirty and we can propagate it as DEL command. Signed-off-by: Binbin <binloveplay1314@qq.com>
1 parent 0012ca7 commit cc788b0

File tree

4 files changed

+26
-19
lines changed

4 files changed

+26
-19
lines changed

src/commands.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11341,7 +11341,7 @@ struct COMMAND_STRUCT serverCommandTable[] = {
1134111341
{MAKE_CMD("append","Appends a string to the value of a key. Creates the key if it doesn't exist.","O(1). The amortized time complexity is O(1) assuming the appended value is small and the already present value is of any size, since the dynamic string library used by the server will double the free space available on every reallocation.","2.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,APPEND_History,0,APPEND_Tips,0,appendCommand,3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_STRING,APPEND_Keyspecs,1,NULL,2),.args=APPEND_Args},
1134211342
{MAKE_CMD("decr","Decrements the integer value of a key by one. Uses 0 as initial value if the key doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,DECR_History,0,DECR_Tips,0,decrCommand,2,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_STRING,DECR_Keyspecs,1,NULL,1),.args=DECR_Args},
1134311343
{MAKE_CMD("decrby","Decrements a number from the integer value of a key. Uses 0 as initial value if the key doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,DECRBY_History,0,DECRBY_Tips,0,decrbyCommand,3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_STRING,DECRBY_Keyspecs,1,NULL,2),.args=DECRBY_Args},
11344-
{MAKE_CMD("delifeq","Delete key if value matches string.","O(1)","9.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,DELIFEQ_History,0,DELIFEQ_Tips,0,delifeqCommand,3,CMD_WRITE,ACL_CATEGORY_STRING,DELIFEQ_Keyspecs,1,NULL,2),.args=DELIFEQ_Args},
11344+
{MAKE_CMD("delifeq","Delete key if value matches string.","O(1)","9.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,DELIFEQ_History,0,DELIFEQ_Tips,0,delifeqCommand,3,CMD_FAST|CMD_WRITE,ACL_CATEGORY_STRING,DELIFEQ_Keyspecs,1,NULL,2),.args=DELIFEQ_Args},
1134511345
{MAKE_CMD("get","Returns the string value of a key.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,GET_History,0,GET_Tips,0,getCommand,2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_STRING,GET_Keyspecs,1,NULL,1),.args=GET_Args},
1134611346
{MAKE_CMD("getdel","Returns the string value of a key after deleting the key.","O(1)","6.2.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,GETDEL_History,0,GETDEL_Tips,0,getdelCommand,2,CMD_WRITE|CMD_FAST,ACL_CATEGORY_STRING,GETDEL_Keyspecs,1,NULL,1),.args=GETDEL_Args},
1134711347
{MAKE_CMD("getex","Returns the string value of a key after setting its expiration time.","O(1)","6.2.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,GETEX_History,0,GETEX_Tips,0,getexCommand,-2,CMD_WRITE|CMD_FAST,ACL_CATEGORY_STRING,GETEX_Keyspecs,1,NULL,2),.args=GETEX_Args},

src/commands/delifeq.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"arity": 3,
88
"function": "delifeqCommand",
99
"command_flags": [
10+
"FAST",
1011
"WRITE"
1112
],
1213
"acl_categories": [

src/t_string.c

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -384,30 +384,24 @@ void psetexCommand(client *c) {
384384
setGenericCommand(c, OBJ_PX | OBJ_ARGV3, c->argv[1], c->argv[3], c->argv[2], UNIT_MILLISECONDS, NULL, NULL, NULL);
385385
}
386386

387+
/* DELIFEQ key value */
387388
void delifeqCommand(client *c) {
388-
robj *existing_value = lookupKeyWrite(c->db, c->argv[1]);
389-
390-
if (existing_value == NULL) {
391-
addReplyLongLong(c, 0);
392-
return;
393-
}
394-
395-
if (checkType(c, existing_value, OBJ_STRING)) {
396-
/* Error reply already sent */
397-
return;
398-
}
389+
robj *o;
390+
if ((o = lookupKeyWriteOrReply(c, c->argv[1], shared.czero)) == NULL || checkType(c, o, OBJ_STRING)) return;
399391

400-
if (compareStringObjects(existing_value, c->argv[2]) != 0) {
401-
addReplyLongLong(c, 0);
392+
if (compareStringObjects(o, c->argv[2]) != 0) {
393+
addReply(c, shared.czero);
402394
return;
403395
}
404396

405-
if (!dbSyncDelete(c->db, c->argv[1])) {
406-
addReplyLongLong(c, 0);
407-
return;
408-
}
397+
serverAssert(dbSyncDelete(c->db, c->argv[1]));
409398

410-
addReplyLongLong(c, 1);
399+
/* Propagate as DEL command */
400+
rewriteClientCommandVector(c, 2, shared.del, c->argv[1]);
401+
signalModifiedKey(c, c->db, c->argv[1]);
402+
notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id);
403+
server.dirty++;
404+
addReply(c, shared.cone);
411405
}
412406

413407
int getGenericCommand(client *c) {

tests/unit/type/string.tcl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,18 @@ if {[string match {*jemalloc*} [s mem_allocator]]} {
777777
assert_error "WRONGTYPE*" {r delifeq foo "test"}
778778
}
779779

780+
test {DELIFEQ propagate as DEL command to replica} {
781+
set repl [attach_to_replication_stream]
782+
r set foo bar
783+
r delifeq foo bar
784+
assert_replication_stream $repl {
785+
{select *}
786+
{set foo bar}
787+
{del foo}
788+
}
789+
close_replication_stream $repl
790+
} {} {needs:repl}
791+
780792
if {[string match {*jemalloc*} [s mem_allocator]]} {
781793
test {Memory usage of embedded string value} {
782794
# Check that we can fit 9 bytes of key + value into a 32 byte

0 commit comments

Comments
 (0)