Skip to content

Commit 6360e85

Browse files
author
Frost
committed
Merge development into PR #2311
Resolve the FilamentCoverageSpoolResolver constructor conflict by retaining both the status-cache dependency from development and the reviewed spool deadline settings dependency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f293177-2aeb-4fc3-9faf-49bb53032dc3
2 parents 5e87347 + d1b8a6e commit 6360e85

4 files changed

Lines changed: 848 additions & 11 deletions

File tree

src/infra/Services/Spoolman/FilamentCoverageSpoolResolver.cs

Lines changed: 193 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Farm.Infrastructure.Services.Interfaces;
77
using Farm.Infrastructure.Services.Mutations;
88
using Farm.Infrastructure.Services.Printers;
9+
using Farm.Infrastructure.Settings;
910
using Microsoft.EntityFrameworkCore;
1011
using Microsoft.Extensions.Logging;
1112

@@ -20,9 +21,23 @@ public sealed class FilamentCoverageSpoolResolver(
2021
ILogger<FilamentCoverageSpoolResolver> logger,
2122
AppDbContext? db = null,
2223
IMutationWatermarkReader? watermarkReader = null,
23-
IPrinterStatusCacheReader? statusCache = null) : IFilamentCoverageSpoolResolver
24+
IPrinterStatusCacheReader? statusCache = null,
25+
ISettingsService? settingsService = null) : IFilamentCoverageSpoolResolver
2426
{
25-
private const int MaxConcurrentSourceRequests = 4;
27+
/// <summary>
28+
/// Upper bound on spool sources resolved at once. Distinct sources are
29+
/// independent hosts, so this exists only to stop a large farm creating
30+
/// unbounded fan-out against the network.
31+
///
32+
/// <para>
33+
/// This is NOT what keeps the endpoint inside the mobile client's readiness budget.
34+
/// Dark sources each hold a slot for the full per-source timeout, so N of them
35+
/// serialise into <c>ceil(N / MaxConcurrentSourceRequests)</c> waves and total latency
36+
/// still grows with fleet size. <see cref="SpoolCoverageSettings.FleetResolveTimeoutMs"/>
37+
/// is what bounds the endpoint, at any fleet size.
38+
/// </para>
39+
/// </summary>
40+
private const int MaxConcurrentSourceRequests = 8;
2641

2742
internal const string ReasonSpoolmanUnconfigured = "spoolman-unconfigured";
2843
internal const string ReasonSourceUnavailable = "spool-source-unavailable";
@@ -34,6 +49,67 @@ public sealed class FilamentCoverageSpoolResolver(
3449
private readonly AppDbContext? _db = db;
3550
private readonly IMutationWatermarkReader? _watermarkReader = watermarkReader;
3651
private readonly IPrinterStatusCacheReader? _statusCache = statusCache;
52+
private readonly ISettingsService? _settingsService = settingsService;
53+
54+
/// <summary>
55+
/// The read deadlines for one resolve operation. Captured once per call so every
56+
/// source in a fan-out uses the same values: <c>SettingsService.Get</c> enumerates a
57+
/// shared dictionary, so reading it from each of the concurrent source tasks can race
58+
/// a concurrent settings save and fall back to defaults for some sources but not
59+
/// others.
60+
/// </summary>
61+
private readonly record struct SpoolReadBudget(TimeSpan PerSource, TimeSpan Fleet);
62+
63+
/// <summary>
64+
/// Resolves the configured read deadlines, falling back to the
65+
/// <see cref="SpoolCoverageSettings"/> defaults when settings are unavailable.
66+
/// Coverage must never inherit the backend's print-control timeout for this
67+
/// read-only projection.
68+
/// </summary>
69+
private SpoolReadBudget ReadBudget()
70+
{
71+
SpoolCoverageSettings settings = new();
72+
try
73+
{
74+
if (_settingsService?.Get<SpoolCoverageSettings>() is SpoolCoverageSettings configured)
75+
{
76+
settings = configured;
77+
}
78+
}
79+
catch (Exception ex)
80+
{
81+
_logger.LogDebug(ex, "[FilamentCoverage] Falling back to default spool read budget");
82+
}
83+
84+
// Clamp both ends. Validate() enforces these ranges on the write path, but a row
85+
// persisted by another path (a migration, a direct edit) must not be able to
86+
// reintroduce the very stall this budget exists to prevent.
87+
return new SpoolReadBudget(
88+
TimeSpan.FromMilliseconds(Math.Clamp(settings.SpoolSourceTimeoutMs, 250, 30_000)),
89+
TimeSpan.FromMilliseconds(Math.Clamp(settings.FleetResolveTimeoutMs, 1_000, 60_000)));
90+
}
91+
92+
/// <summary>
93+
/// Renders a spool source's URL for logging with any embedded userinfo removed.
94+
/// <see cref="LogSanitizer"/> only defeats log forging; it does not strip credentials,
95+
/// and a server URL is free text an operator may have typed as
96+
/// <c>http://user:secret@host</c>.
97+
/// </summary>
98+
private static string? DescribeSource(string? serverUrl)
99+
{
100+
if (string.IsNullOrWhiteSpace(serverUrl))
101+
{
102+
return LogSanitizer.Sanitize(serverUrl);
103+
}
104+
105+
if (Uri.TryCreate(serverUrl, UriKind.Absolute, out Uri? uri) && !string.IsNullOrEmpty(uri.UserInfo))
106+
{
107+
UriBuilder redacted = new(uri) { UserName = string.Empty, Password = string.Empty };
108+
return LogSanitizer.Sanitize(redacted.Uri.ToString());
109+
}
110+
111+
return LogSanitizer.Sanitize(serverUrl);
112+
}
37113

38114
public async Task<FilamentCoverageSpoolSnapshot> ResolveSpoolAsync(
39115
CanonicalSpoolIdentity identity,
@@ -43,6 +119,10 @@ public async Task<FilamentCoverageSpoolSnapshot> ResolveSpoolAsync(
43119
.CaptureAsync(_watermarkReader, _logger, "source-qualified filament spool", ct)
44120
.ConfigureAwait(false);
45121
HashSet<int> spoolIds = [identity.SpoolId];
122+
123+
// Single source, so the fleet budget adds nothing beyond the per-source deadline;
124+
// pass the caller token through as the budget token.
125+
SpoolReadBudget budget = ReadBudget();
46126
Dictionary<int, FilamentCoverageSpoolSnapshot> resolved;
47127

48128
if (identity.SourceKind == SpoolSourceKind.Central)
@@ -80,7 +160,7 @@ public async Task<FilamentCoverageSpoolSnapshot> ResolveSpoolAsync(
80160
ReasonSourceUnavailable);
81161
}
82162

83-
resolved = await ResolveCentralAsync(spoolIds, originWatermark, ct).ConfigureAwait(false);
163+
resolved = await ResolveCentralAsync(spoolIds, originWatermark, budget, ct, ct).ConfigureAwait(false);
84164
}
85165
else
86166
{
@@ -113,6 +193,8 @@ public async Task<FilamentCoverageSpoolSnapshot> ResolveSpoolAsync(
113193
SpoolIds = { identity.SpoolId },
114194
},
115195
originWatermark,
196+
budget,
197+
ct,
116198
ct).ConfigureAwait(false);
117199
}
118200
catch (OperationCanceledException)
@@ -201,9 +283,13 @@ public async Task<FilamentCoverageSpoolSnapshot> ResolveSpoolAsync(
201283

202284
var request = new SourceRequest(selection.NativeClient, selection.ServerUrl);
203285
_ = request.SpoolIds.Add(spoolId);
286+
287+
// Single source, so the fleet budget adds nothing beyond the per-source deadline;
288+
// pass the caller token through as the budget token.
289+
SpoolReadBudget budget = ReadBudget();
204290
Dictionary<int, FilamentCoverageSpoolSnapshot> resolved = selection.Key.Native
205-
? await ResolveNativeAsync(request, originWatermark, ct).ConfigureAwait(false)
206-
: await ResolveCentralAsync(request.SpoolIds, originWatermark, ct).ConfigureAwait(false);
291+
? await ResolveNativeAsync(request, originWatermark, budget, ct, ct).ConfigureAwait(false)
292+
: await ResolveCentralAsync(request.SpoolIds, originWatermark, budget, ct, ct).ConfigureAwait(false);
207293

208294
return resolved.TryGetValue(spoolId, out FilamentCoverageSpoolSnapshot? snapshot)
209295
? snapshot
@@ -280,16 +366,53 @@ public async Task<IReadOnlyDictionary<Guid, IReadOnlyDictionary<int, FilamentCov
280366

281367
// Distinct spool sources are independent HTTP adapters. Resolve them in
282368
// parallel without allowing a large farm to create unbounded fan-out.
369+
//
370+
// The gate alone cannot bound this endpoint: dark sources each hold a slot for
371+
// the full per-source timeout, so they serialise into successive timeout waves
372+
// and total latency grows with fleet size. The fleet deadline below is what makes
373+
// the bound hold at any size — when it expires, sources still in flight degrade
374+
// to "unavailable" and the projection returns the coverage it already has.
375+
SpoolReadBudget budget = ReadBudget();
376+
using CancellationTokenSource fleetCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
377+
fleetCts.CancelAfter(budget.Fleet);
378+
CancellationToken budgetToken = fleetCts.Token;
379+
283380
using SemaphoreSlim sourceRequestGate = new(MaxConcurrentSourceRequests);
284381
Task<KeyValuePair<SourceKey, Dictionary<int, FilamentCoverageSpoolSnapshot>>>[] pendingSources =
285382
requests.Select(async pair =>
286383
{
287-
await sourceRequestGate.WaitAsync(ct).ConfigureAwait(false);
384+
try
385+
{
386+
await sourceRequestGate.WaitAsync(budgetToken).ConfigureAwait(false);
387+
}
388+
catch (OperationCanceledException) when (!ct.IsCancellationRequested)
389+
{
390+
// The fleet deadline expired while this source was still queued behind
391+
// the gate. Report it as unavailable rather than failing the projection.
392+
return KeyValuePair.Create(pair.Key, Failure(pair.Value.SpoolIds, pair.Key.Native, ReasonSourceUnavailable));
393+
}
394+
catch (OperationCanceledException)
395+
{
396+
// The CALLER cancelled while this source was still queued. WaitAsync throws
397+
// carrying the linked budget token, so normalise to the caller's token and
398+
// keep all three cancellation exits uniform.
399+
//
400+
// This is hardening, not a repair of observable behaviour: ResolveAsync
401+
// surfaces cancellation through Task.WhenAll, which reports the LOWEST-INDEX
402+
// cancelled task's token, and indices 0..MaxConcurrentSourceRequests-1 always
403+
// win the gate synchronously and so are always in flight. A queued source can
404+
// therefore never be the task whose token escapes. Normalising anyway means
405+
// the guarantee does not silently depend on that Task.WhenAll ordering detail
406+
// if the fan-out ever consumes sources in completion order instead.
407+
ct.ThrowIfCancellationRequested();
408+
throw;
409+
}
410+
288411
try
289412
{
290413
Dictionary<int, FilamentCoverageSpoolSnapshot> resolved = pair.Key.Native
291-
? await ResolveNativeAsync(pair.Value, originWatermark, ct).ConfigureAwait(false)
292-
: await ResolveCentralAsync(pair.Value.SpoolIds, originWatermark, ct).ConfigureAwait(false);
414+
? await ResolveNativeAsync(pair.Value, originWatermark, budget, budgetToken, ct).ConfigureAwait(false)
415+
: await ResolveCentralAsync(pair.Value.SpoolIds, originWatermark, budget, budgetToken, ct).ConfigureAwait(false);
293416
return KeyValuePair.Create(pair.Key, resolved);
294417
}
295418
finally
@@ -334,13 +457,31 @@ public async Task<IReadOnlyDictionary<Guid, IReadOnlyDictionary<int, FilamentCov
334457
private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveNativeAsync(
335458
SourceRequest request,
336459
long? originWatermark,
460+
SpoolReadBudget budget,
461+
CancellationToken budgetToken,
337462
CancellationToken ct)
338463
{
464+
TimeSpan timeout = budget.PerSource;
339465
try
340466
{
467+
// A powered-down printer that still holds its address black-holes packets
468+
// instead of refusing them, so this read must carry its own deadline. Without
469+
// it the call inherits the backend's print-control timeout (60s) and one dark
470+
// printer stalls the entire fleet projection. Linking off the fleet budget (not
471+
// the caller token) also cuts this read short when the overall deadline expires.
472+
using CancellationTokenSource linked = CancellationTokenSource.CreateLinkedTokenSource(budgetToken);
473+
linked.CancelAfter(timeout);
474+
341475
string? json = await request.NativeClient!
342-
.GetSpoolmanSpoolsAsync(request.ServerUrl!, ct)
476+
.GetSpoolmanSpoolsAsync(request.ServerUrl!, linked.Token)
343477
.ConfigureAwait(false);
478+
479+
// The Moonraker client swallows every exception from its Spoolman proxy and
480+
// reports a cancelled or timed-out call as a null body, so cancellation has to
481+
// be re-surfaced explicitly. Checking the LINKED token covers both cases; the
482+
// catch filters below then separate our timeout from a caller cancellation.
483+
linked.Token.ThrowIfCancellationRequested();
484+
344485
if (json is null)
345486
{
346487
return Failure(request.SpoolIds, true, ReasonSourceUnavailable);
@@ -351,20 +492,36 @@ private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveNative
351492
.ToDictionary(g => g.Key, g => g.First());
352493
return BuildSnapshots(request.SpoolIds, spools, true, originWatermark);
353494
}
495+
catch (OperationCanceledException) when (!ct.IsCancellationRequested)
496+
{
497+
// Either this source's own deadline or the fleet deadline. Both degrade; only a
498+
// genuine caller cancellation propagates, via the rethrow below.
499+
_logger.LogDebug(
500+
"[FilamentCoverage] Native Spoolman source timed out after {TimeoutMs}ms at {ServerUrl}",
501+
timeout.TotalMilliseconds,
502+
DescribeSource(request.ServerUrl));
503+
return Failure(request.SpoolIds, true, ReasonSourceUnavailable);
504+
}
354505
catch (OperationCanceledException)
355506
{
507+
// Reached only when the CALLER cancelled. Rethrow carrying the caller's token
508+
// rather than the internal linked one, so callers can identify their own
509+
// cancellation from ex.CancellationToken.
510+
ct.ThrowIfCancellationRequested();
356511
throw;
357512
}
358513
catch (Exception ex)
359514
{
360-
_logger.LogDebug(ex, "[FilamentCoverage] Native Spoolman source unavailable at {ServerUrl}", LogSanitizer.Sanitize(request.ServerUrl));
515+
_logger.LogDebug(ex, "[FilamentCoverage] Native Spoolman source unavailable at {ServerUrl}", DescribeSource(request.ServerUrl));
361516
return Failure(request.SpoolIds, true, ReasonSourceUnavailable);
362517
}
363518
}
364519

365520
private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveCentralAsync(
366521
HashSet<int> spoolIds,
367522
long? originWatermark,
523+
SpoolReadBudget budget,
524+
CancellationToken budgetToken,
368525
CancellationToken ct)
369526
{
370527
SpoolmanConfigDto? config = _spoolmanService.GetConfig();
@@ -373,8 +530,14 @@ private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveCentra
373530
return Failure(spoolIds, false, ReasonSpoolmanUnconfigured);
374531
}
375532

533+
TimeSpan timeout = budget.PerSource;
376534
try
377535
{
536+
// Bound the whole paged read, not each page: an unreachable central Spoolman
537+
// must not stall coverage any longer than a dark printer does.
538+
using CancellationTokenSource linked = CancellationTokenSource.CreateLinkedTokenSource(budgetToken);
539+
linked.CancelAfter(timeout);
540+
378541
const int pageSize = 500;
379542
Dictionary<int, SpoolmanSpoolDto> found = [];
380543
int offset = 0;
@@ -388,7 +551,15 @@ private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveCentra
388551
Offset = offset,
389552
AllowArchived = true,
390553
},
391-
ct).ConfigureAwait(false);
554+
linked.Token).ConfigureAwait(false);
555+
556+
// SpoolmanService.ListSpoolsAsync catches every exception - including
557+
// cancellation - and returns an EMPTY page. Without this check a timed-out
558+
// read would fall through to BuildSnapshots and be reported as
559+
// `spool-not-found`, i.e. an affirmative "that spool does not exist" claim
560+
// about a source we never actually reached. Re-surface cancellation here so
561+
// the catch filters below degrade to `spool-source-unavailable` instead.
562+
linked.Token.ThrowIfCancellationRequested();
392563

393564
foreach (SpoolmanSpoolDto spool in page.Items.Where(spool => spoolIds.Contains(spool.Id)))
394565
{
@@ -402,8 +573,19 @@ private async Task<Dictionary<int, FilamentCoverageSpoolSnapshot>> ResolveCentra
402573

403574
return BuildSnapshots(spoolIds, found, false, originWatermark);
404575
}
576+
catch (OperationCanceledException) when (!ct.IsCancellationRequested)
577+
{
578+
_logger.LogDebug(
579+
"[FilamentCoverage] Central Spoolman source timed out after {TimeoutMs}ms",
580+
timeout.TotalMilliseconds);
581+
return Failure(spoolIds, false, ReasonSourceUnavailable);
582+
}
405583
catch (OperationCanceledException)
406584
{
585+
// Reached only when the CALLER cancelled. Rethrow carrying the caller's token
586+
// rather than the internal linked one, so callers can identify their own
587+
// cancellation from ex.CancellationToken.
588+
ct.ThrowIfCancellationRequested();
407589
throw;
408590
}
409591
catch (Exception ex)

0 commit comments

Comments
 (0)