Bound the container log rings - #403
Merged
Merged
Conversation
Docker's json-file driver keeps every line forever unless told otherwise. Neither compose file said otherwise, so logs grew until the disk did not have room for them, and a scan appliance with no room left stops scanning. That is a storage failure that presents as a scanning failure, which is the slowest kind to diagnose. Twenty megabytes across five files, for every service that logs: 100 MB per service and 800 MB across the eight in the production file. Days of INFO-level history, and small beside the workspace volume the scans themselves need. LOG_MAX_SIZE and LOG_MAX_FILE change both numbers without editing a shipped file, because an operator raising the ring for a diagnosis would otherwise lose the change at the next upgrade, exactly when the diagnosis needed it. The dev file gets the same bound. A developer's disk fills the same way and DEBUG is louder there. The guard asserts over every service the files declare rather than a fixed list, so a service added later fails the test instead of quietly inheriting the unbounded default. It also pins that both options are present: max-size without max-file keeps every rotated file, and max-file without max-size never rotates, so either alone leaves the ring unbounded. The admin guide said no limit shipped, which was true when it was written this week and is not now.
The mirrored sentence used the have-construction that the style catalogue flags: a retention period is something the history follows, not something it possesses. Caught by running the linter from the branch instead of the copy in a stale worktree. The catalogue had gained the rule in the fifty-eight commits since that checkout, so the local run passed against an older set than CI enforces.
haksungjang
force-pushed
the
feat/log-rotation-limits
branch
from
September 6, 2026 07:11
89cf6bb to
40a0096
Compare
haksungjang
added a commit
that referenced
this pull request
Sep 6, 2026
…405) The adversarial-input timing assertion uses time.perf_counter(), which counts time the OS spent running something else on a shared CI runner the same as time this process actually spent computing. On PR #403 the same case failed three retries in a row at increasing wall-clock times (0.348s, 0.392s, 0.425s) against a 0.25s budget, for a call whose real cost is sub-millisecond - consistent with the process losing the CPU to another tenant, not with the parser getting slower. Switching to time.process_time() measures only CPU time actually spent, so scheduling gaps don't count against the budget while genuine algorithmic blowups still do.
Contributor
Author
|
Closing and reopening to re-trigger CI against current main, which now includes #405 (the CPU-time fix for the license-expression DoS guard that was blocking this PR's |
Contributor
Author
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
1 participant
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.
O6, the last of the observability items that was waiting on something. The eleven compose PRs that made this file unsafe to touch have all landed, and
logging:was still absent from both files.The failure this prevents
Docker's
json-filedriver keeps every line forever unless told otherwise. Neither compose file told it otherwise, so logs grew until the disk did not have room for them. A scan appliance with no room left stops scanning, which means this presents as a scanning failure and gets diagnosed as one.What ships
20 MB across 5 files, for every service that logs: 100 MB per service, 800 MB across the eight in the production file. Days of
INFO-level history, and small beside the workspace volume the scans themselves need.LOG_MAX_SIZEandLOG_MAX_FILEchange both without editing a shipped file. An operator raising the ring for a diagnosis would otherwise lose the change at the next upgrade, exactly when the diagnosis needed it.The dev file gets the same bound: a developer's disk fills the same way, and
DEBUGis louder there.The guard
test_compose_log_rotation.pyasserts over every service the files declare, not a fixed list, so a service added later fails the test rather than quietly inheriting the unbounded default. Nothing about that default is visible until the disk is gone.It also pins that both options are present, because either alone leaves the ring unbounded:
max-sizewithoutmax-filerotates and keeps every rotated filemax-filewithoutmax-sizenever rotates, so the count never appliesAnd that both stay environment-driven, so the tunability above cannot be quietly hard-coded away.
Docs
admin-guide/logs.mdsaid no limit shipped. That was true when it was written earlier this week and is not now; it describes the bound, the arithmetic and the two variables instead, in both locales. A note says plainly that rotation discards, and that the task-run history is unaffected because it lives in the database with its own retention.Verification
12 new tests, 67 related tests green,
ruffclean,docs-uat --lintclean, Korean style lint clean. Both compose files parse and every service resolves the anchor.