-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
fix: bug in segcache cas implementation #68
Conversation
To check this in CI can you revert #58 here as well? It won't give 100% certainty but at least if that test fails then we know the issue is still there. |
This reverts commit 572ed35.
ok I reverted it as part of this PR |
Per discussion on discord... this is actually a little more buggy than it appeared. Will edit the diff |
Fix the CAS behavior for negative TTLs and improves the comments in the code for this weird case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code LGTM. I don't know enough to know if this is the best way to do things but it seems good to me.
BTW you still have a rustfmt failure.
fixes #59
Pretty sure this is the cause of the flakiness because if you set:
so you enforce a long wait in between running commands, then the test will fail every time on my machine (x86_64 Ubuntu WSL2).
I think the reason it doesn't fail all the time is due to machine load/architecture performance differences that cause the GET to occur either before or after the relatively immediate expiration of the key. If Pelikan was slower we'd probably have found this bug already.
The test that specifically fails (or at least the one that seems to be the cause of the nondeterministic failures) is here:
pelikan/src/server/segcache/tests/common.rs
Lines 56 to 66 in 4599e5f
If I run that sequence of commands on memcached, the
get
command will always succeed, regardless of how much time has passed.However, when I run those commands against
segcache
, the key is expired by the time I try toget
it (at least that's the likely behavior I see, based on the GET returning nothing)