fix: rotate bounded retention size scans - #367
codeforester wants to merge 2 commits into
Conversation
| # under the lock so another invocation can never turn a live bundle into a | ||
| # deletion candidate while discovery is in progress. | ||
| bundles = _discover_run_bundles( | ||
| size_scan_cursor = _read_size_scan_cursor(runs_root) |
There was a problem hiding this comment.
Concurrency: the byte-scan cursor is read and the expensive recursive size walk is performed entirely outside _retention_lock, so concurrent prune_run_bundles invocations can all read the same stale cursor and redundantly re-walk the same bundle range; whichever acquires the lock last simply overwrites the other's cursor, which can silently revert forward progress a faster sibling already made under sustained concurrent load.
| # The run index's cursor affects only which discovered bundles receive | ||
| # an expensive size walk. It never authorizes deletion; every candidate | ||
| # is re-read and revalidated before the destructive phase. | ||
| scan_order = sorted(bundles, key=lambda bundle: Path(bundle["path"]).name) |
There was a problem hiding this comment.
Efficiency (minor): _discover_run_bundles sorts bundles twice (once by (started_at, path), then again by name for scan_order) and re-wraps bundle["path"] in Path(...) even though it's already a Path. Capturing the name-sorted order once before the started_at sort would avoid the duplicate O(n log n) pass.
| "preserve": False, | ||
| }, | ||
| ) | ||
| # Retention now fails closed if a bundle has no lease record, |
There was a problem hiding this comment.
Test gap: the only true multi-process concurrency test for retention configures RetentionPolicy(max_bundles=2) with no max_total_bytes, so measure_sizes is always False and the new byte-scan-cursor path is never exercised under real multi-process concurrency — a race in the cursor read/write (see the other comment on _runtime.py:417) wouldn't be caught by the existing suite.
1 participant
Summary
Closes #355
Dependency
This PR is intentionally stacked on #366 (issue #354) so retention lease hardening merges first.
Validation
uv run --extra dev --extra typer --extra quality python -m pytest tests/test_run_bundle_retention.py -quv run --extra dev --extra typer --extra quality python -m mypy --strict lib/python/base_cli/_runtime.pygit diff --check