Fix flaky test case for absolute TTL replication #9069
Conversation
|
@ny0312 I think it would be okay to merge it now so we can start watching the nightly runs, It's Oran's weekend now. |
oranagra
left a comment
There was a problem hiding this comment.
Good catch!!!
isn't it sufficient to just add a FLUSHALL to that test before attaching to the replication stream.
Ie. Instead of this massive LOC change?
That would also be sufficient. It depends on which convention we want to establish when it comes to isolating a test case from all side effects of other test cases:
Personally I would prefer (1). It is slower. But it is explicit, simple and clean. It frees an author of a new test from having to understand the side effects of all preceding tests in order to properly "clean up" their side effects. I would encourage all contributors to follow this convention. |
|
Spinning up a dedicated server for each test would slow down the test suite a lot. |
So your preferred convention is "re-use servers across test cases unless it is impossible or too hard to make work". I agree that this approach would give us the fastest tests therefore shortest feedback cycle. It would more friendly to contributors of Redis, which is very important. I will change to |
e751c0f to
efe45cf
Compare
The root cause is that one test (`5 keys in, 5 keys out`) is leaking a volatile key that can expire while another later test(`All TTL in commands are propagated as absolute timestamp in replication stream`) is running. Such leaked expiration injects an unexpected `DEL` command into the replication command during the later test, causing it to fail. The fixes are two fold: 1. Plug the leak in the first test. 2. Add FLUSHALL to the later test, to avoid future interference from other tests.
Fix flaky test case reported here: #8474
The root cause is that one test (
5 keys in, 5 keys out) is leaking a volatile key that can expire while another later test(All TTL in commands are propagated as absolute timestamp in replication stream) is running. Such leaked expiration injects an unexpectedDELcommand into the replication command during the later test, causing it to fail.The fixes are two fold: