test: keep Redis fail_max concurrency test from timing out mid-run - #107
Merged
Merged
Conversation
CircuitRedisStorage persists opened_at as a whole-second Unix timestamp via calendar.timegm()/time.gmtime(), so a reset_timeout of 1 second can elapse anywhere from nearly 0s to 1s after the circuit opens. CircuitBreakerRedisConcurrencyTestCase issues 3 threads x 2000 guarded calls with fail_max=3000. After the breaker trips, the remaining calls still hit Redis. On slower hosts the truncated timeout expires, the breaker goes half-open, the trial call fails, and fail_counter is incremented again. Those extra counts are unrelated to the concurrent INCR overshoot the assertion allows (fail_max + num_threads) and fail the test, e.g. assert 3005 < 3000 + 3. On a riscv64 host (centiskorch) this test takes ~9-10s for the test body and ~14s for the pytest session. Raise this class's reset_timeout to 20s so the circuit stays open for the rest of the run with a small margin. test_half_open_thread_safety creates its own breaker and is unchanged. This is the same rounding already documented by test_successful_after_timeout, which sleeps 2s "since redis rounds to a second".
Owner
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CircuitRedisStorage persists opened_at as a whole-second Unix timestamp via calendar.timegm()/time.gmtime(), so a reset_timeout of 1 second can elapse anywhere from nearly 0s to 1s after the circuit opens.
CircuitBreakerRedisConcurrencyTestCase issues 3 threads x 2000 guarded calls with fail_max=3000. After the breaker trips, the remaining calls still hit Redis. On slower hosts the truncated timeout expires, the breaker goes half-open, the trial call fails, and fail_counter is incremented again. Those extra counts are unrelated to the concurrent INCR overshoot the assertion allows (fail_max + num_threads) and fail the test, e.g. assert 3005 < 3000 + 3.
On a riscv64 host this test takes ~9-10s for the test body and ~14s for the pytest session. Raise this class's reset_timeout to 20s so the circuit stays open for the rest of the run with a small margin. test_half_open_thread_safety creates its own breaker and is unchanged.