breadbug client drops newest log lines when the queue lock is busy #76
Breadway
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found during independent exact-head review of bread-ecosystem PR 68 at
63b855bce6. Confidence high: reproduced twice under cargo test --workspace --locked (base, then --features extract), and the code path matches the failure. Repair on existing PR 68.What
Queue::push in bread-utils/src/bug/mod.rs:256-274 uses try_lock. If the worker holds the mutex inside take (mod.rs:278-287), push returns 1 and enqueue (mod.rs:518-530) counts the new line as dropped. The line is never stored. That is drop-newest, and it happens while the queue still has room (the failure lost 1 of 3 lines). Drop-oldest only runs after the lock is acquired and the queue is already at 512.
The worker holds that same mutex across Condvar::wait_timeout, so a producer that logs while the worker is idle-waiting loses the line and never wakes the worker. A later line can still be delivered, which is why the test sometimes sees "first" and the long line but not "third".
Evidence
Why it matters
BREAD-16 requires drop-oldest under backpressure. A busy drain must not discard a diagnostic that still fits. The previous review (comment 4431) rejected drop-newest; this repair fixed the full-queue case and reintroduced drop-newest on lock contention.
Repair
Keep the newest line when the lock is busy and the queue is under capacity (short spin, or a side slot the worker merges). Drop the oldest only at the 512 cap. Add a regression that contends take/wait with a producer and asserts the newest line is still delivered while len < 512. Do not weaken logs_are_batched_clipped_and_leveled.
Independent re-review of PR68 at
711e3e8261: ECO-77/provider implementation passes, conditional on exact-SHA CI green. Persisted verdict/evidence: #68 (comment). Queue nonblocking/drop-oldest tests and workspace suite passed; integrated store prerequisite is present, but CLIP-19 pilot remains unmerged. This is not full BREAD-16/17 acceptance. Preserve pilot/rollout gates, keep cards in review, and do not mark done. No merge until check/check (push) is green; then re-read current head/base/reviews/mergeability and merge per authorization, no tag.Independent final defect gate on merged PR68 (code head
711e3e8261, merge94ea37490f): ECO-77 defect acceptance PASS. Lock-free ArrayQueue enqueue retains the newest line below cap and evicts oldest only at 512; contended_push_drops_nothing_while_under_cap and producer_is_nonblocking_while_consumer_is_stalled pass. Merge tree verified equal to reviewed head (all 15 PR blobs identical; synthetic merge-tree of base4f70691+ head equals merge tree). Exact-SHA check/check (push) success on711e3e8(run 302) and on merge (run 303); reviews API empty. Own reruns on94ea374: bug:: 38 passed, bug_crashes 7 passed, raised/kill/fault signal probe. Full evidence and per-defect map: #68 (comment). Not full BREAD-16/17 acceptance: CLIP-19 pilot, rollout and overhead gates stay open. No tag, no done set by this pass; board transition is the coordinator's.Coordinator completion: final independent defect-only pass PR68 comment5312 recovered at exact711e3e8, integrated94ea374 tree verified and green check/devbakery/devtheme. Coordinator prior clean merged38bug+7crash tests pass, reviewer repeated38bug+7crash and fatal-signal probes. No full BREAD16/17 or pilot completion claim; those remain separate gates.