Skip to content
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 flaky TestIngester_Push_CircuitBreaker_DeadlineExceeded test #9900

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

pr00se
Copy link
Contributor

@pr00se pr00se commented Nov 14, 2024

This test would sometimes fail:

--- FAIL: TestIngester_Push_CircuitBreaker_DeadlineExceeded (2.00s)
    --- FAIL: TestIngester_Push_CircuitBreaker_DeadlineExceeded/test_slow_push_with_initial_delay_disabled (2.00s)
        circuitbreaker_test.go:1015: 
            	Error Trace:	/go/src/github.com/grafana/mimir/pkg/ingester/circuitbreaker_test.go:1015
            	Error:      	Received unexpected error:
            	           	circuit breaker open on push request type with remaining delay 9.999979667s
            	Test:       	TestIngester_Push_CircuitBreaker_DeadlineExceeded/test_slow_push_with_initial_delay_disabled

The test assumes that it will execute one successful request, and then 4 failing requests. However, sometimes the first request would take longer to execute than the configured 100ms timeout, and the circuit breaker would close after the second overall request, rather than after the third.

The trivial fix is to increase the push timeout to a larger value, so that the initial request has time to complete successfully, even if the test runner is slow. This PR bumps it to 1s.

With this change I was able to run this test 1000x without failure locally:

go test -timeout 300m ./pkg/ingester/... -run "TestIngester_Push_CircuitBreaker_DeadlineExceeded" -race -count 1000 -failfast
ok  	github.com/grafana/mimir/pkg/ingester	4026.572s
ok  	github.com/grafana/mimir/pkg/ingester/activeseries	1.018s [no tests to run]
ok  	github.com/grafana/mimir/pkg/ingester/activeseries/model	1.018s [no tests to run]
ok  	github.com/grafana/mimir/pkg/ingester/client	1.047s [no tests to run]
make: Leaving directory '/go/src/github.com/grafana/mimir'

real	67m16.581s
user	0m0.121s
sys	0m0.026s

@pr00se pr00se requested a review from a team as a code owner November 14, 2024 01:54
Copy link
Contributor

@duricanikolic duricanikolic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thank you

@pr00se pr00se merged commit 04d5a05 into main Nov 14, 2024
29 checks passed
@pr00se pr00se deleted the fix-flaky-test branch November 14, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants