diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index efe0b78..0f75336 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -5,8 +5,11 @@ namespace Segment.Analytics.Retry { public class RateLimitConfig { - /// Largest Retry-After the client will honour, in seconds. RFC 7231 allows - /// more, but the TAPI agreements cap it here and the other SDKs fix it at this value. + /// Largest Retry-After the client will honour, in seconds. A guard against + /// an absurd header, not a second budget: waiting less than the server asked for does + /// not make the next attempt more likely to succeed, it just sends more requests at + /// something already rate-limiting us. How long we keep trying is + /// 's job. public const int MaxRetryIntervalCeiling = 300; public bool Enabled { get; } @@ -15,18 +18,27 @@ public class RateLimitConfig /// /// Wall-clock ceiling, in seconds, on how long one rate-limit episode may keep a - /// batch alive. A last-ditch guard so a pathological Retry-After stream cannot hold - /// a batch forever; is what stops retrying in practice. - /// At the defaults the count is reached first by a wide margin, since - /// MaxRetryCount * MaxRetryIntervalCeiling is well under this. + /// batch alive. + /// + /// Unlike the other Segment SDKs, this one bounds the rate-limit path by a + /// count as well (), and which of the two binds depends + /// on the Retry-After being served: below roughly + /// MaxRateLimitDuration / MaxRetryCount — 18 seconds at the defaults — the + /// count runs out first, above it the duration does. + /// + /// Deliberately several times . When the two are + /// equal, a response with no usable Retry-After waits + /// by default, which consumes the entire budget — the elapsed check runs before the + /// wait, so the batch is dropped after a single attempt having stalled the whole + /// pipeline for the duration. /// public long MaxRateLimitDuration { get; } public RateLimitConfig( bool enabled = true, int maxRetryCount = 100, - int maxRetryInterval = 300, - long maxRateLimitDuration = 43200) + int maxRetryInterval = MaxRetryIntervalCeiling, + long maxRateLimitDuration = 1800) { Enabled = enabled; MaxRetryCount = maxRetryCount; @@ -40,7 +52,10 @@ public RateLimitConfig( // count, so 0 would drop every batch before it was ever sent. maxRetryCount: Math.Max(1, Math.Min(MaxRetryCount, 1000)), maxRetryInterval: Math.Max(1, Math.Min(MaxRetryInterval, MaxRetryIntervalCeiling)), - maxRateLimitDuration: Math.Max(0, Math.Min(MaxRateLimitDuration, 604800)) + // Floored at 1 like its siblings: 0 here means "no budget", so every + // rate-limited batch is dropped on its first evaluation. A CDN payload + // pushing 0 would silently disable rate-limit retrying altogether. + maxRateLimitDuration: Math.Max(1, Math.Min(MaxRateLimitDuration, 604800)) ); } diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs index edfc25e..e248c08 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs @@ -66,7 +66,13 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response) if (statusBehavior == RetryBehavior.Retry && _config.BackoffConfig.Enabled) return HandleRetryableError(state, response, currentTime); - return state.RemoveBatch(response.BatchFile); + // The request completed and carried no rate-limit signal, so the episode is + // over. Leaving RateLimitStartTime set strands it: nothing else clears it, + // and the next batch to be evaluated after the budget elapses is dropped for + // a rate limit that ended here, without ever being uploaded. + return state + .With(clearRateLimitStartTime: true) + .RemoveBatch(response.BatchFile); } public Tuple ShouldUploadBatch(RetryState state, string batchFile) @@ -104,9 +110,9 @@ public Tuple ShouldUploadBatch(RetryState state, str resetState); } - // Check 2b: how long this rate-limit episode has run. A last-ditch guard so a - // pathological Retry-After stream cannot hold a batch indefinitely; at the - // defaults Check 2 is reached long before this. + // Check 2b: how long this rate-limit episode has run. At the defaults this + // is what stops retrying — rate-limited attempts are uncounted, so elapsed + // time is the real limit and Check 2's count is the backstop behind it. if (_config.RateLimitConfig.Enabled && clearedState.RateLimitStartTime.HasValue && currentTime - clearedState.RateLimitStartTime.Value @@ -199,6 +205,24 @@ public bool ShouldDeleteBatch(int statusCode, int? retryAfterSeconds) private RetryState HandleRateLimitResponse(RetryState state, ResponseInfo response, long currentTime) { long waitUntilTimeMs = CalculateWaitUntilTimeMs(response.RetryAfterSeconds, currentTime); + + // A wait that runs past the end of the budget ends the episode. Shortening + // it to fit would resume inside the window the server asked us to wait out + // -- one it has already said it will not serve -- and ShouldUploadBatch + // would then drop the batch on the elapsed check anyway, so the shortened + // wait buys a single guaranteed-refused request. + long episodeStart = state.RateLimitStartTime ?? currentTime; + long deadline = episodeStart + (_config.RateLimitConfig.MaxRateLimitDuration * 1000L); + if (waitUntilTimeMs > deadline) + { + return state + .With( + pipelineState: PipelineState.Ready, + clearWaitUntilTime: true, + globalRetryCount: 0, + clearRateLimitStartTime: true) + .RemoveBatch(response.BatchFile); + } return state.With( pipelineState: PipelineState.RateLimited, waitUntilTime: waitUntilTimeMs, diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateStorage.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateStorage.cs index a57d506..1696427 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateStorage.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateStorage.cs @@ -52,8 +52,14 @@ private static JsonObject Serialize(RetryState state) }; if (state.WaitUntilTime.HasValue) root["waitUntilTime"] = state.WaitUntilTime.Value; - if (state.RateLimitStartTime.HasValue) - root["rateLimitStartTime"] = state.RateLimitStartTime.Value; + + // RateLimitStartTime is deliberately not persisted. It measures how long this + // process has been retrying a batch, and time while the process was not + // running is not time spent retrying. Persisting it means an app closed for + // longer than MaxRateLimitDuration loads an already-expired episode and + // discards the batch on its first flush without ever having retried it. The + // retry counts below do persist, so a batch still cannot be retried + // indefinitely across restarts. if (state.BatchMetadata.Count > 0) { @@ -85,7 +91,6 @@ private static RetryState Deserialize(JsonObject root) pipelineState = PipelineState.RateLimited; long? waitUntilTime = ReadNullableLong(root, "waitUntilTime"); - long? rateLimitStartTime = ReadNullableLong(root, "rateLimitStartTime"); int globalRetryCount = ReadInt(root, "globalRetryCount"); var batchMetadata = new Dictionary(); @@ -104,7 +109,8 @@ private static RetryState Deserialize(JsonObject root) } } - return new RetryState(pipelineState, waitUntilTime, globalRetryCount, batchMetadata, rateLimitStartTime); + // rateLimitStartTime is intentionally absent; see Serialize. + return new RetryState(pipelineState, waitUntilTime, globalRetryCount, batchMetadata); } private static int ReadInt(JsonObject json, string key) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3b75a8..07984ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,16 +6,16 @@ This file carries the notes that need more than a pull-request title. ## Unreleased -### Behavior change: retries and backoff are on by default +### Behaviour change: retries and backoff are on by default -Through 2.6.0, rate limiting and exponential backoff were both disabled unless you -supplied an `HttpConfig` or a CDN settings payload turned them on. Server-side -deployments receive no CDN settings, so in practice they retried nothing: 408, 410 and -460 were dropped, `Retry-After` was ignored, and a 429 or 5xx was held with no delay and -no budget. Both subsystems now default to enabled, so a client that configures nothing -gets the documented retry behavior. +Through 2.6.0, rate limiting and exponential backoff were both disabled unless an +`HttpConfig` was supplied or a CDN settings payload turned them on. Server-side +deployments receive no CDN settings, so in practice they retried nothing: 408, 410 +and 460 were dropped, `Retry-After` was ignored, and a 429 or 5xx was held with no +delay and no budget. Both subsystems now default to enabled, so a client that +configures nothing gets the retry behaviour described below. -To keep the old behavior, disable both explicitly: +To keep the previous behaviour, disable both explicitly: ```csharp new Configuration("writeKey") @@ -26,29 +26,30 @@ new Configuration("writeKey") } ``` -CDN settings are unaffected and still take precedence: a payload carrying an -`httpConfig` key replaces whatever the pipeline is running with, and a payload without -that key leaves your configuration in effect. +CDN settings still take precedence: a payload carrying an `httpConfig` key replaces +whatever the pipeline is running with, and a payload without that key leaves the +supplied configuration in effect. -- Backoff defaults now match the other Segment SDKs: `MaxRetryCount` 10 (was 100) and `MaxBackoffInterval` 60s (was 300s). With retries off by default those numbers were latent; enabling them unchanged would have had C# clients making an order of magnitude more attempts against the endpoint than any other SDK. - -### Upgrade note: new request headers and proxy allowlists +### Upgrade note: new request headers This release sends two request headers that 2.6.0 did not: `Authorization` -(HTTP Basic, carrying your write key) and `X-Retry-Count` (on retries only). -If your traffic to Segment goes through a proxy, gateway or WAF that -allowlists request headers, add both before upgrading or uploads will be -rejected. Unity WebGL builds must also add them to the CORS -`Access-Control-Allow-Headers` allowlist on any proxy they point at. - -- Send the write key as an `Authorization: Basic` header. It is still included in the request body, so no server-side change is required. -- Send `X-Retry-Count` on retries, so the server can distinguish a retry from a first attempt. -- `HttpConfig` is now a settable property on `Configuration` rather than a constructor parameter, so retry behavior can be configured after construction. For mobile targets, CDN settings replace `Configuration.HttpConfig` when they are present. -- `Retry-After` is honoured on every retryable status rather than 429 alone, which brings 529 in through the generic 5xx rule. Numeric seconds and the RFC 7231 HTTP-date formats are both accepted, capped at `MaxRetryInterval`. -- New `RateLimitConfig.MaxRateLimitDuration` (default 12 hours) bounds how long a single rate-limit episode can keep a batch alive. Every other Segment SDK already had this; C# bounded the rate-limit path by a retry count alone. The count still stops retrying in practice — at the defaults it is reached long before the duration. -- `MaxRetryInterval` is now capped at 300s rather than 3600s, matching the fixed 300s ceiling in the other SDKs. -- 511 is dropped rather than retried: it asks the client to authenticate, which this library cannot do. -- Only 2xx responses count as a successful upload. A 3xx is now reported as a failed upload rather than silently treated as delivered. It is not retried: a redirect the HTTP client already declined to follow will not succeed on a retry. The Segment endpoint does not redirect, so this only affects custom host values. -- `RateLimitConfig.MaxRetryCount` and `BackoffConfig.MaxRetryCount` are floored at 1. A configured 0 previously dropped every batch before it was ever sent. -- `BackoffConfig.StatusCodeOverrides` is merged over the built-in defaults rather than replacing them, and is copied rather than held by reference. Previously, supplying an override for one status silently changed seven others: 408, 410, 429 and 460 stopped being retried, and 511 fell through to `Default5xxBehavior` and started being retried. A CDN settings payload whose overrides were all unparseable had the same effect. -- `BackoffConfig.MaxTotalBackoffDuration` is floored at 1 second. A configured 0 meant "no budget" — the batch was abandoned on its second attempt — rather than "no cap". +(HTTP Basic, carrying the write key) and `X-Retry-Count` (on retries only). If +traffic to Segment passes through a proxy, gateway or WAF that allowlists request +headers, add both before upgrading or uploads will be rejected. Unity WebGL builds +must also add them to the CORS `Access-Control-Allow-Headers` allowlist on any +proxy they point at. + +### Retry handling + +- A `Retry-After` header is honoured on any retryable response, not only 429. Numeric seconds and the RFC 7231 HTTP-date formats are both accepted, and the value is capped at `RateLimitConfig.MaxRetryInterval` (default 300 seconds). +- Responses carrying `Retry-After` are retried for up to `RateLimitConfig.MaxRateLimitDuration` (default 30 minutes), or `RateLimitConfig.MaxRetryCount` attempts (default 100), whichever comes first. Which one binds depends on the interval being served: below roughly `MaxRateLimitDuration / MaxRetryCount` — 18 seconds at the defaults — the count runs out first, above it the duration does. A `Retry-After` that will not fit in what is left of the budget ends the episode rather than being shortened, since resuming inside the window the server named sends a request it has already declined to serve. Other failures use exponential backoff from 500ms to a 60 second ceiling, limited by `BackoffConfig.MaxRetryCount` (default 10) and by `BackoffConfig.MaxTotalBackoffDuration` (default 12 hours) as an upper bound. +- 511 is dropped rather than retried: it asks the client to re-authenticate, which this library cannot do. +- `RetryBehavior`, `RateLimitConfig`, `BackoffConfig` and `HttpConfig` are now public, and `HttpConfig` is a settable property on `Configuration`, so retry behaviour can be configured in code. On mobile targets, CDN settings replace it when present. +- `BackoffConfig.StatusCodeOverrides` is merged over the built-in defaults rather than replacing them, so overriding one status leaves the rest unchanged. +- Retry counts and interval limits are clamped to usable ranges rather than accepted as given. + +### Other changes + +- The write key is sent as an `Authorization: Basic` header. It remains in the request body, so no server-side change is required. +- `X-Retry-Count` is sent on retries, allowing the server to distinguish a retry from a first attempt. +- Only 2xx responses count as a successful upload. A 3xx is reported as a failed upload rather than treated as delivered, and is not retried: a redirect the HTTP client has already declined to follow will not succeed on one. The Segment endpoint does not redirect, so this affects only custom host values. diff --git a/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs index a4c5077..e47dddd 100644 --- a/Tests/Retry/ConfigurationHttpConfigTest.cs +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -150,30 +150,65 @@ public void MaxTotalBackoffDurationOfZeroDoesNotAbandonOnTheSecondAttempt() } [Fact] - public void RateLimitCountIsReachedLongBeforeTheDurationBudget() + public void WhichRateLimitBoundBindsDependsOnTheRetryAfterBeingServed() { - // MaxRateLimitDuration is a last-ditch guard, not the working limit: the - // count is what should stop retrying at the defaults. If this ever inverts, - // batches start dying on a 12h timer instead of a countable number of tries. + // The previous version of this test asserted the duration was below the + // count's theoretical maximum span (100 x 300s), which is true and says + // nothing: it passes while the count is the limit actually reached. The + // crossover is what matters, and at the defaults it sits at 18 seconds -- + // so the count, not the duration, bounds every episode whose Retry-After + // is shorter than that, which is most of them. var rateLimit = new RateLimitConfig(); - long worstCaseSeconds = - (long)rateLimit.MaxRetryCount * RateLimitConfig.MaxRetryIntervalCeiling; + double crossoverSeconds = + (double)rateLimit.MaxRateLimitDuration / rateLimit.MaxRetryCount; + // Below the crossover the count runs out first. Assert.True( - worstCaseSeconds < rateLimit.MaxRateLimitDuration, - $"count trips after at most {worstCaseSeconds}s but the duration budget is " - + $"{rateLimit.MaxRateLimitDuration}s; the duration should never be reached first"); + rateLimit.MaxRetryCount * (crossoverSeconds / 2) < rateLimit.MaxRateLimitDuration, + "expected the count to bind for a Retry-After below the crossover"); + + // Above it the duration does. + Assert.True( + rateLimit.MaxRetryCount * (crossoverSeconds * 2) > rateLimit.MaxRateLimitDuration, + "expected the duration to bind for a Retry-After above the crossover"); + + // Documented so a change to either constant has to restate it. + Assert.Equal(18.0, crossoverSeconds); } [Fact] - public void RetryAfterIsCappedAtFiveMinutes() + public void DefaultBudgetLeavesRoomForMoreThanOneMaximalWait() { - // Other SDKs fix this at 300s; C# allowed configuring up to 3600s. - var validated = new RateLimitConfig(maxRetryInterval: 3600).Validated(); + // At parity the rate-limit path performs no retries at all. A response with + // no usable Retry-After waits MaxRetryInterval by default, ShouldUploadBatch + // tests elapsed time before the wait, so that one wait spends the budget and + // the next evaluation drops the batch — one attempt, having stalled the whole + // pipeline for the duration first. + var rateLimit = new RateLimitConfig(); + + Assert.True( + rateLimit.MaxRateLimitDuration > rateLimit.MaxRetryInterval, + $"budget is {rateLimit.MaxRateLimitDuration}s against a {rateLimit.MaxRetryInterval}s " + + "interval; a single maximal wait would consume it and leave no retry"); - Assert.Equal(RateLimitConfig.MaxRetryIntervalCeiling, validated.MaxRetryInterval); - Assert.Equal(300, validated.MaxRetryInterval); + Assert.True( + rateLimit.MaxRateLimitDuration / rateLimit.MaxRetryInterval >= 2, + "budget leaves room for fewer than two maximal waits"); + } + + [Fact] + public void RetryAfterCeilingClampsAnAbsurdValue() + { + // The ceiling exists to reject a nonsense header, not to shorten a + // reasonable one — a value inside it is honoured as given, because waiting + // less than asked only adds requests against a server already rate-limiting + // us. MaxRateLimitDuration is what bounds how long we keep trying. + Assert.Equal( + RateLimitConfig.MaxRetryIntervalCeiling, + new RateLimitConfig(maxRetryInterval: 3600).Validated().MaxRetryInterval); + + Assert.Equal(120, new RateLimitConfig(maxRetryInterval: 120).Validated().MaxRetryInterval); } [Fact] diff --git a/Tests/Retry/HttpConfigParserTest.cs b/Tests/Retry/HttpConfigParserTest.cs index b8791e4..0c2cc0b 100644 --- a/Tests/Retry/HttpConfigParserTest.cs +++ b/Tests/Retry/HttpConfigParserTest.cs @@ -50,11 +50,12 @@ public void Parse_BackoffConfig_ParsesValues() public void Parse_RateLimitConfig_ParsesValues() { var json = JsonUtility.FromJson( - "{\"rateLimitConfig\":{\"maxRetryCount\":\"10\",\"maxRetryInterval\":\"120\"}}"); + "{\"rateLimitConfig\":{\"maxRetryCount\":\"10\",\"maxRetryInterval\":\"45\"}}"); HttpConfig config = HttpConfigParser.Parse(json); Assert.Equal(10, config.RateLimitConfig.MaxRetryCount); - Assert.Equal(120, config.RateLimitConfig.MaxRetryInterval); + // Under the ceiling, so it survives validation unchanged. + Assert.Equal(45, config.RateLimitConfig.MaxRetryInterval); } [Fact] @@ -95,7 +96,7 @@ public void Parse_ClampsValues() HttpConfig config = HttpConfigParser.Parse(json); Assert.Equal(1000, config.RateLimitConfig.MaxRetryCount); - // Retry-After is capped at 300s, matching the other SDKs; it used to allow 3600. + // Clamped to the Retry-After ceiling. Assert.Equal(300, config.RateLimitConfig.MaxRetryInterval); Assert.Equal(60.0, config.BackoffConfig.BaseBackoffInterval); Assert.Equal(3600, config.BackoffConfig.MaxBackoffInterval); diff --git a/Tests/Retry/RetryStateMachineTest.cs b/Tests/Retry/RetryStateMachineTest.cs index 9d8df50..585ff44 100644 --- a/Tests/Retry/RetryStateMachineTest.cs +++ b/Tests/Retry/RetryStateMachineTest.cs @@ -486,11 +486,89 @@ public void GetRetryCount_GlobalHigher_ReturnsGlobal() Assert.Equal(10, machine.GetRetryCount(state, "batch1.json")); } + [Fact] + public void ARateLimitWaitThatCannotFitTheBudgetEndsTheEpisode() + { + // Shortening it to fit would resume inside the window the server named -- + // one it has already said it will not serve -- and ShouldUploadBatch would + // drop the batch on the elapsed check straight afterwards, so the shortened + // wait buys exactly one refused request. + var clock = new FakeTimeProvider(); + var machine = CreateMachine( + maxRetryCount: 1000, timeProvider: clock, maxRateLimitDuration: 300); + + long began = clock.CurrentTimeMillis(); + RetryState state = machine.HandleResponse( + new RetryState(), new ResponseInfo(429, 5, "b.json", began)); + + // 1 second of budget left, and the server asks for 60. + clock.Time += 299_000; + state = machine.HandleResponse( + state, new ResponseInfo(429, 60, "b.json", clock.CurrentTimeMillis())); + + Assert.Null(state.WaitUntilTime); + Assert.Null(state.RateLimitStartTime); + Assert.Equal(PipelineState.Ready, state.PipelineState); + Assert.False(state.BatchMetadata.ContainsKey("b.json")); + } + + [Fact] + public void ARateLimitWaitThatFitsIsHonouredInFull() + { + // "Never shorten" must not become "never wait". + var clock = new FakeTimeProvider(); + var machine = CreateMachine( + maxRetryCount: 1000, timeProvider: clock, maxRateLimitDuration: 300); + + long began = clock.CurrentTimeMillis(); + RetryState state = machine.HandleResponse( + new RetryState(), new ResponseInfo(429, 60, "b.json", began)); + + Assert.Equal(began + 60_000, state.WaitUntilTime); + Assert.Equal(PipelineState.RateLimited, state.PipelineState); + } + + [Fact] + public void ANonRetryableResponseEndsTheRateLimitEpisode() + { + // The request completed and carried no rate-limit signal. Leaving + // RateLimitStartTime set strands it -- nothing else clears it -- and the + // next batch evaluated after the budget elapses is dropped for a rate limit + // that ended here, without ever being uploaded. + var clock = new FakeTimeProvider(); + var machine = CreateMachine(timeProvider: clock, maxRateLimitDuration: 300); + + RetryState state = machine.HandleResponse( + new RetryState(), new ResponseInfo(429, 5, "b.json", clock.CurrentTimeMillis())); + Assert.NotNull(state.RateLimitStartTime); + + clock.Time += 1_000; + state = machine.HandleResponse( + state, new ResponseInfo(400, null, "b.json", clock.CurrentTimeMillis())); + + Assert.Null(state.RateLimitStartTime); + } + + [Fact] + public void RateLimitWaitIsUnclampedWhileTheBudgetIsAmple() + { + // The clamp must not shorten a wait that fits, or every Retry-After would + // be truncated to the episode deadline rather than honoured. + var clock = new FakeTimeProvider(); + var machine = CreateMachine(timeProvider: clock, maxRateLimitDuration: 300); + + long now = clock.CurrentTimeMillis(); + RetryState state = machine.HandleResponse( + new RetryState(), new ResponseInfo(429, 30, "b.json", now)); + + Assert.Equal(now + 30_000, state.WaitUntilTime); + } + [Fact] public void RateLimitEpisodeIsBoundedByMaxRateLimitDuration() { - // A pathological Retry-After stream used to be bounded only by a retry count; - // this is the wall-clock backstop the other SDKs have had all along. + // The wall-clock bound on one episode. Without it a pathological + // Retry-After stream is limited only by the retry count. var clock = new FakeTimeProvider(); var machine = CreateMachine( maxRetryCount: 1000, timeProvider: clock, maxRateLimitDuration: 60); diff --git a/Tests/Retry/RetryStateStorageTest.cs b/Tests/Retry/RetryStateStorageTest.cs index dfb89c9..0408065 100644 --- a/Tests/Retry/RetryStateStorageTest.cs +++ b/Tests/Retry/RetryStateStorageTest.cs @@ -52,10 +52,12 @@ public void RoundTrip_RateLimitedState() } [Fact] - public void RoundTrip_RateLimitStartTime() + public void RateLimitStartTimeIsNotCarriedAcrossRestarts() { - // MaxRateLimitDuration measures from this, so losing it across a restart - // would restart the episode clock and let a batch outlive its budget. + // It measures how long this process has been retrying. Carrying it over + // means an app closed for longer than MaxRateLimitDuration loads an + // already-expired episode and discards the batch on its first flush, + // having never retried it while running. The counts do carry over. var state = new RetryState( pipelineState: PipelineState.RateLimited, waitUntilTime: 1_700_000_030_000, @@ -65,7 +67,7 @@ public void RoundTrip_RateLimitStartTime() RetryStateStorage.SaveRetryState(_storage.Object, state); RetryState loaded = RetryStateStorage.LoadRetryState(_storage.Object); - Assert.Equal(1_700_000_000_000, loaded.RateLimitStartTime); + Assert.Null(loaded.RateLimitStartTime); Assert.Equal(1_700_000_030_000, loaded.WaitUntilTime); Assert.Equal(3, loaded.GlobalRetryCount); }