Writable replica may use the data of expired keys #6842
Description
for example, using SUNIONSTORE:
master:
127.0.0.1:6379> DEBUG set-active-expire 0
OK
127.0.0.1:6379> SADD s1 a b c
(integer) 3
127.0.0.1:6379> SADD s2 1 2 3
(integer) 3
127.0.0.1:6379> EXPIRE s1 5
(integer) 1
replica:
127.0.0.1:12345> REPLICAOF localhost 6379
OK
127.0.0.1:12345> CONFIG SET replica-read-only no
OK
127.0.0.1:12345> DEBUG set-active-expire 0
OK
127.0.0.1:12345> SMEMBERS s1
1) "b"
2) "a"
3) "c"
127.0.0.1:12345> SMEMBERS s2
1) "1"
2) "2"
3) "3"
127.0.0.1:12345> DEBUG sleep 6
OK
(6.00s)
127.0.0.1:12345> SMEMBERS s1
(empty array)
127.0.0.1:12345> SUNIONSTORE s3 s1 s2
(integer) 6
127.0.0.1:12345> SMEMBERS s3
1) "1"
2) "c"
3) "3"
4) "b"
5) "a"
6) "2"
The problem is that lookupKeyRead behaves differently than lookupKeyWrite in the context of writable replicas with expiration.
Actually, I'm not even sure why the source sets are opened in write-mode.. Is there a reason for that?
The same problem affects SUNIONSTORE, SDIFFSTORE, SINTERSTORE, ZUNIONSTORE and ZINTERSTORE. For those commands it's enough to just use lookupKeyRead but the occurs in other commands that modify existing value (e.g. INCR, APPEND, etc.):
master:
127.0.0.1:6379> DEBUG set-active-expire 0
OK
127.0.0.1:6379> SET c 100
OK
127.0.0.1:6379> EXPIRE c 5
(integer) 1
replica:
127.0.0.1:12345> REPLICAOF localhost 6379
OK
127.0.0.1:12345> DEBUG set-active-expire 0
OK
127.0.0.1:12345> CONFIG SET replica-read-only no
OK
127.0.0.1:12345> GET c
"100"
127.0.0.1:12345> DEBUG sleep 6
OK
(6.00s)
127.0.0.1:12345> GET c
(nil)
127.0.0.1:12345> INCR c
(integer) 101
The solution is either to mirror that logic from lookupKeyRead into lookupKeyWrite, or eliminate the latter and share code.