HarperFast / HarperFast/harper
logging.rotation has no count- or total-size-based cap on rotated logs (retention is age-only, and the shipped default sets none)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Feature Summary
`logging.rotation` can bound how *old* a rotated log is (`retention`), but not how *many* rotated logs are kept or how much disk they occupy in total. Add a count-based and/or total-size-based cap — e.g. `logging.rotation.maxFiles` and `logging.rotation.maxTotalSize` — so the rotated-log directory has a hard ceiling.
## Problem This Solves
Today the full set of knobs under `logging.rotation` is:
| key | effect |
| --- | --- |
| `enabled` | on/off |
| `compress` | gzip rotated files |
| `interval` | rotate every N (`D`/`H`/`M`/`m`) |
| `maxSize` | rotate when `hdb.log` exceeds N (`K`/`M`/`G`) |
| `retention` | delete rotated files **older than** N (`D`/`H`/`M`/`m`) |
| `path` | where rotated files land |
`retention` is the only cleanup mechanism, and it is purely age-based — `utility/logging/logRotator.ts` compares each file's `mtimeMs` against `convertToMS(retention)` and unlinks what is older:
https://github.com/HarperFast/harper/blob/main/utility/logging/logRotator.ts#L103-L126
There is no `maxFiles`, no `maxTotalSize`, and no equivalent anywhere in the config schema (`config-root.schema.json` → `logging.rotation`) or the validator (`validation/configValidator.ts`). Same in 4.x (`HarperDB/harperdb`), so this is not a 5.x regression.
Two consequences:
**1. The shipped default keeps rotated logs forever.** `static/defaultConfig.yaml` ships:
```yaml
logging:
rotation:
enabled: true
compress: false
interval: null
maxSize: 64M
path: null
```
No `retention` key. The cleanup branch is `if (retention || reclamationPriority)`, so with `retention` unset nothing is ever deleted — a default install rotates at 64 MB and accumulates rotated files indefinitely. Storage reclamation (`onStorageReclamation`) is the only backstop, and it only fires under disk pressure, i.e. after the problem already exists.
**2. Age-based retention does not actually bound disk.** `retention` bounds the *window*, not the *volume*. A node that is quiet normally but emits an error storm — a replication partner flapping, a bad component logging per-request at `debug`, a crash loop — can write hundreds of 64 MB files inside a `7D` window and fill the volume while the configured retention is nominally being honored. The operator's mental model of "I set retention, disk is bounded" is wrong, and there is no setting that makes it right.
Most log-rotation implementations offer both axes (logrotate `rotate N`, winston-daily-rotate-file `maxFiles` accepting either `14d` or `14`, Python's `RotatingFileHandler(backupCount=)`, Go's lumberjack `MaxBackups` + `MaxAge`). Harper only has the age axis.
Related but distinct: #2136 (an `external` logging block silently tears down its rotator) is about rotation being *disabled*; this issue is about rotation being *enabled and working as designed* while still not bounding disk.
## Suggested Direction
Add to `logging.rotation`, both optional and composable with `retention` (delete when **any** limit is exceeded, oldest-`mtime` first):
- **`maxFiles`** — integer; keep at most N rotated files.
- **`maxTotalSize`** — size string (`K`/`M`/`G`), same parser as `maxSize`; keep the rotated dir under N bytes in aggregate.
Implementation notes:
- The cleanup pass in `logRotator.ts` already `readdir`s the rotated dir and `stat`s every entry each tick, so it has the file list and sizes in hand — the count/size sweep is a sort-by-mtime plus an unlink loop on the same data, no extra I/O.
- The `if (retention || reclamationPriority)` guard needs to widen to include the new keys, otherwise setting only `maxFiles` is silently inert.
- `maxTotalSize` should be evaluated **after** compression, and the sweep should only consider files it created (the `HDB-*.log`/`.gz` pattern) rather than everything in the directory — `path` can be pointed at a shared dir.
- Sizing interacts with `maxSize`: worst case on disk is roughly `maxSize + maxTotalSize`, worth stating in the docs.
- Validation mirrors `validateRotationRetention` in `validation/configValidator.ts` (reject `<= 0`, reject non-integer for `maxFiles`).
- Consider whether the shipped default should gain a conservative cap (e.g. `maxFiles: 10`) so a fresh install is bounded out of the box. That is a behavior change for existing installs, so it may belong behind a major, but the current default of "keep everything" seems worth revisiting either way.
One thing worth checking before treating this as purely additive: `config/configUtils.ts` still carries a **deprecated** `logging.rotation.retain` key (`DEPRECATED_CONFIG`, inert — it only emits a deprecation warning) left over from the pre-rewrite logrotate-based implementation, alongside `rotate`, `rotateModule`, `rotateInterval`, `timezone`, `workerInterval`. If `retain` was a keep-N-files count in that older implementation, then count-based retention is a capability Harper lost in the rewrite rather than one it never had, and reusing the name may be worth considering.
## Additional Context
Verified against `HarperFast/harper` `origin/main` (5.2.1) and `HarperDB/harperdb` `origin/main` (4.x) — neither has a count or total-size cap.
Contributor guide
Research direction
Start in utility/logging/logRotator.ts, especially the retention cleanup around lines 103-126, then inspect config-root.schema.json, validation/configValidator.ts, and static/defaultConfig.yaml. Define how maxFiles and maxTotalSize compose with retention, validate their limits, and ensure cleanup handles only created log files after compression; done means the schema, validator, default behavior, and rotation cleanup agree.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- observability-sre
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100