diff --git a/src/commands.c b/src/commands.c index 5db70b487..7698f2055 100644 --- a/src/commands.c +++ b/src/commands.c @@ -7210,11 +7210,11 @@ struct redisCommand redisCommandTable[] = { {"pexpireat","Set the expiration for a key as a UNIX timestamp specified in milliseconds","O(1)","2.6.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,PEXPIREAT_History,PEXPIREAT_tips,pexpireatCommand,-3,CMD_WRITE|CMD_FAST,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RW|CMD_KEY_UPDATE,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=PEXPIREAT_Args}, {"pexpiretime","Get the expiration Unix timestamp for a key in milliseconds","O(1)","7.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,PEXPIRETIME_History,PEXPIRETIME_tips,pexpiretimeCommand,2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=PEXPIRETIME_Args}, {"pttl","Get the time to live for a key in milliseconds","O(1)","2.6.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,PTTL_History,PTTL_tips,pttlCommand,2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=PTTL_Args}, -{"randomkey","Return a random key from the keyspace","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,RANDOMKEY_History,RANDOMKEY_tips,randomkeyCommand,1,CMD_READONLY,ACL_CATEGORY_KEYSPACE}, +{"randomkey","Return a random key from the keyspace","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,RANDOMKEY_History,RANDOMKEY_tips,randomkeyCommand,1,CMD_READONLY|CMD_TOUCHES_ARBITRARY_KEYS,ACL_CATEGORY_KEYSPACE}, {"rename","Rename a key","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,RENAME_History,RENAME_tips,renameCommand,3,CMD_WRITE,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RW|CMD_KEY_ACCESS|CMD_KEY_DELETE,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}},{NULL,CMD_KEY_OW|CMD_KEY_UPDATE,KSPEC_BS_INDEX,.bs.index={2},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=RENAME_Args}, {"renamenx","Rename a key, only if the new key does not exist","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,RENAMENX_History,RENAMENX_tips,renamenxCommand,3,CMD_WRITE|CMD_FAST,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RW|CMD_KEY_ACCESS|CMD_KEY_DELETE,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}},{NULL,CMD_KEY_OW|CMD_KEY_INSERT,KSPEC_BS_INDEX,.bs.index={2},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=RENAMENX_Args}, {"restore","Create a key using the provided serialized value, previously obtained using DUMP.","O(1) to create the new key and additional O(N*M) to reconstruct the serialized value, where N is the number of Redis objects composing the value and M their average size. For small string values the time complexity is thus O(1)+O(1*M) where M is small, so simply O(1). However for sorted set values the complexity is O(N*M*log(N)) because inserting values into sorted sets is O(log(N)).","2.6.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,RESTORE_History,RESTORE_tips,restoreCommand,-4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_KEYSPACE|ACL_CATEGORY_DANGEROUS,{{NULL,CMD_KEY_OW|CMD_KEY_UPDATE,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=RESTORE_Args}, -{"scan","Incrementally iterate the keys space","O(1) for every call. O(N) for a complete iteration, including enough command calls for the cursor to return back to 0. N is the number of elements inside the collection.","2.8.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,SCAN_History,SCAN_tips,scanCommand,-2,CMD_READONLY,ACL_CATEGORY_KEYSPACE,.args=SCAN_Args}, +{"scan","Incrementally iterate the keys space","O(1) for every call. O(N) for a complete iteration, including enough command calls for the cursor to return back to 0. N is the number of elements inside the collection.","2.8.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,SCAN_History,SCAN_tips,scanCommand,-2,CMD_READONLY|CMD_TOUCHES_ARBITRARY_KEYS,ACL_CATEGORY_KEYSPACE,.args=SCAN_Args}, {"sort","Sort the elements in a list, set or sorted set","O(N+M*log(M)) where N is the number of elements in the list or set to sort, and M the number of returned elements. When the elements are not sorted, complexity is O(N).","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,SORT_History,SORT_tips,sortCommand,-2,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_SET|ACL_CATEGORY_SORTEDSET|ACL_CATEGORY_LIST|ACL_CATEGORY_DANGEROUS,{{NULL,CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}},{"For the optional BY/GET keyword. It is marked 'unknown' because the key names derive from the content of the key we sort",CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_UNKNOWN,{{0}},KSPEC_FK_UNKNOWN,{{0}}},{"For the optional STORE keyword. It is marked 'unknown' because the keyword can appear anywhere in the argument array",CMD_KEY_OW|CMD_KEY_UPDATE,KSPEC_BS_UNKNOWN,{{0}},KSPEC_FK_UNKNOWN,{{0}}}},sortGetKeys,.args=SORT_Args}, {"sort_ro","Sort the elements in a list, set or sorted set. Read-only variant of SORT.","O(N+M*log(M)) where N is the number of elements in the list or set to sort, and M the number of returned elements. When the elements are not sorted, complexity is O(N).","7.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,SORT_RO_History,SORT_RO_tips,sortroCommand,-2,CMD_READONLY,ACL_CATEGORY_SET|ACL_CATEGORY_SORTEDSET|ACL_CATEGORY_LIST|ACL_CATEGORY_DANGEROUS,{{NULL,CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}},{"For the optional BY/GET keyword. It is marked 'unknown' because the key names derive from the content of the key we sort",CMD_KEY_RO|CMD_KEY_ACCESS,KSPEC_BS_UNKNOWN,{{0}},KSPEC_FK_UNKNOWN,{{0}}}},sortROGetKeys,.args=SORT_RO_Args}, {"touch","Alters the last access time of a key(s). Returns the number of existing keys specified.","O(N) where N is the number of keys that will be touched.","3.2.1",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_GENERIC,TOUCH_History,TOUCH_tips,touchCommand,-2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_KEYSPACE,{{NULL,CMD_KEY_RO,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={-1,1,0}}},.args=TOUCH_Args}, diff --git a/src/commands/randomkey.json b/src/commands/randomkey.json index 93473968c..ceaa3c886 100644 --- a/src/commands/randomkey.json +++ b/src/commands/randomkey.json @@ -7,7 +7,8 @@ "arity": 1, "function": "randomkeyCommand", "command_flags": [ - "READONLY" + "READONLY", + "TOUCHES_ARBITRARY_KEYS" ], "acl_categories": [ "KEYSPACE" diff --git a/src/commands/scan.json b/src/commands/scan.json index a689bacca..e0856d0f0 100644 --- a/src/commands/scan.json +++ b/src/commands/scan.json @@ -13,7 +13,8 @@ ] ], "command_flags": [ - "READONLY" + "READONLY", + "TOUCHES_ARBITRARY_KEYS" ], "acl_categories": [ "KEYSPACE" diff --git a/src/db.c b/src/db.c index 9b759dfbb..e1d28a3e4 100644 --- a/src/db.c +++ b/src/db.c @@ -1622,8 +1622,8 @@ void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj) { * because call() handles server.also_propagate(); or * 2. Outside of call(): Example: Active-expire, eviction. * In this the caller must remember to call - * propagatePendingCommands, preferably at the end of - * the deletion batch, so that DELs will be wrapped + * postExecutionUnitOperations, preferably just after a + * single deletion batch, so that DELs will NOT be wrapped * in MULTI/EXEC */ void propagateDeletion(redisDb *db, robj *key, int lazy) { robj *argv[2]; diff --git a/src/server.c b/src/server.c index faf010135..85c48c41d 100644 --- a/src/server.c +++ b/src/server.c @@ -3304,29 +3304,35 @@ static void propagatePendingCommands() { int j; redisOp *rop; - int multi_emitted = 0; - /* Wrap the commands in server.also_propagate array, - * but don't wrap it if we are already in MULTI context, - * in case the nested MULTI/EXEC. - * - * And if the array contains only one command, no need to - * wrap it, since the single command is atomic. */ - if (server.also_propagate.numops > 1) { + /* If we got here it means we have finished an execution-unit. + * If that unit has caused propagation of multiple commands, they + * should be propagated as a transaction */ + int transaction = server.also_propagate.numops > 1; + + /* In case a command that may modify random keys was run *directly* + * (i.e. not from within a script, MULTI/EXEC, RM_Call, etc.) we want + * to avoid using a transaction (much like active-expire) */ + if (server.current_client && + server.current_client->cmd && + server.current_client->cmd->flags & CMD_TOUCHES_ARBITRARY_KEYS) + { + transaction = 0; + } + + if (transaction) { /* We use dbid=-1 to indicate we do not want to replicate SELECT. * It'll be inserted together with the next command (inside the MULTI) */ propagateNow(-1,&shared.multi,1,PROPAGATE_AOF|PROPAGATE_REPL); - multi_emitted = 1; } - for (j = 0; j < server.also_propagate.numops; j++) { rop = &server.also_propagate.ops[j]; serverAssert(rop->target); propagateNow(rop->dbid,rop->argv,rop->argc,rop->target); } - if (multi_emitted) { + if (transaction) { /* We use dbid=-1 to indicate we do not want to replicate select */ propagateNow(-1,&shared.exec,1,PROPAGATE_AOF|PROPAGATE_REPL); } @@ -4515,6 +4521,7 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) { {CMD_NO_MULTI, "no_multi"}, {CMD_MOVABLE_KEYS, "movablekeys"}, {CMD_ALLOW_BUSY, "allow_busy"}, + /* {CMD_TOUCHES_ARBITRARY_KEYS, "TOUCHES_ARBITRARY_KEYS"}, Hidden on purpose */ {0,NULL} }; addReplyCommandFlags(c, cmd->flags, flagNames); diff --git a/src/server.h b/src/server.h index f639ce150..6ff882d4c 100644 --- a/src/server.h +++ b/src/server.h @@ -227,6 +227,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; * Populated by populateCommandLegacyRangeSpec. */ #define CMD_ALLOW_BUSY ((1ULL<<26)) #define CMD_MODULE_GETCHANNELS (1ULL<<27) /* Use the modules getchannels interface. */ +#define CMD_TOUCHES_ARBITRARY_KEYS (1ULL<<28) /* Command flags that describe ACLs categories. */ #define ACL_CATEGORY_KEYSPACE (1ULL<<0) @@ -2225,6 +2226,12 @@ typedef int redisGetKeysProc(struct redisCommand *cmd, robj **argv, int argc, ge * * CMD_NO_MULTI: The command is not allowed inside a transaction * + * CMD_ALLOW_BUSY: The command can run while another command is running for + * a long time (timedout script, module command that yields) + * + * CMD_TOUCHES_ARBITRARY_KEYS: The command may touch (and cause lazy-expire) + * arbitrary key (i.e not provided in argv) + * * The following additional flags are only used in order to put commands * in a specific ACL category. Commands can have multiple ACL categories. * See redis.conf for the exact meaning of each. diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl index 2b0fc22e9..e23e2a89a 100644 --- a/tests/unit/expire.tcl +++ b/tests/unit/expire.tcl @@ -768,4 +768,46 @@ start_server {tags {"expire"}} { close_replication_stream $repl assert_equal [r debug set-active-expire 1] {OK} } {} {needs:debug} + + test {SCAN: Lazy-expire should not be wrapped in MULTI/EXEC} { + r debug set-active-expire 0 + r flushall + + r set foo1 bar PX 1 + r set foo2 bar PX 1 + after 2 + + set repl [attach_to_replication_stream] + + r scan 0 + + assert_replication_stream $repl { + {select *} + {del foo*} + {del foo*} + } + close_replication_stream $repl + assert_equal [r debug set-active-expire 1] {OK} + } {} {needs:debug} + + test {RANDOMKEY: Lazy-expire should not be wrapped in MULTI/EXEC} { + r debug set-active-expire 0 + r flushall + + r set foo1 bar PX 1 + r set foo2 bar PX 1 + after 2 + + set repl [attach_to_replication_stream] + + r randomkey + + assert_replication_stream $repl { + {select *} + {del foo*} + {del foo*} + } + close_replication_stream $repl + assert_equal [r debug set-active-expire 1] {OK} + } {} {needs:debug} }