mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 02:19:07 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
73fff8d2d5 | ||
|
|
446207ffbc |
+1
-54
@@ -130,12 +130,7 @@ def setup_logging(
|
|||||||
# Console handler (always add)
|
# Console handler (always add)
|
||||||
console_handler = logging.StreamHandler(sys.stdout)
|
console_handler = logging.StreamHandler(sys.stdout)
|
||||||
console_handler.setLevel(level)
|
console_handler.setLevel(level)
|
||||||
# Under systemd, tag each line so the journal records the real severity
|
console_handler.setFormatter(formatter)
|
||||||
# rather than filing everything as informational. The file handler below
|
|
||||||
# keeps the plain formatter: the prefix is meaningful to journald and noise
|
|
||||||
# anywhere else.
|
|
||||||
console_handler.setFormatter(
|
|
||||||
JournalPriorityFormatter(formatter) if _under_systemd() else formatter)
|
|
||||||
root_logger.addHandler(console_handler)
|
root_logger.addHandler(console_handler)
|
||||||
|
|
||||||
# File handler (if specified)
|
# File handler (if specified)
|
||||||
@@ -150,54 +145,6 @@ def setup_logging(
|
|||||||
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
|
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
|
||||||
|
|
||||||
|
|
||||||
#: syslog priorities, which is what systemd parses from a "<N>" prefix on
|
|
||||||
#: stdout. Mapped from Python's levels.
|
|
||||||
_SYSLOG_PRIORITY = {
|
|
||||||
logging.CRITICAL: 2, # LOG_CRIT
|
|
||||||
logging.ERROR: 3, # LOG_ERR
|
|
||||||
logging.WARNING: 4, # LOG_WARNING
|
|
||||||
logging.INFO: 6, # LOG_INFO
|
|
||||||
logging.DEBUG: 7, # LOG_DEBUG
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
class JournalPriorityFormatter(logging.Formatter):
|
|
||||||
"""Wraps a formatter, prefixing each line with its syslog priority.
|
|
||||||
|
|
||||||
Under systemd everything this process writes to stdout lands in the journal
|
|
||||||
as PRIORITY=6, whatever the Python level was. Measured on a live rig: 55
|
|
||||||
ERROR lines and 13 WARNING lines in a day, every one of them recorded as
|
|
||||||
informational, so `journalctl -p err -u ledmatrix` returned nothing at all
|
|
||||||
while errors were being logged. Anyone triaging has to grep the message
|
|
||||||
text instead, which is both slower and wrong -- a search for "oom" matches
|
|
||||||
the radar logging "zoom=9".
|
|
||||||
|
|
||||||
systemd reads a leading "<N>" on each line and uses it as the priority
|
|
||||||
(sd-daemon(3)), so this needs no extra dependency. Multi-line records get
|
|
||||||
the prefix on every line, since the journal splits them and an unprefixed
|
|
||||||
continuation would fall back to the default.
|
|
||||||
"""
|
|
||||||
|
|
||||||
def __init__(self, inner: logging.Formatter):
|
|
||||||
super().__init__()
|
|
||||||
self._inner = inner
|
|
||||||
|
|
||||||
def format(self, record: logging.LogRecord) -> str:
|
|
||||||
text = self._inner.format(record)
|
|
||||||
prefix = f"<{_SYSLOG_PRIORITY.get(record.levelno, 6)}>"
|
|
||||||
return "\n".join(prefix + line for line in text.split("\n"))
|
|
||||||
|
|
||||||
|
|
||||||
def _under_systemd() -> bool:
|
|
||||||
"""True when stdout is the journal.
|
|
||||||
|
|
||||||
systemd sets JOURNAL_STREAM for services whose output it captures. Without
|
|
||||||
this check the "<N>" prefixes would show up as literal noise when the
|
|
||||||
program is run from a terminal, in the emulator, or in tests.
|
|
||||||
"""
|
|
||||||
return bool(os.environ.get("JOURNAL_STREAM"))
|
|
||||||
|
|
||||||
|
|
||||||
class PluginLoggerAdapter(logging.LoggerAdapter):
|
class PluginLoggerAdapter(logging.LoggerAdapter):
|
||||||
"""LoggerAdapter that stamps every record with its plugin_id.
|
"""LoggerAdapter that stamps every record with its plugin_id.
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,18 @@ Type=simple
|
|||||||
User=root
|
User=root
|
||||||
WorkingDirectory=__PROJECT_ROOT_DIR__
|
WorkingDirectory=__PROJECT_ROOT_DIR__
|
||||||
Environment=PYTHONDONTWRITEBYTECODE=1
|
Environment=PYTHONDONTWRITEBYTECODE=1
|
||||||
|
# glibc gives each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||||
|
# and an arena that has grown is never handed back to the OS. This process runs
|
||||||
|
# 9 threads on a 3-core Pi, so the ceiling is 24 arenas -- and a rig measured at
|
||||||
|
# 1030 MB resident held 23 large anonymous mappings on 64 MB-aligned addresses,
|
||||||
|
# 920 MB of them, while the live data it was actually holding (widest scroll
|
||||||
|
# strip seen: 35,746 x 64) accounts for roughly 15 MB. That gap is arena bloat,
|
||||||
|
# not leaked objects: RSS was flat across repeated sampling, not climbing.
|
||||||
|
#
|
||||||
|
# Capping the arenas trades a little allocator concurrency for a large amount of
|
||||||
|
# resident memory on a device that has neither to spare. 2 is the usual value;
|
||||||
|
# raise it if frame times regress.
|
||||||
|
Environment=MALLOC_ARENA_MAX=2
|
||||||
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
|
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
|
||||||
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
|
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
|
||||||
# for a reason that no longer applies, e.g. a config reload) would otherwise leave
|
# for a reason that no longer applies, e.g. a config reload) would otherwise leave
|
||||||
|
|||||||
@@ -1,101 +0,0 @@
|
|||||||
"""Log lines must reach the journal with their real severity.
|
|
||||||
|
|
||||||
Everything this process writes to stdout lands in the journal as PRIORITY=6,
|
|
||||||
whatever the Python level was, because journald has no other signal. Measured
|
|
||||||
on a live rig over 24 hours: 55 lines containing " - ERROR - " and 13
|
|
||||||
containing " - WARNING - ", every one of them recorded as informational. So
|
|
||||||
|
|
||||||
journalctl -p err -u ledmatrix
|
|
||||||
|
|
||||||
returned nothing while errors were being logged, and anyone triaging has to
|
|
||||||
grep the message text instead. That is slower and it is wrong: a search for
|
|
||||||
"oom" also matches the radar logging "zoom=9", which is exactly the false
|
|
||||||
positive it produced during this audit.
|
|
||||||
|
|
||||||
systemd reads a leading "<N>" on each stdout line and uses it as the priority
|
|
||||||
(sd-daemon(3)), so this needs no extra dependency -- and it must only be
|
|
||||||
applied when systemd is actually reading, or the prefixes become literal noise
|
|
||||||
in a terminal, the emulator, and test output.
|
|
||||||
"""
|
|
||||||
import logging
|
|
||||||
import os
|
|
||||||
from unittest.mock import patch
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from src.logging_config import JournalPriorityFormatter, _SYSLOG_PRIORITY, _under_systemd
|
|
||||||
|
|
||||||
|
|
||||||
class _Plain(logging.Formatter):
|
|
||||||
def format(self, record):
|
|
||||||
return record.getMessage()
|
|
||||||
|
|
||||||
|
|
||||||
def _record(level, msg="hello"):
|
|
||||||
return logging.LogRecord("t", level, "f.py", 1, msg, None, None)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("level,expected", [
|
|
||||||
(logging.CRITICAL, 2),
|
|
||||||
(logging.ERROR, 3),
|
|
||||||
(logging.WARNING, 4),
|
|
||||||
(logging.INFO, 6),
|
|
||||||
(logging.DEBUG, 7),
|
|
||||||
])
|
|
||||||
def test_each_level_maps_to_its_syslog_priority(level, expected):
|
|
||||||
out = JournalPriorityFormatter(_Plain()).format(_record(level))
|
|
||||||
assert out.startswith(f"<{expected}>"), out
|
|
||||||
assert _SYSLOG_PRIORITY[level] == expected
|
|
||||||
|
|
||||||
|
|
||||||
def test_error_and_info_are_distinguishable():
|
|
||||||
"""The whole point: journalctl -p err must be able to tell them apart."""
|
|
||||||
fmt = JournalPriorityFormatter(_Plain())
|
|
||||||
assert fmt.format(_record(logging.ERROR))[:3] != fmt.format(_record(logging.INFO))[:3]
|
|
||||||
|
|
||||||
|
|
||||||
def test_every_line_of_a_multiline_record_is_tagged():
|
|
||||||
"""The journal splits them, and an untagged continuation loses its level.
|
|
||||||
|
|
||||||
A traceback is the case that matters -- it is the most important thing in
|
|
||||||
the log and the longest.
|
|
||||||
"""
|
|
||||||
out = JournalPriorityFormatter(_Plain()).format(
|
|
||||||
_record(logging.ERROR, "Traceback:\nline one\nline two"))
|
|
||||||
lines = out.split("\n")
|
|
||||||
assert len(lines) == 3
|
|
||||||
assert all(line.startswith("<3>") for line in lines), lines
|
|
||||||
|
|
||||||
|
|
||||||
def test_the_message_survives_intact():
|
|
||||||
out = JournalPriorityFormatter(_Plain()).format(_record(logging.WARNING, "disk full"))
|
|
||||||
assert out == "<4>disk full"
|
|
||||||
|
|
||||||
|
|
||||||
def test_an_unknown_level_falls_back_to_info():
|
|
||||||
out = JournalPriorityFormatter(_Plain()).format(_record(25))
|
|
||||||
assert out.startswith("<6>")
|
|
||||||
|
|
||||||
|
|
||||||
def test_prefixing_is_off_outside_systemd():
|
|
||||||
"""Otherwise a terminal run, the emulator and pytest all show `<6>`."""
|
|
||||||
with patch.dict(os.environ, {}, clear=True):
|
|
||||||
assert not _under_systemd()
|
|
||||||
with patch.dict(os.environ, {"JOURNAL_STREAM": "8:12345"}):
|
|
||||||
assert _under_systemd()
|
|
||||||
|
|
||||||
|
|
||||||
def test_setup_uses_the_wrapper_only_under_systemd():
|
|
||||||
from src.logging_config import setup_logging
|
|
||||||
|
|
||||||
for env, expect_wrapped in (({}, False), ({"JOURNAL_STREAM": "8:1"}, True)):
|
|
||||||
with patch.dict(os.environ, env, clear=True):
|
|
||||||
setup_logging()
|
|
||||||
handlers = [h for h in logging.getLogger().handlers
|
|
||||||
if isinstance(h, logging.StreamHandler)]
|
|
||||||
assert handlers, "no stream handler installed"
|
|
||||||
wrapped = any(isinstance(h.formatter, JournalPriorityFormatter)
|
|
||||||
for h in handlers)
|
|
||||||
assert wrapped is expect_wrapped, (
|
|
||||||
f"JOURNAL_STREAM={env}: wrapped={wrapped}, expected {expect_wrapped}")
|
|
||||||
logging.getLogger().handlers.clear()
|
|
||||||
@@ -0,0 +1,95 @@
|
|||||||
|
"""The display unit must cap glibc's malloc arenas.
|
||||||
|
|
||||||
|
glibc hands each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||||
|
and an arena that has grown is never returned to the OS. This process runs
|
||||||
|
threads for the render loop, the update workers and the background fetchers, so
|
||||||
|
on a 3-core Pi the ceiling is 24 arenas.
|
||||||
|
|
||||||
|
Measured on a live rig, 2.5 hours in:
|
||||||
|
|
||||||
|
RSS 1030 MB
|
||||||
|
Private_Dirty 988 MB
|
||||||
|
anonymous mappings > 10 MB 23 (ceiling is 8 x 3 = 24)
|
||||||
|
largest few 104, 79, 66, 63, 63 MB, on 64 MB-aligned addresses
|
||||||
|
|
||||||
|
against live data that accounts for perhaps 15 MB -- the widest scroll strip
|
||||||
|
observed was 35,746 x 64, about 7 MB as RGB and the same again for its numpy
|
||||||
|
mirror. Repeated sampling showed RSS flat between 990 and 1030 MB rather than
|
||||||
|
climbing, so this is arena bloat rather than a leak: memory Python has freed
|
||||||
|
but glibc is holding per-arena.
|
||||||
|
|
||||||
|
The device had 59 MB free at the time.
|
||||||
|
|
||||||
|
Capping the arena count trades a little allocator concurrency for that resident
|
||||||
|
memory. The render loop is latency-sensitive, so if p99 frame time regresses the
|
||||||
|
right response is to raise this rather than remove it.
|
||||||
|
"""
|
||||||
|
import re
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
UNIT = (Path(__file__).resolve().parent.parent / "systemd" / "ledmatrix.service")
|
||||||
|
|
||||||
|
#: The value the unit is expected to carry. 2 is the usual choice for a
|
||||||
|
#: threaded Python process; 1-4 all keep some of the saving, but only one of
|
||||||
|
#: them is what this project ships.
|
||||||
|
EXPECTED_ARENA_MAX = 2
|
||||||
|
|
||||||
|
|
||||||
|
def _environment(unit_text):
|
||||||
|
return dict(
|
||||||
|
line.split("=", 2)[1:3] if line.count("=") >= 2 else (line.split("=", 1)[1], "")
|
||||||
|
for line in unit_text.splitlines()
|
||||||
|
if line.startswith("Environment=")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_unit_exists():
|
||||||
|
assert UNIT.is_file(), f"{UNIT} is missing"
|
||||||
|
|
||||||
|
|
||||||
|
def test_malloc_arena_max_is_capped():
|
||||||
|
env = _environment(UNIT.read_text(encoding="utf-8"))
|
||||||
|
assert "MALLOC_ARENA_MAX" in env, (
|
||||||
|
"the display unit does not cap glibc arenas; on a 3-core Pi the default "
|
||||||
|
"ceiling is 24 and a measured rig held 23 of them, 920 MB"
|
||||||
|
)
|
||||||
|
value = int(env["MALLOC_ARENA_MAX"])
|
||||||
|
# Pinned, not a range. A range let a change to 4 -- which hands most of the
|
||||||
|
# saving back -- pass unnoticed, which was the point of the finding that
|
||||||
|
# prompted this. Raising it is a legitimate response to a frame-time
|
||||||
|
# regression, but it should be a visible edit here rather than a silent
|
||||||
|
# drift, so the number lives in one place and changing it shows up in
|
||||||
|
# review.
|
||||||
|
assert value == EXPECTED_ARENA_MAX, (
|
||||||
|
f"MALLOC_ARENA_MAX={value}, expected {EXPECTED_ARENA_MAX}. If this was "
|
||||||
|
"raised deliberately because frame times regressed, update "
|
||||||
|
"EXPECTED_ARENA_MAX here and say so in the commit."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reason_is_recorded_next_to_it():
|
||||||
|
"""A bare tuning knob invites removal by whoever meets it next."""
|
||||||
|
text = UNIT.read_text(encoding="utf-8")
|
||||||
|
index = text.index("Environment=MALLOC_ARENA_MAX")
|
||||||
|
preamble = text[:index].splitlines()[-12:]
|
||||||
|
comment = "\n".join(line for line in preamble if line.startswith("#"))
|
||||||
|
assert "arena" in comment.lower(), "no explanation precedes the setting"
|
||||||
|
assert re.search(r"\d", comment), (
|
||||||
|
"the explanation cites no measurement, so a reader cannot tell whether "
|
||||||
|
"it still applies to their hardware"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("unit", ["ledmatrix.service"])
|
||||||
|
def test_the_unit_still_parses_as_ini(unit):
|
||||||
|
"""systemd will refuse a malformed unit, and the panel stays dark."""
|
||||||
|
import configparser
|
||||||
|
|
||||||
|
path = UNIT.parent / unit
|
||||||
|
parser = configparser.ConfigParser(strict=False)
|
||||||
|
# systemd allows repeated keys; ConfigParser needs them merged, not rejected.
|
||||||
|
parser.read_string(path.read_text(encoding="utf-8"))
|
||||||
|
assert parser.has_section("Service")
|
||||||
|
assert parser.has_option("Service", "ExecStart")
|
||||||
Reference in New Issue
Block a user