feat: conform FDv1 streaming and polling data sources to the RETRY spec - #347
Draft
tanderson-ld wants to merge 1 commit into
Draft
feat: conform FDv1 streaming and polling data sources to the RETRY spec#347tanderson-ld wants to merge 1 commit into
tanderson-ld wants to merge 1 commit into
Conversation
Classify data source failures as normal or unexpected per RETRY 1.6.1 and 1.7.1, and apply an extended backoff regime (5 minute initial delay, 1 hour ceiling) after an unexpected failure instead of terminating the data source. - Unexpected HTTP statuses such as 401, 403 and 405 no longer permanently stop a data source. Both sources stay Interrupted and keep retrying at extended-regime timing. - Streaming applies the regime through the EventSource temporary retry delay bounds; sustained healthy operation clears them. - Polling gains PollingStrategy, which carries the attempt count and the regime, and reschedules each poll from the strategy rather than from a fixed interval. - Transport failures classify as normal. Cases that cannot be recognised with confidence are deliberately treated as normal, since misreading a transient fault as unexpected is the more damaging error. - Adds FailureClass and TaskExecutor.ScheduleTask to InternalSdk. - Declares the retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling contract-test capabilities. Tests cover the HTTP and transport classification tables, PollingStrategy delay progression and regime transitions, ScheduleTask timing, cancellation and error handling, and data source lifecycle and bounds behaviour. The FDv2 data source test mocks implement the two new IEventSource members inertly, since those sources do not use the extended-regime bounds. Requires LaunchDarkly.EventSource 6.0.0, which is not yet released, and an InternalSdk release carrying FailureClass and ScheduleTask before the SDK pins can pick them up. SDK-2791
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.
Requirements
The third box is deliberately unchecked.
net462is Windows-only and has not beenexercised locally; this PR's CI matrix (
ubuntu-22.04,windows-latest,macos-latest)is the first place it will run.
Important
This branch cannot compile in CI yet, by design. The source calls
IEventSource.SetTemporaryRetryDelayBounds/ClearTemporaryRetryDelayBounds, whichonly exist in the unreleased
LaunchDarkly.EventSource6.0.0. The local package pin andnuget.configused to build this work are intentionally not committed, so thePackageReferencehere still reads 5.3.1. Draft until the dependency chain lands.Related issues
Implements the RETRY specification for the FDv1 streaming and polling data sources, the
.NET counterpart of:
launchdarkly/java-coretest: Wait start task for SuccessfulRequestCausesDataToBeStoredAndDataSourceInitialized. #200, chore: Conditional build targets for linux #204, chore: Add read/write mode support for persistent store. #206launchdarkly/go-server-sdk#429launchdarkly/dotnet-eventsourcefix: Update CommonSDK to v7.0.1 #126 (the SSE-client half, in review)Where the references disagree, this follows Java, since the two implementations are
structurally close.
Describe the solution you've provided
Failures are classified as
NormalorUnexpected, and an unexpected failure engages anextended backoff regime (5 minute initial delay, 1 hour ceiling) instead of terminating
the data source.
source. Both sources stay
Interruptedand keep retrying at extended-regime timing.sustained healthy operation clears them.
PollingStrategy, carrying the attempt count and the regime, andreschedules each poll from the strategy rather than from a fixed interval.
FailureClassandTaskExecutor.ScheduleTasktoLaunchDarkly.InternalSdk.retry-conformance-fdv1-streamingandretry-conformance-fdv1-pollingcontract-test capabilities.
Describe alternatives you've considered
Transport-failure classification. An earlier revision inspected exceptions for TLS and
certificate causes to classify them as
Unexpected. That was withdrawn: the cases cannotbe recognised reliably across target frameworks, and misreading a transient fault as
unexpected costs the customer a 5-minute-to-1-hour outage, while the reverse costs some
retries. Transport failures now classify as
Normal, matching the default-to-normal shapeof the spec's transport table.
Scheduling. Polling could have self-scheduled with a bare
Task.Run, but keeping thework in
TaskExecutorpreserves parity with Java's executor and keeps exception logging inone place.
ScheduleTaskaccepts aCancellationTokenper BCL convention rather thanreturning a source, and starts
Task.Delayon the calling thread so argument validationsurfaces to the caller instead of being lost on a detached task.
Server-directed retry. Matches Java: an SSE
retry:value sets the initial delay, cappedat 1 hour, and does not alter the configured maximum or the attempt count.
Additional context
Release sequencing, following the precedent set by #339 and #343:
LaunchDarkly.InternalSdkcarryingFailureClassandScheduleTaskLaunchDarkly.EventSource6.0.0Verification so far, on
net8.0only:LaunchDarkly.InternalSdk.Tests— 222/222LaunchDarkly.ServerSdk.Tests— 1644/1644 on five of six full runsOne failure occurred on the first run inside
PollingDataSource.UpdateTaskAsync, has notreproduced in nine subsequent runs, and is still being investigated; two candidate defects
have already been identified by inspection. Not ready for review until that is settled.
The harness-side retry-conformance suites are merged and released (
sdk-test-harnessv2.40.0). They require
-enable-long-running-testsand run at full production timing, so acomplete pass takes roughly 60-80 minutes; that run has not been performed yet. Note that
declaring the two capabilities also causes the harness to skip its legacy
permanent-stop tests, which no longer describe conformant behavior.
SDK-2791