192.168.0.20:9000> DBSIZE
(integer) 0
192.168.0.20:9000> select 1
OK
192.168.0.20:9000[1]> DBSIZE
(integer) 0
--> after swapdb from terminal 1
192.168.0.20:9000[1]> DBSIZE
(integer) 0
192.168.0.20:9000[1]> keys *
(empty array)
192.168.0.20:9000[1]>
Expected result: Terminal 2 should correctly display the result for DB0 after the swapdb.
Bug 2: Swapdb result is not recovered after restart in flash mode
192.168.0.20:9000> DBSIZE
(integer) 0
192.168.0.20:9000> set k0 v0
OK
192.168.0.20:9000> DBSIZE
(integer) 1
192.168.0.20:9000> keys *
1) "k0"
192.168.0.20:9000> SWAPDB 0 1
OK
192.168.0.20:9000> keys *
(empty array)
192.168.0.20:9000> DBSIZE
(integer) 0
192.168.0.20:9000> select 1
OK
192.168.0.20:9000[1]> DBSIZE
(integer) 1
192.168.0.20:9000[1]> keys *
1) "k0"
--> after server restart
192.168.0.20:9000[1]> keys *
Error: Server closed the connection
192.168.0.20:9000[1]> keys *
(empty array)
192.168.0.20:9000[1]> DBSIZE
(integer) 0
192.168.0.20:9000[1]>
Expected result: Even after restarting the server, it should recover the swapdb results correctly.
Bug 3: Watched key/swapdb unit tests failure
As part of this PR, I have fixed a known bug as below (line 1731):
int dbSwapDatabases(int id1, int id2) {
//...
std::swap(g_pserver->db[id1]->ready_keys, g_pserver->db[id2]->ready_keys);
std::swap(g_pserver->db[id1]->watched_keys, g_pserver->db[id2]->watched_keys);
//...
After this fix, following unit tests failed:
test {SWAPDB is able to touch the watched keys that exist} {
r flushall
r select 0
r set x 30
r watch x ;# make sure x (set to 30) doesn't change (SWAPDB will "delete" it)
r swapdb 0 1
r multi
r ping
r exec
} {}
test {SWAPDB is able to touch the watched keys that do not exist} {
r flushall
r select 1
r set x 30
r select 0
r watch x ;# make sure the key x (currently missing) doesn't change (SWAPDB will create it)
r swapdb 0 1
r multi
r ping
r exec
} {}
I have successfully verified all the above cases, and following are the test results after the fix:
This PR fixes couple of bugs related to swapdb. Following are the list of bugs:
Bug 1: Incorrect swapdb results in multiple clients (or cli) from different terminals
Terminal 1 (cli)
Terminal 2 (cli)
Expected result: Terminal 2 should correctly display the result for DB0 after the swapdb.
Bug 2: Swapdb result is not recovered after restart in flash mode
Expected result: Even after restarting the server, it should recover the swapdb results correctly.
Bug 3: Watched key/swapdb unit tests failure
As part of this PR, I have fixed a known bug as below (line 1731):
After this fix, following unit tests failed:
I have successfully verified all the above cases, and following are the test results after the fix:
Test bug 1 fix:
Terminal 1 (cli)
Terminal 2 (cli)
Test bug 2 fix:
Test bug 3 fix:
./runtest unit test passed
Additional information
The bug 2 was expected to be fixed in the previous PR https://github.com/Snapchat/KeyDB/pull/668. However, in the PR, the line https://github.com/Snapchat/KeyDB/blob/main/src/db.cpp#L1728 was still kept though it was expected to be removed. But, now as part of this PR we still don't need to remove that line but with a new solution of
storage_id.Also, for the bug 3, when the same unit test was executed manually, server crashed with the following stack race:
@paulmchen @hengku @CrazyTennisFan