-
Notifications
You must be signed in to change notification settings - Fork 23.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redis slaves, while not allowed to expire keys without master input, should reply to clients consistently with the key expire information. #187
Comments
@shafreeck This commit never hit master.
|
same situation can be reproduced easier by:
|
For info this step is not necessary IME: On a busy redis cluster, this was my experience (redis_version:4.0.8):
|
indeed this was solved long ago. The only problem mentioned here that's not already solved in the Lua issue, but that's in some way solved by b089ba9 and other changes in that area |
Facts:
The above points are good and make the Redis expire semantic simpler from the point of view of the append only file and replication.
However consider this: a Redis master could have 50 million keys with an expire set, however all the keys but just one have the expire set in the future. The only key with a short expire is unlikely to be found by random sampling, but if a client will access this key, the key will be evicted by the lookupKey...() function and everything will work as expected.
But if a client will try to access this key in a slave it will found it existing: lookupKey...() can't expire keys in a slave.
How to fix that?
The obvious way to fix this issue is to make the slave more aware of what it is doing. It could simply compute if the key is expired so that lookupKey...() will return NULL and the client will be non existing from the point of view of the client request, even if the salve will not evict the key form the database, waiting for the DEL from the master instance in order to do so. This way we don't have time dependent behaviors when the slave receives, for example, an SUNIONSTORE command that has that key on its arguments (the key may not be already expired on the master since the two host timers may not be perfectly synchronized, or because there is a delay in the replication link, and so forth).
This fix is just one line and trivial, but there are a few problems with it:
This requires a refactoring of the internal API, so for now it is considered a post-3.0 stuff, but it is better to write it down and think about ways to resolve this issue in the future.
The text was updated successfully, but these errors were encountered: