Repository navigation
Conversation
0cf207f to
8aa8c77
Compare
f78d65f to
0dafa42
Compare
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 0dafa42. Configure here.
| } | ||
| break; | ||
| case VALID: | ||
| consecutiveSuccessfulResults++; |
There was a problem hiding this comment.
I don't think you can conclude success from consecutive valid status.
StreamProcessor calls updateStatus(State.VALID, null) at the end of handleMessage for every message event, including the default: branch that logs "Unexpected event found in stream" (StreamProcessor.java:327-331). Two unknown event types in a row (the forward-compatibility case: any new server event name) produce two VALIDs with zero data delivered.
There was a problem hiding this comment.
Yes, this code kind of assumes that the FDv1 fallback is a polling synchronizer. Making this work for any kind of synchronizer would require adding a way to signal the reset from any kind of FDv1 DataSource, and I don't think that's worth the effort, given FDv1 isn't the future anyway.
| switch (res.getResultType()) { | ||
| case CHANGE_SET: | ||
| consecutiveSuccessfulPolls++; | ||
| if (consecutiveSuccessfulPolls == RESET_THRESHOLD_SUCCESSFUL_POLLS) { |
There was a problem hiding this comment.
| if (consecutiveSuccessfulPolls == RESET_THRESHOLD_SUCCESSFUL_POLLS) { | |
| if (consecutiveSuccessfulPolls >= RESET_THRESHOLD_SUCCESSFUL_POLLS) { |
There was a problem hiding this comment.
I would prefer to keep it as ==, so the reset only gets set once, per the spec. I don't think it makes much difference right now, but it would be more fragile to have the reset signal firing on every result.
0dafa42 to
a700bff
Compare

Summary
Applies the RETRY behavior to FDv2 synchronizer orchestration in the server SDK. A synchronizer that fails is parked and retried later, instead of being blocked for good.
Parking
next(), a failure to build, or shutting down on its own. The park delay starts at 5 minutes, doubles with each failure up to 1 hour, and has jitter. After a park, the next synchronizer starts after a failover delay of 0.5-1 seconds.Directed fallback
Conditions removed
Conditionsclass andFDv2DataSourceConditionsare removed. They weren't compatible with the new parking and FDv1 fallback timers, so the synchronizer loop is simplified to check all of its timers each time it wakes.Not included
Retry-After.Note
Overview
Replaces the old condition/timer model (
FDv2DataSourceConditions,FDv2DataSource.Conditions) with a single synchronizer loop that wakes on results or deadlines (fallback, recovery, unpark, FDv1 TTL). Failed synchronizers are parked with exponential backoff (5 min → 1 h, jitter) instead of being blocked for the process lifetime; terminal errors and build failures park the source rather than permanently blocking it.Adds
AsyncLatchsoclose()and timed waits can raceCompletableFuture.anyOfwithout leaking completion nodes on long-lived streams.FDv1 fallback is time-bounded (default 1 h, jitter):
SourceManagertracks end time, blocks FDv2 sources during fallback, and returns when TTL expires. Server directives extend TTL or switch to FDv1 without halting the whole data system when no FDv1 synchronizer exists (reports INTERRUPTED and waits for TTL).Reset threshold is propagated via
FDv2SourceResult.resetThresholdMet— polling (2 successes), streaming (60 s healthy), FDv1 adapter (2 VALID) — and resets park counts. HTTP/transport classification (FailureClass) turns unexpected errors into terminal results in polling, streaming, and the FDv1 synchronizer adapter.close()releases the latch, shuts down sources, and joins the run thread (5 s cap) before reporting OFF.Reviewed by Cursor Bugbot for commit 94f7478. Bugbot is set up for automated code reviews on this repo. Configure here.