Skip to content

Fix timing issue in EXEC fail on lazy expired WATCHed key test#10332

Merged
oranagra merged 1 commit intoredis:unstablefrom
enjoy-binbin:fix_timing_issue_in_multi
Feb 23, 2022
Merged

Fix timing issue in EXEC fail on lazy expired WATCHed key test#10332
oranagra merged 1 commit intoredis:unstablefrom
enjoy-binbin:fix_timing_issue_in_multi

Conversation

@enjoy-binbin
Copy link
Contributor

The test will fail on slow machines (valgrind or FreeBsd).
Because in #10256 when WATCH is called on a key that's already
logically expired, we will add an expired flag, and we will
skip it in isWatchedKeyExpired check.

Apparently we need to increase the expiration time so that
the key can not expire logically then the WATCH is called.
Also added retries to make sure it doesn't fail. I suppose
100ms is enough in valgrind, tested locally, no need to retry.

The test will fail on slow machines (valgrind or FreeBsd).
Because in redis#10256 when WATCH is called on a key that's already
logically expired, we will add an `expired` flag, and we will
skip it in `isWatchedKeyExpired` check.

Apparently we need to increase the expiration time so that
the key can not expire logically then the WATCH is called.
Also added retries to make sure it doesn't fail. I suppose
100ms is enough in valgrind, tested locally, no need to retry.
@enjoy-binbin enjoy-binbin added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Feb 23, 2022
@oranagra oranagra merged commit 488aecb into redis:unstable Feb 23, 2022
@enjoy-binbin enjoy-binbin deleted the fix_timing_issue_in_multi branch February 23, 2022 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants