From 39f7938c8a3b9e28a1097490c86698493b04c453 Mon Sep 17 00:00:00 2001 From: feruzm Date: Fri, 21 Aug 2026 09:21:25 +0000 Subject: [PATCH 1/3] review: no replacement fill after the lookup's deadline A coalesced reader whose fill was rejected retries only while its own deadline has not passed; past it the lookup is a timeout and starts nothing, so no fill is created for a reader that has already given up. --- dotnet/EcencyApi/Handlers/SsrRpc.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dotnet/EcencyApi/Handlers/SsrRpc.cs b/dotnet/EcencyApi/Handlers/SsrRpc.cs index 25711300..9d0ab2d2 100644 --- a/dotnet/EcencyApi/Handlers/SsrRpc.cs +++ b/dotnet/EcencyApi/Handlers/SsrRpc.cs @@ -268,15 +268,21 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa var bytes = await pending.Task; return new Resolution(coalesced ? Outcome.Coalesced : Outcome.Miss, bytes, null); } - catch (FillRejectedException) when (coalesced && !retried) + catch (FillRejectedException) when (coalesced && !retried && Environment.TickCount64 < deadline) { // The fill this reader attached to was judged expired (or refused) before // the reader's own wait began, which can only happen if the reader was // descheduled for longer than the budget between attaching and waiting. - // Its budget has not been spent on anything yet, so start over once. + // While its deadline has not passed, start over once; past it, the + // lookup is a timeout and no replacement fill is started for it. retried = true; goto again; } + catch (FillRejectedException) when (coalesced) + { + Interlocked.Increment(ref counter.Timeout); + return new Resolution(Outcome.Timeout, Array.Empty(), "budget exceeded"); + } catch (HiveRpcClient.RpcException e) { Interlocked.Increment(ref counter.Error); From 8a6cb09433e267767673ad3b3097474a50a0ca59 Mon Sep 17 00:00:00 2001 From: feruzm Date: Fri, 21 Aug 2026 10:29:53 +0000 Subject: [PATCH 2/3] review: a repeat rejection before the deadline stays unavailable Only a rejection past the lookup's deadline is reported as a timeout; a second rejection before it (queue full twice) keeps the unavailable result and its error accounting. --- dotnet/EcencyApi/Handlers/SsrRpc.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dotnet/EcencyApi/Handlers/SsrRpc.cs b/dotnet/EcencyApi/Handlers/SsrRpc.cs index 9d0ab2d2..f93a0e94 100644 --- a/dotnet/EcencyApi/Handlers/SsrRpc.cs +++ b/dotnet/EcencyApi/Handlers/SsrRpc.cs @@ -278,8 +278,10 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa retried = true; goto again; } - catch (FillRejectedException) when (coalesced) + catch (FillRejectedException) when (coalesced && Environment.TickCount64 >= deadline) { + // Past the deadline the lookup is a timeout; a repeat rejection + // before it falls through to the generic unavailable path below. Interlocked.Increment(ref counter.Timeout); return new Resolution(Outcome.Timeout, Array.Empty(), "budget exceeded"); } From d762fd5a61f80d60ed2fd0f88aae0d762392124a Mon Sep 17 00:00:00 2001 From: feruzm Date: Fri, 21 Aug 2026 10:32:17 +0000 Subject: [PATCH 3/3] review: regression test for the post-deadline rejection, behind a clock seam The lookup deadline and the attach/expiry timestamps read a replaceable clock (upstream timing stays real), so a test can drive a coalesced reader past its deadline before its queued fill is judged expired: the reader answers timeout, the timeout counter increments, the creator answers unavailable, and no replacement call goes upstream. --- dotnet/EcencyApi.Tests/SsrRpcTests.cs | 33 +++++++++++++++++++++++++++ dotnet/EcencyApi/Handlers/SsrRpc.cs | 18 +++++++++------ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/dotnet/EcencyApi.Tests/SsrRpcTests.cs b/dotnet/EcencyApi.Tests/SsrRpcTests.cs index 13a7afbe..71a97d77 100644 --- a/dotnet/EcencyApi.Tests/SsrRpcTests.cs +++ b/dotnet/EcencyApi.Tests/SsrRpcTests.cs @@ -87,6 +87,7 @@ private static void Use(RpcStub stub, long cacheBytes = 1 << 20, int budgetMs = SsrRpc.FillGate = new SemaphoreSlim(maxFills, maxFills); SsrRpc.MaxQueuedFills = maxQueued; SsrRpc.SecretDigest = null; + SsrRpc.Now = () => Environment.TickCount64; SsrRpc.ResetForTests(); } @@ -323,6 +324,38 @@ public async Task A_fresh_reader_that_coalesces_onto_a_queued_fill_keeps_it_aliv Assert.Equal(SsrRpc.Outcome.Hit, (await SsrRpc.Resolve(Post, P("k", "2"))).Outcome); } + [Fact] + public async Task A_coalesced_reader_whose_deadline_passed_before_its_fill_was_rejected_gets_timeout_and_no_replacement_fill() + { + await using var stub = new RpcStub { DelayMs = 300 }; + Use(stub, budgetMs: 1000, maxFills: 1); + // A controllable clock: real time drives the stub and the waits, the + // clock drives the deadline and the attach/expiry bookkeeping. + long offset = 0; + SsrRpc.Now = () => Environment.TickCount64 + Interlocked.Read(ref offset); + var timeoutsBefore = Interlocked.Read(ref SsrRpc.CounterFor(Post.Key).Timeout); + + var a = SsrRpc.Resolve(Post, P("d", "1")); // holds the gate for ~300ms + await Task.Delay(30); + var creator = SsrRpc.Resolve(Post, P("d", "2")); // queued fill, waits for the gate + await Task.Delay(30); + var late = SsrRpc.Resolve(Post, P("d", "2")); // coalesces onto it, deadline = now + 1000 + await Task.Delay(30); + // Jump the clock past every deadline and past the attach window, while + // the readers' real waits (1000ms) are still running. + Interlocked.Exchange(ref offset, 1_500); + // The gate frees at ~300ms real; the queued fill is then judged expired. + var results = await Task.WhenAll(a, creator, late); + + Assert.Equal(SsrRpc.Outcome.Miss, results[0].Outcome); + Assert.Equal(SsrRpc.Outcome.Unavailable, results[1].Outcome); // the creator is not coalesced + Assert.Equal(SsrRpc.Outcome.Timeout, results[2].Outcome); // past its deadline: timeout, no retry + Assert.Equal(timeoutsBefore + 1, Interlocked.Read(ref SsrRpc.CounterFor(Post.Key).Timeout)); + await Task.Delay(400); + Assert.Equal(1, stub.Hits); // no replacement fill went upstream + SsrRpc.Now = () => Environment.TickCount64; + } + [Fact] public async Task With_the_secret_configured_both_routes_serve_the_matching_header_and_nothing_else() { diff --git a/dotnet/EcencyApi/Handlers/SsrRpc.cs b/dotnet/EcencyApi/Handlers/SsrRpc.cs index f93a0e94..1c2c4e32 100644 --- a/dotnet/EcencyApi/Handlers/SsrRpc.cs +++ b/dotnet/EcencyApi/Handlers/SsrRpc.cs @@ -77,6 +77,10 @@ internal sealed record MethodPolicy(string Api, string Method, int TtlMs) internal static int BudgetMs = Config.SsrBudgetMs; + // Clock behind the lookup deadline and the attach/expiry timestamps; + // replaceable so tests can drive the post-deadline paths deterministically. + internal static Func Now = () => Environment.TickCount64; + // Bounds detached fills: a fill outlives the request budget on purpose, so // a slow pool plus many distinct keys must not pile up unbounded calls. internal static SemaphoreSlim FillGate = new(Config.SsrMaxConcurrentFills, Config.SsrMaxConcurrentFills); @@ -95,7 +99,7 @@ private sealed class Pending // the expiry decision both happen under `lock (this)`, so a reader can // never attach to a fill that has just decided to expire: it finds // Expired set and starts a fresh fill instead. - public long LastAttachMs = Environment.TickCount64; + public long LastAttachMs = Now(); public bool Expired; public bool TryAttach() @@ -103,7 +107,7 @@ public bool TryAttach() lock (this) { if (Expired) return false; - LastAttachMs = Environment.TickCount64; + LastAttachMs = Now(); return true; } } @@ -112,7 +116,7 @@ public bool TryExpire(int budgetMs) { lock (this) { - if (Environment.TickCount64 - LastAttachMs <= budgetMs) return false; + if (Now() - LastAttachMs <= budgetMs) return false; Expired = true; return true; } @@ -216,7 +220,7 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa } // One wall-clock budget for the whole lookup, retry included. - var deadline = Environment.TickCount64 + BudgetMs; + var deadline = Now() + BudgetMs; var retried = false; again: var coalesced = true; @@ -255,7 +259,7 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa if (coalesced) Interlocked.Increment(ref counter.Coalesced); else Interlocked.Increment(ref counter.Miss); - var remaining = (int)Math.Max(0, deadline - Environment.TickCount64); + var remaining = (int)Math.Max(0, deadline - Now()); var finished = await Task.WhenAny(pending.Task, Task.Delay(remaining)); if (!ReferenceEquals(finished, pending.Task)) { @@ -268,7 +272,7 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa var bytes = await pending.Task; return new Resolution(coalesced ? Outcome.Coalesced : Outcome.Miss, bytes, null); } - catch (FillRejectedException) when (coalesced && !retried && Environment.TickCount64 < deadline) + catch (FillRejectedException) when (coalesced && !retried && Now() < deadline) { // The fill this reader attached to was judged expired (or refused) before // the reader's own wait began, which can only happen if the reader was @@ -278,7 +282,7 @@ internal static async Task Resolve(MethodPolicy policy, JsonNode @pa retried = true; goto again; } - catch (FillRejectedException) when (coalesced && Environment.TickCount64 >= deadline) + catch (FillRejectedException) when (coalesced && Now() >= deadline) { // Past the deadline the lookup is a timeout; a repeat rejection // before it falls through to the generic unavailable path below.