fix(tail): reduce ring buffer and byte-mode buffer caps from 64/32 MiB to 5 MiB#148
fix(tail): reduce ring buffer and byte-mode buffer caps from 64/32 MiB to 5 MiB#148
Conversation
MaxRingBytes: 64 MiB → 5 MiB Byte-mode circular buffer: 32 MiB → 5 MiB A single tail invocation could previously hold up to 64 MiB of live data in the ring buffer, and up to 32 MiB in byte mode. These limits are reduced to 5 MiB to keep single-command memory usage bounded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review this PR |
thieman
left a comment
There was a problem hiding this comment.
Self-Review Summary — PR #148: Reduce tail memory limits to 5 MiB
Overall assessment: safe to merge.
This PR reduces two memory caps in the tail builtin:
MaxRingBytes(line-mode ring buffer): 64 MiB → 5 MiBMaxBytesBuffer(byte-mode circular buffer): 32 MiB → 5 MiB- Bonus cleanup: replaces a 3-line
if/elsewithmax()builtin (semantically identical)
All constants, package-level doc comments, and inline function comments are updated consistently. No security concerns — this change is strictly a memory limit reduction. Sandbox integrity is unaffected.
Findings summary
Coverage summary
| Code path | Scenario test | Go test | Status |
|---|---|---|---|
MaxRingBytes constant reduction |
— | — | No direct test |
MaxBytesBuffer constant reduction |
— | — | No direct test |
max() refactor (suppressPos) |
tests/scenarios/cmd/tail/headers/ |
tail_test.go | Covered via header behavior tests |
| Ring line-count overflow error | — | tail_test.go:TestTailRingOverflowError |
Covered |
| Byte-mode overflow error | — | — | Missing |
Positive observations
- Constants, doc comments, and inline comments are all updated consistently — no stale references remain in
tail.goitself. - The
max()refactor is a clean simplification with identical semantics. - Memory reduction is in the right direction for the project's safety goals.
| // MaxBytesBuffer is the maximum size of the circular byte buffer used in | ||
| // last-N-bytes mode. | ||
| const MaxBytesBuffer = 32 << 20 // 32 MiB | ||
| const MaxBytesBuffer = 5 << 20 // 5 MiB |
There was a problem hiding this comment.
MaxBytesBuffer comment could mention the overflow scenario
The constant comment describes the buffer's role correctly, but it doesn't mention that when count > MaxBytesBuffer and input exceeds MaxBytesBuffer, readLastBytes returns an error. Consider adding a note matching the style of MaxRingBytes above:
| const MaxBytesBuffer = 5 << 20 // 5 MiB | |
| // MaxBytesBuffer is the maximum size of the circular byte buffer used in | |
| // last-N-bytes mode. If the input exceeds this size and the requested count | |
| // also exceeds it, tail returns an error rather than silently truncating. | |
| const MaxBytesBuffer = 5 << 20 // 5 MiB |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…B to 5 MiB (#148) * fix(tail): reduce ring buffer and byte-mode buffer caps to 5 MiB MaxRingBytes: 64 MiB → 5 MiB Byte-mode circular buffer: 32 MiB → 5 MiB A single tail invocation could previously hold up to 64 MiB of live data in the ring buffer, and up to 32 MiB in byte mode. These limits are reduced to 5 MiB to keep single-command memory usage bounded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style(tail): use builtin max() per modernize linter --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem
tailhad two memory limits well above 10 MiB per invocation:M-1: Ring buffer cap was 64 MiB
MaxRingBytes = 64 << 20— a singletail -n <large>on a file with long lines could hold 64 MiB of live data in the ring buffer simultaneously.M-2: Byte-mode circular buffer cap was 32 MiB
tail -c <large>allocated a circular buffer capped at 32 MiB.Both limits are far above the project's 10 MiB per-invocation guideline.
Fix
Both caps reduced to 5 MiB (
5 << 20).MaxTotalReadBytes(the read cap for infinite streams like pipes) is unchanged — that controls how much total data is consumed from an infinite source before bailing, which is a different concern from how much live data is held in memory.Test plan
tail -n 100 filestill works correctlytail -c 1000 filestill works correctlytail -n 1000000 filecorrectly returns the last lines within the 5 MiB cap🤖 Generated with Claude Code