Make IO tests more stable #3497
Merged
Conversation
This file contains 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
The statement timeout needs to be cleaned up after each test that modifies it instead of before the test. Otherwise the changed timeout leaks into other tests.
wolfgangwalther
force-pushed
the
io-test-stability
branch
2 times, most recently
from
87ae687 to
334537d
Compare
…g out By increasing the delays in this test by factor 400x, postgrest will not swamp pg with connection retries after the failed schema cache anymore. This would happen because there is no backoff included after fatal errors. Once it does, the io tests hang indefinitely in CI.
wolfgangwalther
force-pushed
the
io-test-stability
branch
from
334537d to
c869433
Compare
|
I just pushed a no-change amended commit to trigger a new run. I also opened 4 PRs in my own fork with the same branch, to run the pipeline a few more times. Normally, this should trigger at least one failed IO test given the rate it occurred recently. Let's see whether that will still be the case or not. I also repeatedly ran the io tests locally in a loop for a few hours. This eventually kept hanging at the test fixed in this PR when I did it yesterday - but didn't happen this time around. |
|
All of the PRs passed the tests fine - I think this will work. Will merge. |
|
(please still have a look at the changes later, @steve-chavez, can't hurt) |
steve-chavez
reviewed
May 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This tries to address #3424 by preventing
test_admin_ready_includes_schema_cache_statefrom creating 100s of connection retries per second and thus somehow making postgrest unresponsive. This does not fix the underlying problem, but should at least make the tests not cause random failures anymore.One thing that is not clear to me is: Why are the IO tests only failing recently and not before? The test here has been in there for a long time already. It might be connected to the much extended logging, maybe the huge number of connection retries only becomes a problem due to the big number of logs generated that way.