logmessaged: drop records too big for the msgq queue instead of dying - #314
Open
PeterPhuTran wants to merge 1 commit into
Open
logmessaged: drop records too big for the msgq queue instead of dying#314PeterPhuTran wants to merge 1 commit into
PeterPhuTran wants to merge 1 commit into
Conversation
logmessaged re-publishes every log record onto the logMessage msgq socket. msgq queues are 256KiB and msgq_msg_send() asserts that a message fits in a third of the queue, so any record over ~85KB is a SIGABRT - but the existing guard only rejected records over 2MB. Records in the 85KB-2MB band therefore killed the daemon, and since a crashed logmessaged takes down the whole logging pipeline, the crash itself is never logged and the processNotRunning alert persists for the rest of the drive. Hit in the field on 2026-07-10: updated logged a 130KB 'git diff output' record for a dirty working tree, logmessaged died at boot and stayed dead all day (see FrogAi#311 discussion). Lower the guard to fit the actual queue capacity. Oversized records are still written to the on-disk log (the file write happens before the publish) - they are only dropped from the msgq stream, same as the existing >2MB behavior. Adds test_medium_log: a 200KB record must not kill logmessaged, later records must still flow, and both must land on disk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for contributing! A maintainer will review your pull request soon. |
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
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.
Description
logmessaged re-publishes every log record onto the
logMessagemsgq socket. msgqqueues are 256KiB and
msgq_msg_send()asserts that a message fits in a third ofthe queue — so any record over ~85KB is a SIGABRT. The existing guard only rejected
records over 2MB, leaving an 85KB–2MB band where a single large log record kills the
daemon. And since a crashed logmessaged takes down the whole logging pipeline, the
crash itself can never be logged: the only symptom is a permanent
"Process Not Running: logmessaged" and silent loss of all swaglogs for the drive.
Hit in the field on 2026-07-10 (same device as #309/#311):
updatedlogged a 130KBgit diff outputrecord for a dirty working tree; logmessaged died seconds afterboot and stayed dead all day.
Fix: lower the guard to fit the actual queue capacity (
MAX_PUBLISH_BYTES = 80KB).Oversized records are still written to the on-disk log (the file write happens before
the publish) — they're only dropped from the msgq stream, exactly like the existing
Verification
New
test_medium_loginsystem/tests/test_logmessaged.py, validated on a comma three:and both records land in the on-disk log
Existing
test_simple_logpasses. Note:test_big_logfails on-device with andwithout this change (it globs the live
/data/logwhile a running logmessagedrotates files — a pre-existing test-environment race; it passes in isolated CI).
Related: #311 flags logmessaged
restart_if_crashas defense in depth; this PRremoves the crash trigger itself.
🤖 Generated with Claude Code