Repository navigation
Conversation
…y off (#959) - When the auto-restart stops the sync and then cannot start it again (for example, the database is down), clear the "scheduled to stop" flag and set the error flag. The health check then retries. Before, the flag stayed set and auto-recovery treated it as a deliberate stop forever. - StartService.start() leaves the service startable when it throws. - A new sync session gets the full block-receive delay before the health check calls it stalled. A fresh session that never receives a block is now restarted. - After max-attempts is reached, keep restarting once per store.auto-restart.retry-interval-after-max-ms (default 60 s) instead of pausing for the whole window. 0 restores the old behaviour. Part of #1328. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit a2f6c5f)
… restart backoff - The auto-restart now skips its start when the sync was stopped or started by someone else while it waited, and only a real failure marks the restart failed. - Cap the backoff exponent. Attempts keep counting during a long outage and 5000 * 2^n overflowed to a negative sleep after ~64 attempts. - start/stop are synchronized and alreadyStarted is volatile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 5d8581f)
… honour stops after a failed restart - A restart request dropped by the debounce, the attempt limit or a running restart, and a restart that failed, are checked again after the debounce window. The restart runs again if the sync is still stopped by an error. The health-check service is optional and off by default, so it cannot be the only thing that asks again. - stop() always records the stop, even when no session is running, and the admin stop endpoints always call it. A stop after a failed restart now keeps the sync stopped. - start() clears the error flag left by a failed restart, so a retry on a fresh database can write the genesis cursor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ab0a437)
Found by the fault-injection re-test (scenario C06: a fresh JVM whose node connection is accepted but never answered). - yaci 0.4.7 waits for the handshake with no time limit. The block check (BlockFinder), the genesis lookup and the tip lookup now run through NodeCalls, which gives up after 40 s when auto-restart is on (no limit when it is off, as before) and cleans up the stuck connection. - BlockFinder no longer treats "no answer" as "block not on the chain". That made the startup step back to an older cursor and delete newer ones. - The health check no longer throws NullPointerException before the sync client was started, and skips while the sync is starting. - A first start that fails because the node, the database or the network is not reachable is retried by auto-restart instead of closing the app. Other failures (configuration errors) still stop the app. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 9722c0a)
satran004
added
fault-injection
|
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: Issues found:
Not tested: I reviewed the code and CI without running the build, tests, or live fault scenarios. |
Fault-injection result on release/2.0.xThe same scenarios as #1328 ran on plain
Full results: #1339. This PR in the run: C01 and C04 (a restart fails while Postgres is down or StartService fails) got stuck on plain 2.0.x in both the initial sync and at the tip, and recovered on their own with this fix (40 s and 68 s). D04 (DB network partition, initial sync) got stuck on plain 2.0.x and recovered in 3 s with this fix. C06 (the node accepts the connection but never answers) recovered in 34 s at the tip. |
reviewer1 — review of
|
…ackoff Review follow-ups for #1330. - The live connection opened last by StartService.start (startFetch or startSync) had no time limit. yaci 0.4.7 opens the connection and waits for the handshake inside that call (Session.handshake loops with no deadline, bloxbean/yaci#241). A node that answered the cursor and tip checks and then stalled held start(), the restart lock and the "starting" flag the health check skips. With auto-restart on, the start now gives up after 40 s like the other node calls: the connection is closed without taking the fetch service's lock (the stalled start holds it), the thread is released, and the start fails so the restart retries. With auto-restart off it still waits as before. - IntersectionNotFound on the live session published a restart without setting the error flag. A request dropped because a restart was running or within the debounce window (always the case right after a restart) was then never retried, since the follow-up check only retries while the sync is in the error state. The session is now marked failed first. - The auto-restart took its stop token only after the 5-60 s backoff, so an admin or snapshot stop during the backoff was overridden. The stop count is now read before the backoff; if it changed, the restart does nothing. - With auto-restart off the cursor check waited forever for the node's answer (before the PR: 40 s). The answer is waited for at most 40 s again; a timeout fails the start and is still not taken as "block not on the chain". - BlockFetchServiceRestartStateTest started a session with a mocked keep-alive interval of 0, which spins and records calls on the mock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb (cherry picked from commit c2a7b43)
…meout/startSyncWithTimeout #1313 adds startFetch(from, to, Long fromBlockNumber). A startFetch(from, to, long timeoutMs) overload next to it would let a call silently pick the wrong one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb (cherry picked from commit 9d50730)
…e the restart read its count A start can finish just after its time limit, when NodeCalls no longer runs the cancel, and the caller then treats it as failed with its connection still open. Close the connection whenever the timed start fails. A restart request can wait in the queue, or come from the closing connection, after a deliberate stop. Its stop-count check then passes. Refuse the restart while the sync is stopped on purpose (scheduledToStop): a failed restart clears that flag, so only a deliberate stop leaves it set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb (cherry picked from commit 090dde9)
|
|
Thanks, reviewer1. Handled as follows:
Commits: 31027bd, 2f5a265, c388bff here, c2a7b43, 9d50730, 090dde9 on #1330. The fixes were made in #1330 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. |
|
Verdict: APPROVED reviewer1 — round 2. Reviewed All four original findings are resolved for the startup/retry paths changed here.
I agree to defer the hung yaci DNS lookup and the unbounded range→chain transition in The combined Validation: seven module suites on isolated combined commit Exact commandJAVA_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=plainNo implementation changes made by this review. The updated VM fault-suite results are still pending. This verdict covers the reviewed head only. |
|
Fault-injection rerun on 2.0.x with all eight backports at their approved heads ( For this PR:
The unpatched baseline is |
Backport of #1330 to
release/2.0.x. Tracked in #1339.What it fixes: see #1330. The backport changes the same code in the same way.
How it was made: The 4 commits of #1330. The
docs/app/docs/v3/...page is dropped because it isn't on this branch. Commits are cherry-picked with-x, and the changed lines were compared one by one with #1330.Checks: 134
components/coretests pass;admin,admin-ui, the starter andapplications:allcompile.Fault-injection run: the same scenarios as #1328 are running on
release/2.0.xwith all the backports. The results will be posted here and in #1339.🤖 Generated with Claude Code