Repository navigation
fix: fall back from a silent synchronizer during FDv2 startup - #461
tanderson-ld wants to merge 2 commits into
Conversation
A synchronizer that fails permanently during startup reports Off, which the data system publishes as Interrupted. Nothing restored Initializing, so the ten-second startup fallback branch -- which tested the status value -- was unreachable for the rest of startup and only the one-minute runtime timer applied. When the next synchronizer connected and then stayed silent it produced no result for the loop to act on, so the client's start-wait expired before the fallback could advance to a healthy source. Key the startup branch on whether data has ever been applied rather than on the status value. The fallback and recovery conditions previously had zero execution coverage; this adds in-package tests for both the predicate and the multi-synchronizer loop path. SDK-3235
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 338196d. Configure here.
| cannotInitialize := status.State == interfaces.DataSourceStateInitializing && | ||
| // Cannot initialize is intentionally not a status check since a permanent | ||
| // failure during startup leaves the status on Interrupted, which made this | ||
| // branch unreachable for the rest of startup. |
There was a problem hiding this comment.
Comments use historical framing
Low Severity
New comments describe the previous Initializing check and "before the fix" behavior instead of the current rule. Comments on fallbackCond and the new tests mix in old status-keying and the one-minute hang; that history belongs in the PR, not in the code.
Additional Locations (2)
Triggered by learned rule: Go comments: no spec citations, tickets, or historical framing
Reviewed by Cursor Bugbot for commit 338196d. Configure here.
2 participants
Requirements
Related issues
SDK-3235. Also fixes the one known failure in the nightly contract tests added in #454:
streaming/fdv2/permanent fallback with recovery.Describe the solution you've provided
The data system has two fallback timers, selected by reading the status: a 10-second one for startup (
cannotInitialize) and a 1-minute one for steady state (interruptedAtRuntime).A synchronizer that fails permanently during startup reports
Off, which the loop publishes asInterrupted. Nothing restoresInitializing, so the startup branch — which tested the status value — was unreachable for the rest of startup, leaving only the 1-minute timer. That only becomes visible when the next synchronizer connects and then stays silent: it produces no result for the loop to act on, so the loop depends entirely on the timer, and the client's start-wait expires first. The healthy source is never contacted.Keying on whether data has ever been applied expresses the actual predicate and cannot go stale as states are added. Runtime behavior is unchanged: once data is applied, only the 1-minute timer applies.
Pre-existing, not a regression — it reproduces on 7.15.6, before the RETRY work.
Describe alternatives you've considered
Initializingon removal. Rejected on cross-SDK evidence: Java and .NET both reportINITIALIZING → INTERRUPTEDand neither re-publishesInitializingafter a source fails. .NET makes that choice explicitly ("spoof interrupted"). This keeps Go's externally visible status sequence identical to today's and consistent with both siblings.Interruptedto the startup branch's state check. Rejected:Interrupted && >10ssubsumesInterrupted && >1min, which would make the runtime timer dead code and abandon a source after 10 seconds instead of a minute — and recovery back to it takes 5 minutes of healthy operation.StateSinceon the transition. Insufficient: the 1-minute timer would restart and fire at ~70s, still past a 60s start-wait.Additional context
The fallback and recovery conditions had zero execution coverage before this (the closure bodies measured 0 executions, while the file overall sat at 59.6%) — they are only evaluated on a 10-second tick with 2+ synchronizers configured, which nothing but the long-running contract test reached. The new in-package tests cover both the predicate and the multi-synchronizer loop path, and both fail when the fix is reverted.
Verified: all 4
streaming/fdv2fallback contract tests pass against the patched SDK, including the recovery assertion that had never executed for Go; all 28 root-package FDv2 end-to-end tests pass;golangci-lintreports 0 issues.Note
Overview
Fixes FDv2 startup getting stuck when an early synchronizer fails permanently and the next one connects but never sends results.
The 10-second startup fallback (
cannotInitialize) no longer requiresDataSourceStateInitializing. After a permanent failure the published status stays Interrupted, so that branch never fired and only the 1-minute runtime timer could advance the synchronizer chain—often past the client init timeout.Startup fallback is now keyed on
!dataApplied(no flag data applied yet) plus the same 10sStateSincethreshold, so a silent second source still triggers fallback to the next synchronizer. Once data has been applied, behavior is unchanged: only Interrupted > 1 minute triggers fallback.Adds unit tests for the fallback predicate (including the Interrupted-during-startup regression) and an integration test that walks failing → silent → healthy synchronizers using real timing.
Reviewed by Cursor Bugbot for commit 220a5c8. Bugbot is set up for automated code reviews on this repo. Configure here.