Add log rotation + startup retention cap (#2498). Principle V.#2765
Conversation
AsyncLogWriter gains a size-cap rotation hook driven by a callback set by LogManager. When the active file exceeds ActiveLogMaxMb (default 100), the writer thread drains its buffer to disk, closes the old file, and opens a new timestamped file under the same directory. The callback is the only seam back to LogManager — it updates the active path under m_pathMutex and re-points the aethersdr.log symlink so Support dialog and support-bundle scans continue to find the live file. Startup prune in main.cpp now goes through LogManager::pruneOldLogs which honors RetentionDays (default 7) and RetentionMaxTotalMb (default 500), keeping at least the two most-recent logs unconditionally so "yesterday's log" remains available even under aggressive caps. The hardcoded "keep 5" cap is removed. All three settings live under a single nested-JSON root key AppSettings["LogRetention"] per constitution Principle V (no new flat keys). AppSettings::load() and the SHistorySoftEdgeDb migration move above startLogging so retention config is read from disk on first start, not just on the second run. Size check fires only between batches (after the existing flush) so partial-batch state never straddles two files. Drop-summary lines and the writtenLines counter follow the rotation cleanly because they are already enqueued/recorded from inside the writer loop. SupportBundle.cpp:64-78 needs no change: it already scans the dir for aethersdr-*.log by QDir::Time, so mid-session rotation is picked up automatically. main.cpp's startup symlink creation stays as-is and the rotation callback handles subsequent symlink updates. Blast radius: risk_score=0.310 (LogManager.h widely #included), but this change is purely additive on LogManager — no existing method signature, semantic, or storage layout changes — so the 10 high-risk callers (MainWindow, RadioModel, etc.) are unaffected by recompile beyond rebuild cost.
There was a problem hiding this comment.
Nice work — the rotation/retention design reads cleanly, the snapshot-then-callback pattern keeps the writer thread autonomous, and the choice to rotate only between drained batches (so partial-batch state can't straddle two files) is exactly right. Wiring through AppSettings["LogRetention"] as nested JSON honors Principle V. Two things worth addressing before merge, plus one note:
1. Inconsistent state on rotation-open failure (AsyncLogWriter.cpp + LogManager.cpp)
In the rotation lambda in LogManager::startLogging, setActiveLogFilePath(candidate) and the symlink swap (QFile::remove/QFile::link) happen before the writer reopens the file. If file.open(newPath, ...) then fails in AsyncLogWriter::run, the writer correctly falls back to oldPath, but:
LogManager::m_activeLogFilePathstill points atcandidateaethersdr.logsymlink still points atcandidate- The writer is actually appending to
oldPath
Result: the Support dialog, logFileSize(), and any support-bundle scan would point at a file the writer isn't touching. Two ways to fix — either have the writer signal back failure so the callback can revert, or restructure the callback to compute-only and have the writer perform the rename/symlink swap after the new file opens successfully. The latter is simpler — the callback returns just the candidate path; AsyncLogWriter calls a second onRotated(newPath) hook only after open succeeds.
2. kAlwaysKeep = 2 can silently exceed the size cap on aggressive caps
In pruneOldLogs, the kept < kAlwaysKeep branch keeps the file and adds its size to cumulative regardless of overSize. That's intentional per the comment, but means a single 600 MB stale log on a 500 MB cap survives forever as one of the "always kept 2" and pushes subsequent files into the over-size branch where they're deleted — including newer, smaller logs that the user probably wants. Two cheap mitigations: either don't count always-kept files toward cumulative (so they're a floor, not a ceiling), or only protect always-kept from the age check, not the size check.
3. Note (not blocking): pruning is startup-only
pruneOldLogs runs once at startup, but setRotationConfig creates new files mid-session. Long-running sessions with frequent rotations can drift well above retentionMaxTotalMb until the next restart. The RetentionConfig field name suggests a hard cap; either rename to clarify it's a startup-time floor, or call pruneOldLogs from the rotation callback after a successful rotate (cheap, runs on writer thread, dir is small).
Other things I checked and they look fine:
- AppSettings usage (nested JSON via single key, not three flat keys) — matches Principle V
- RAII (
std::lock_guard,QFile,QMutexLocker) — clean - Rotation callback captures
[this]— safe given LogManager is a process-lifetime singleton - Snapshot of
maxFileBytes/rotationCbat writer-thread entry — correct given the "must call before start()" contract - Glob
aethersdr-*.logcorrectly excludes theaethersdr.logsymlink - main.cpp reordering (AppSettings load + SHistory migration moved above logging start) makes sense given LogManager now reads settings during
startLogging
Thanks!
Summary
Fixes #2498
What was changed
Add log rotation + startup retention cap (#2498). Principle V.
Files modified
src/core/AsyncLogWriter.cppsrc/core/AsyncLogWriter.hsrc/core/LogManager.cppsrc/core/LogManager.hsrc/main.cppGenerated by AetherClaude (automated agent for AetherSDR)