Skip to content

[2.0.x] fix: wait for the database at startup; quieter logs when stopping a sync that is not running (backport of #1333) - #1346

Open
satran004 wants to merge 5 commits into
release/2.0.xfrom
backport/2.0.x/1333-startup-db-wait-quiet-stop
Open

satran004 wants to merge 5 commits into
release/2.0.xfrom
backport/2.0.x/1333-startup-db-wait-quiet-stop

Conversation

@satran004

Copy link
Copy Markdown
Member

Backport of #1333 to release/2.0.x. Tracked in #1339.

What it fixes: see #1333. The backport changes the same code in the same way.

How it was made: The 2 commits of #1333, no changes. Commits are cherry-picked with -x, and the changed lines were compared one by one with #1333.

Checks: 107 components/core tests pass; admin compiles.

Fault-injection run: the same scenarios as #1328 are running on release/2.0.x with all the backports. The results will be posted here and in #1339.

🤖 Generated with Claude Code

satran004 and others added 2 commits November 7, 2026 14:59
…sync that is not running

- Flyway retries the first database connection (30 retries, up to 10 s apart),
  and Hikari no longer fails at pool creation. The app no longer exits when it
  starts before Postgres is ready, for example after a host restart
  (fault-injection scenario J09, #1328).
- Stopping a range fetch or chain sync that was never started or is already
  closed logs one warning line instead of a stack trace (#1314).

Part of #1328.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit bc8bd0f)
… stop errors

Review: Spring Boot resolves Flyway's {vendor} location through the DataSource
before Flyway's connect-retries apply, so the retries never ran. Replace them
with a small BeanPostProcessor that retries a connection right after the
DataSource is created (store.db.startup-wait-seconds, default 300, 0 = off).
Only the NullPointerException yaci throws for a session that was never started
gets the one-line log; other stop errors keep their stack trace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit f4b487a)
@satran004 satran004 added fault-injection Found or verified by the auto-recovery fault-injection test 2.0.x Fix or backport for the release/2.0.x stable line labels Nov 7, 2026

Copy link
Copy Markdown

Initial automated review · Aster

Hi @satran004, thanks for the PR! 🙏 This is an automatic first check by an AI reviewer. It does not replace maintainer review and may be wrong.

Reviewed commit: dd6244715f6698e13705d260eea34f90f5e8f5e3

Issues found:

  • components/core/src/main/java/com/bloxbean/cardano/yaci/store/core/configuration/DatabaseStartupWait.java:57 — With a one-second startup wait, a refused connection can still trigger a five-second sleep and another connection attempt after the deadline. Could we check the remaining time before sleeping and retrying?

Not tested: I reviewed the code and CI without running the build, tests, or a database outage scenario.

@satran004

Copy link
Copy Markdown
Member Author

Fault-injection result on release/2.0.x

The same scenarios as #1328 ran on plain release/2.0.x and on release/2.0.x with all 8 backports (#1340–#1347). With the backports:

  • all 104 scenarios recovered on their own, with no manual restart (plain 2.0.x: 6 stuck);
  • no duplicate rows (plain 2.0.x: 31 scenarios);
  • only the 2 blocks that B04 skips on purpose are missing (plain 2.0.x: 9 blocks).

Full results: #1339.

This PR in the run: J09 (app and Postgres killed together, as in a host crash) during the initial sync: on plain 2.0.x the app exited because it started before Postgres, and needed a manual start. With this fix it waited for the database and recovered in 50 s.

Copy link
Copy Markdown

reviewer1 — review of dd6244715f6698e13705d260eea34f90f5e8f5e3

One [P2] fix requested: enforce the configured retry deadline before sleeping and before the next attempt. DatabaseStartupWait.java:46–57 checks the deadline only after a failed connection attempt, then always sleeps five seconds. With store.db.startup-wait-seconds=1 and immediately refused connections, I compiled and ran the exact PR source and measured 5,004 ms and two connection attempts. The second attempt begins after the budget has already expired and can add the pool/driver's entire connection timeout.

Calculate the remaining budget, cap the sleep to it, and check expiry before starting another attempt. Also make clear that an in-flight getConnection() needs its own bounded pool/driver timeout; the current deadline cannot interrupt a connection attempt that never returns. Add expiry and interruption tests alongside the existing retry-success/disabled tests.

The hook's placement before consumers use the datasource is appropriate for the reported startup failure, and the J09 recovery in 50 seconds is useful evidence that this fixes the tested host-restart ordering. I found no additional blocking issue in the stop-log changes.

Validation: exact-head source/test inspection and a small compiled harness with a refusing stub datasource. No real database outage or full-suite rerun.

satran004 and others added 2 commits November 8, 2026 19:20
Review: the deadline was only checked after a failed attempt, and the
pause before the next attempt was always 5 s. With
store.db.startup-wait-seconds=1 and a refused connection the wait took
about 5 s and made a second attempt after the budget was already used up.

Now the pause is capped to the time left, and no attempt starts once the
budget is used up. A single getConnection() that hangs is still bounded by
the pool/driver connection timeout, not by this wait; the class comment
says so.

The clock and sleep are injectable so the tests check the timing without
waiting. Added tests for a 1 s budget, the capped last pause, an attempt
that uses up the budget, a real-clock 1 s budget, and interruption.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
(cherry picked from commit 76cc063)
The last pause is capped to the time left, so it ends at the deadline. Try once more then instead of giving up
right after the pause, so a database that comes up within the budget is still used. No attempt starts after the
deadline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
(cherry picked from commit 08b7df4)
@satran004

Copy link
Copy Markdown
Member Author

Thanks, reviewer1. Handled as follows:

  • The pause is capped to the time left. The last attempt starts at the deadline, so a database that comes up within the budget is used; no attempt starts after the deadline.
  • The javadoc says a hanging getConnection() is bounded by the pool timeout.
  • New tests: expiry, a capped last pause, interruption (fake clock).

Commits: 9988540, 86d475f here, 76cc063, 08b7df4 on #1333.

The fixes were made in #1333 first and cherry-picked here. Each finding, its verdict and its test are in the reply on #1339. The fault suite is running again on the updated combined build.

@satran004

Copy link
Copy Markdown
Member Author

Verdict: CHANGES REQUESTED

reviewer1 — round 2. Reviewed 86d475fc552a958ac74cbe773ba3799780a4750b, compared with dd6244715f6698e13705d260eea34f90f5e8f5e3.

F1 [P2] is partially resolved. Capping the pause fixes the original five-second oversleep for short budgets. Allowing an attempt exactly at the deadline is acceptable, but the implementation still starts attempts after it.

F1 [P2], completion of the original finding. DatabaseStartupWait.java:84: after sleeping, the loop retries without checking the clock. Thread.sleep(remaining) can resume late because of scheduling, GC or process suspension. That late attempt can incur another connection timeout.

A deterministic probe against the actual class uses a 1,000 ms budget and a sleeper that resumes 1,000 ms late. Output: deadline_ms=1000, attempt_start_ms=[0, 2000]. The source is identical on this head and the tested combined commit. The existing fake-clock tests resume exactly on time, so they miss this.

Please recheck the clock before a retry and stop when it is greater than the deadline; equality can retain the requested final attempt. Add one oversleep test. No larger retry redesign is needed.

Validation: seven module suites on isolated combined commit 121ee4599fcdc41226239b9195ce45daa47cfc9d: 366 passed, 1 skipped, 0 failures/errors (core: 219 passed, 1 skipped). This is combined-build validation, not a separate run on every backport.

Exact command
JAVA_HOME=/Users/satya/.sdkman/candidates/java/21.0.3-tem ./gradlew :components:core:test :components:common:test :stores:assets:test :stores:metadata:test :stores:script:test :stores:mir:test :stores:staking:test --no-daemon --console=plain

No implementation changes made by this review. The updated VM fault-suite results are still pending. This verdict covers the reviewed head only.

The last pause ends at the deadline, but the thread can wake up late (scheduling, GC, a suspended process), and
the next attempt then started after the deadline. Check the clock after the pause: an attempt at the deadline is
still made, a later one is not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
(cherry picked from commit b3726b0)
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

@satran004

Copy link
Copy Markdown
Member Author

Verdict: APPROVED

reviewer1 — round 3. Reviewed 22aaa9ead98c72db212e7f94966062e6dc5fc8c1, diffed from 86d475fc552a958ac74cbe773ba3799780a4750b.

F1 [P2]: Resolved. The post-sleep clock check rejects a retry after the deadline while allowing equality. The new oversleep regression covers the missing case. No further change requested.

Validation on this exact head in an isolated checkout: 10 tests passed, none failed or skipped:

JAVA_HOME=/Users/satya/.sdkman/candidates/java/21.0.3-tem ./gradlew :components:core:test --tests com.bloxbean.cardano.yaci.store.core.configuration.DatabaseStartupWaitTest --no-daemon --console=plain

The independent round-2 oversleep probe now reports deadline_ms=1000, attempt_start_ms=[0] (previously [0, 2000]). No implementation files changed by this review. Approval covers this head only.

@satran004

Copy link
Copy Markdown
Member Author

Author reply: round r2 · head 22aaa9e

Finding Change Where
F1 [P2] After the pause, the clock is checked again. An attempt exactly at the deadline is still made; an attempt after it is not. New test: a pause that ends 1 s late leads to no second attempt. DatabaseStartupWait.waitFor; DatabaseStartupWaitTest.noAttemptAfterAPauseThatEndsLate

Made on #1333 first (b3726b0), then cherry-picked here. Thanks for the approval.

@satran004

Copy link
Copy Markdown
Member Author

Fault-injection rerun on 2.0.x with all eight backports at their approved heads (test/recovery-combined-2.0.x bbac1a25a): 104 of 104 scenarios recovered, with no duplicate rows and no unexpected missing blocks. The full report is on #1339: #1339 (comment)

For this PR:

  • The Postgres scenarios all recovered: D01–D11 in both phases (Postgres down, frozen or partitioned, among others), J09 (app and Postgres killed together) and C07 (fresh JVM while the node refuses).

The unpatched baseline is release/2.0.x (b2x), run on the same matrix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.0.x Fix or backport for the release/2.0.x stable line fault-injection Found or verified by the auto-recovery fault-injection test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants