Skip to content

Commit

Permalink
Make KEYS can visit expired key in import-source state
Browse files Browse the repository at this point in the history
After valkey-io#1185, a client in import-source state can visit expired
key both in read commands and write commands, this commit handle
keyIsExpired function to handle import-source state as well, so
KEYS can visit the expired key.

This is not particularly important, but it ensures the definition,
also doing some cleanup around the test, verified that the client
can indeed visit the expired key.

Signed-off-by: Binbin <[email protected]>
  • Loading branch information
enjoy-binbin committed Nov 20, 2024
1 parent 4986310 commit 3032a99
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 12 deletions.
10 changes: 8 additions & 2 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ robj *dbRandomKey(serverDb *db) {
if (allvolatile && (server.primary_host || server.import_mode) && --maxtries == 0) {
/* If the DB is composed only of keys with an expire set,
* it could happen that all the keys are already logically
* expired in the repilca, so the function cannot stop because
* expired in the replica, so the function cannot stop because
* expireIfNeeded() is false, nor it can stop because
* dictGetFairRandomKey() returns NULL (there are keys to return).
* To prevent the infinite loop we do some tries, but if there
Expand Down Expand Up @@ -1798,7 +1798,13 @@ int keyIsExpiredWithDictIndex(serverDb *db, robj *key, int dict_index) {
/* Check if the key is expired. */
int keyIsExpired(serverDb *db, robj *key) {
int dict_index = getKVStoreIndexForKey(key->ptr);
return keyIsExpiredWithDictIndex(db, key, dict_index);
if (!keyIsExpiredWithDictIndex(db, key, dict_index)) return 0;

/* See expireIfNeededWithDictIndex for more details. */
if (server.primary_host == NULL && server.import_mode) {
if (server.current_client && server.current_client->flag.import_source) return 0;
}
return 1;
}

keyStatus expireIfNeededWithDictIndex(serverDb *db, robj *key, int flags, int dict_index) {
Expand Down
2 changes: 1 addition & 1 deletion src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -3586,7 +3586,7 @@ void clientCommand(client *c) {
"NO-TOUCH (ON|OFF)",
" Will not touch LRU/LFU stats when this mode is on.",
"IMPORT-SOURCE (ON|OFF)",
" Mark this connection as an import source if server.import_mode is true.",
" Mark this connection as an import source if import-mode is enabled.",
" Sync tools can set their connections into 'import-source' state to visit",
" expired keys.",
NULL};
Expand Down
23 changes: 14 additions & 9 deletions tests/unit/expire.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ start_server {tags {"expire"}} {

r set foo1 bar PX 1
r set foo2 bar PX 1
after 100
after 10

assert_equal [r dbsize] {2}

Expand Down Expand Up @@ -879,22 +879,27 @@ start_server {tags {"expire"}} {
assert_equal [r debug set-active-expire 1] {OK}
} {} {needs:debug}

test {RANDOMKEY can return expired key in import mode} {
test {Client can visit expired key in import-source state} {
r flushall

r config set import-mode yes
assert_equal [r client import-source on] {OK}

r set foo1 bar PX 1
r set foo1 1 PX 1
after 10

set client [valkey [srv "host"] [srv "port"] 0 $::tls]
if {!$::singledb} {
$client select 9
}
assert_equal [$client ttl foo1] {-2}
# Normal clients cannot visit expired key.
assert_equal [r get foo1] {}
assert_equal [r ttl foo1] {-2}
assert_equal [r dbsize] 1

# Client can visit expired key when in import-source state.
assert_equal [r client import-source on] {OK}
assert_equal [r ttl foo1] {0}
assert_equal [r get foo1] {1}
assert_equal [r incr foo1] {2}
assert_equal [r randomkey] {foo1}
assert_equal [r scan 0 match * count 10000] {0 foo1}
assert_equal [r keys *] {foo1}

assert_equal [r client import-source off] {OK}
r config set import-mode no
Expand Down

0 comments on commit 3032a99

Please sign in to comment.