fix(ci): give the db2 testcontainer boot real wait headroom - #44399
Open
sadpandajoe wants to merge 2 commits into
Open
sadpandajoe wants to merge 2 commits into
sadpandajoe wants to merge 2 commits into
Conversation
The nightly `testcontainers (db2, 25)` leg fails intermittently with `Wait time (120.0s) exceeded for _connect`. `Db2Container._connect` calls `wait_for_logs(self, predicate="Setup has completed")` with no explicit timeout, so it inherits testcontainers' global default -- `max_tries` (120) x `sleep_time` (1s) = 120s -- which is bound as a default argument when `core/waiting_utils` is imported and is not configurable through `Db2Container`'s constructor. A full Db2 instance bring-up varies widely. Across the retained nightly runs, the pytest session for this module measured 88s to 153s end to end, so a hard 120s cap on the boot wait alone has near-zero margin. In the failing run the container was still `running` and had just logged `(*) Creating database testdb ...` when the timer expired -- a slow boot, not a broken container or a stale predicate. The 25-minute job budget was never close to binding: that job used 4m02s. Attach an explicit wait strategy before starting the container, matching the idiom already used by test_databend.py, test_monetdb.py, test_oceanbase.py, test_risingwave.py and test_yugabytedb.py. `DbContainer.start()` delegates to `DockerContainer.start()`, which honors `_wait_strategy`, so this runs to completion before `_connect` is called; `_connect` then finds the line already present in the accumulated logs and returns on its first poll, leaving its own 120s window permanently off the critical path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Code Review Agent Run #63811aActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44399 +/- ##
==========================================
- Coverage 80.41% 80.41% -0.01%
==========================================
Files 2932 2932
Lines 174621 174621
Branches 40559 40559
==========================================
- Hits 140415 140413 -2
- Misses 31539 31541 +2
Partials 2667 2667
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
SUMMARY
The nightly
testcontainers (db2, 25)leg fails intermittently withTimeoutError: Wait time (120.0s) exceeded for _connect(run 35185304121).
Root cause.
Db2Container._connect(testcontainers 4.15.0,community/db2/__init__.py:54-56) callswait_for_logs(self, predicate="Setup has completed")with no explicittimeout, so it inherits the library's global default —
max_tries(120) ×sleep_time(1s) = 120s(
core/config.py:103-104,160-161). That value is bound as a defaultargument when
core/waiting_utilsis imported, andDb2Containerexposes no constructor argument to widen it (
**kwargsis forwarded todocker_client.create(), not to the waiter).A full Db2 instance bring-up varies widely. Across the retained nightly
runs the pytest session for this module measured 88s to 153s end to
end, so a hard 120s cap on the boot wait alone has near-zero margin. In
the failing run the container was still
runningand had just logged(*) Creating database testdb ...when the timer expired — a slow boot,not a broken container or a stale predicate. The 25-minute job budget
is unrelated and was nowhere near binding: that job used 4m02s.
Fix. Attach an explicit wait strategy before starting the container,
matching the idiom already used by
test_databend.py,test_monetdb.py,test_oceanbase.py,test_risingwave.pyandtest_yugabytedb.py.DbContainer.start()delegates toDockerContainer.start(), whichhonors
_wait_strategy(core/container.py:280-286), so this runs tocompletion before
_connectis called.LogMessageWaitStrategysearches the full accumulated
container.get_logs()with the samepattern, so
_connectthen matches on its first poll and its own 120swindow is permanently off the critical path — no library internals are
monkeypatched and no private method is overridden.
Behavior change worth noting:
LogMessageWaitStrategyalso raisesRuntimeErrorif the container exits before the message appears(
core/wait_strategies.py:138-150). This is an improvement — a crashedDb2 now fails fast instead of burning the full timeout — but it is a
change from the previous behavior, which would have waited out the
whole window regardless.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — not a UI change.
TESTING INSTRUCTIONS
This cannot be verified on Apple Silicon:
icr.io/db2_community/db2publishes no arm64 build. On an x86_64 host with Docker:
Otherwise, trigger the Testcontainers workflow via
workflow_dispatchon this branch and confirm the
testcontainers (db2, 25)leg is green.Because the failure is an intermittent margin problem (1 failure in the
8 retained nightly runs), a single green run confirms no regression but
not the absence of the original defect; the nightly cron is the real
signal.
No hermetic unit test is possible here — the defect is wall-clock margin
against a real container boot, which needs Docker and a multi-GB image.
The rationale for the 900s value is recorded in a comment in the fixture
in place of the test that cannot be written.
ADDITIONAL INFORMATION