Repository navigation
[BUG] One container's stats timeout discards the whole resource-sampling round of its instance #270
Description
Summary
Resource statistics are sampled per running container, one stats request at a time. A container whose stats request answers non-success is simply left out of the round, but a container whose stats request times out (or breaks with a transport/JSON error) aborts the whole collection for the instance: no container sample and no instance sample is recorded for that round. A single hung container (a frequent case for a container under heavy load or in a broken state) thereby blanks the resource charts of every container on that host. A per-container timeout must only drop the affected container.
Area
docs/areas/docker-instances.md(Resource statistics, Limits and errors)- Security area: None
Current behavior
src/DockerUpdateGuard/Docker/DockerInstanceClient.cs:1052-1135 (CollectContainerResourceUsageAsync): the container list and all per-container stats requests run inside one try:
using var statsTimeoutSource = CreateRequestTimeoutCancellationTokenSource(instanceOptions.RequestTimeoutSeconds, cancellationToken);
using var statsResponse = await httpClient.GetAsync($"v1.41/containers/{...}/stats?stream=false", statsTimeoutSource.Token)...;
if (statsResponse.IsSuccessStatusCode == false)
{
continue; // non-success: only this container is dropped (line 1098-1101)
}
...
catch (OperationCanceledException) // per-container timeout lands here (line 1127-1130)
{
return ...Failed($"Docker container resource sampling timed out for '{instanceOptions.Name}' after {instanceOptions.RequestTimeoutSeconds} seconds");
}
catch (Exception exception) // transport error or malformed stats JSON (line 1131-1134)
{
return ...Failed($"Docker container resource sampling failed for '{instanceOptions.Name}': {exception.Message}");
}src/DockerUpdateGuard/Images/ResourceStatisticsCollector.cs:122-135 then logs a warning and returns without writing anything for the instance — neither the samples already collected for the other containers nor the instance sample.
Failure scenario: an instance runs 20 containers; container #7 does not answer its stats request within RequestTimeoutSeconds (default 15). The 6 samples already read and the 13 remaining containers are discarded, and the instance chart gets a gap. If container #7 stays hung, the instance never gets a sample again.
Expected behavior
Decision (product manager): a timeout drops only the affected container.
- A stats request of one container that times out, fails at the transport level, or returns a body that cannot be parsed leaves only that container out of the round, exactly like a non-success answer today. Sampling continues with the next container.
- Each dropped container is logged (warning, with instance name, container name/ID and the reason); the log level and wording must not flood the log for a permanently hung container more than once per round.
- The instance sample is recorded from the containers that were sampled.
- Edge case: when the container list contained at least one running container but no container could be sampled, the round is treated as failed for the instance (no instance sample, warning as today). Otherwise an engine that answers the list but hangs on all stats calls would record misleading samples with container count 0, CPU 0 and memory 0.
- Unchanged: a failed or timed-out container list request still fails the round for that instance; cancellation of the calling job (shutdown) still stops the operation and is not reported as a failure; other instances are never affected.
Acceptance criteria
- With three running containers where the second one's stats request exceeds the timeout, samples for the first and third container and one instance sample (count 2) are recorded.
- Same for a stats request that throws
HttpRequestExceptionand for a stats body that is not valid JSON. - With all stats requests timing out, no samples are recorded for the instance and a warning is logged.
- A cancelled job token still propagates
OperationCanceledException(no sample, no failure log). - A failing container list still records no samples for the instance.
- Unit tests (MSTest, naming per
docs/UNIT_TESTS.md), e.g.DockerInstanceClientCollectContainerResourceUsageWithOneStatsTimeoutSkipsOnlyThatContainer,…WithAllStatsTimeoutsReturnsFailed,…WhenCallerCancelsThrowsOperationCanceled; and for the collector (noResourceStatisticsCollectorTestsexist yet — add the class) a test that a partial result writes container and instance samples. Timeouts are simulated through a stubHttpMessageHandler/cancellation, never by real waiting. - The area document
docs/areas/docker-instances.mddescribes the new behavior (Resource statistics and Limits and errors) and its Known gaps entry for this issue (currently Open questions item 4) is removed.
Implementation notes
- Move the per-container stats call into its own
try/catchinside the loop (or a private helper returningRuntimeContainerResourceDescriptor?plus a reason). Distinguish the caller's cancellation (cancellationToken.IsCancellationRequested→ rethrow) from the per-request timeout (statsTimeoutSourcecancelled → skip container). - To implement the "nothing could be sampled" rule, the client has to report how many running containers were listed versus sampled (e.g. return failed when listed > 0 and sampled == 0, or extend the result data). Recommendation: keep it inside
DockerInstanceClientand return failed with a message such as "no container stats could be read for ''", soResourceStatisticsCollectorneeds no change for that case. - Consider the total duration: N hung containers ×
RequestTimeoutSecondscan now make one round long (before, the first timeout ended it). Collection rounds do not overlap (docs/areas/scan-engine.md), so this delays the next round but is acceptable; mention it in the area document. ResourceStatisticsCollectorlogs with_logger.LogWarningdirectly (ResourceStatisticsCollector.cs:129); new log entries should follow the source-generated logging pattern used elsewhere (DockerInstanceClientLogging.cs).- No configuration, README or migration impact. Must not change: integration clients return outcomes instead of throwing (
.squad/project.md, Guarantees).
Related
- F-026 (P3) —
ResourceStatisticsCollectormaterializes the instance's entire sample history per cycle #80 (ResourceStatisticsCollectormaterializes the instance's entire sample history per cycle — same method, independent fix) - PR [Docs] Describe the current behavior by area #263 (area documents)
Generated by Claude Code
Activity