fix: reconcile big segment data with polling endpoint when stream is quiet - #790
Draft
kinyoklion wants to merge 1 commit into
Draft
fix: reconcile big segment data with polling endpoint when stream is quiet#790kinyoklion wants to merge 1 commit into
kinyoklion wants to merge 1 commit into
Conversation
…quiet The big segment synchronizer syncs via poll, then connects the SSE stream and bridges the gap with a final poll. After that it relied entirely on stream events: the periodic 30-second timer only refreshed the store's synchronizedOn timestamp. A stream event that is missed (for example, one published while the upstream subscription is still being established) is never redelivered, so the corresponding revision stayed unapplied until some later unrelated event arrived. The periodic timer now reconciles against the cursor-based revisions polling endpoint, applying any revisions that were not delivered as stream events, so a missed event is picked up in bounded time. Also raises the big segments integration test evaluation wait from 20s to 60s to cover a full reconciliation cycle (the same change was made on the v8 branch but never ported), and stops logging the 'will retry' warning when the synchronizer is shutting down cleanly. SDK-2840
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
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.
Ticket: SDK-2840
Root cause of the
big_segmentsintegration flakeTestEndToEnd/big_segments/{Redis,DynamoDB}/another_big_segment_is_created_after_synchronizer_has_startedis the most frequent remaining CI flake (12+ occurrences May–July). Log forensics on two recent failures (run 30562656155, v9, 07-30; run 29779343829, v8, 07-20) show the same signature:/big-segmentsSSE stream connected successfully and then delivered nothing for the entire wait — 60 seconds in the v8 run, so test(integrationtests): increase SDK key expiry margin and big-segment sync timeout #711's timeout bump demonstrably does not fix this.consumeStreamonly refreshedsynchronizedOn. A missed stream event is never redelivered, so the revision stayed unapplied indefinitely.The likely trigger in CI is that the test environments are created seconds before the stream connects, and events published while the upstream subscription is still being registered are dropped. But the underlying exposure is general: any silently-dropped stream event leaves Relay's big segment data stale until the next unrelated event for that environment arrives (heartbeats keep the connection alive, so the 5-minute read timeout doesn't rescue it).
Changes
consumeStream's periodic timer (30s of stream quiet) now runs a reconciliation poll against the cursor-based revisions endpoint — applying any missed revisions and notifying update listeners — before refreshingsynchronizedOn. This bounds the staleness from a missed stream event to ~30s. When nothing was missed, the poll is a single cheap request returning an empty array (the request carries the cursor).will retrywarning when it is shutting down cleanly — that warning appears at the tail of every failed-test log dump and reads as a stream failure when it's actually teardown (it misdirected this investigation for a while).TestSyncReconcilesWhenStreamIsQuiet: stream connects but stays silent, a missed revision exists only on the poll endpoint, and the reconciliation poll applies it without a stream reconnect.Trade-off for reviewers
On a quiet stream this adds one revisions poll per environment per 30 seconds (previously zero once the stream was up). For large auto-config deployments that is a real increase in requests to the big-segments revisions endpoint, though each request is cursor-based and cheap when there are no changes. If that's a concern, alternatives are a longer reconcile interval or backing off after consecutive empty reconciles — happy to adjust.
Verification
go test -race -count=5 ./internal/bigsegments/green; the new and neighboring sync tests pass 50 consecutive-raceruns under full CPU load.go vet -tags integrationtests ./integrationtests/clean.