Skip to content

perf: retention serializes concurrent invocations on an untimed exclusive lock #386

Description

@codeforester

Problem

Every invocation acquires a blocking, exclusive advisory lock on
runs/.base-cli-run-index.lock to do retention maintenance, and does so twice — once during
_create_context() via prune_run_bundles(), and once at teardown via RunRecorder.finish() →
refresh_run_bundle_index(). The lock is flock(LOCK_EX) with no timeout
(_lock_retention_stream(), lib/python/base_cli/_runtime.py:845-851).

Two consequences:

  1. Concurrent invocations of the same CLI serialize on housekeeping. Parallel fan-out is the
    normal shape for the target audience: xargs -P, parallel, a CI matrix, Ansible or a
    scheduler driving the same CLI across many targets.
  2. No timeout means no bound. A process stopped (SIGSTOP), paged out, or blocked on a hung
    network filesystem while holding the lock blocks every other invocation of that CLI
    indefinitely, with no diagnostic. Retention is explicitly maintenance —
    prune_run_bundles()'s own comment says "an unavailable lock ... must not turn an otherwise
    valid invocation into a command failure" — but an unavailable lock currently blocks forever
    rather than being unavailable.

docs/performance.md documents per-pass work bounds and the retention suite covers "concurrent
invocations" for correctness, but no benchmark scenario measures concurrent cost, so this is
ungated.

Verified evidence

Reviewed 2026-09-30 at a58ec109349fa3f3d03eae5b0de078b39ea361a2 (macOS, Python 3.14.6, Click 8.4.2).

Same CLI, same cache root, warmed to the default steady state of 20 retained bundles. Each number
is the in-process run_app() duration reported by the child:

serial baseline (5 runs):        17.7  18.1  18.6  19.2  18.3   ms
12 concurrent invocations:       50.6  50.3  64.3  64.4  68.3  66.3
                                 67.5  66.2  66.9  67.6  68.0  55.8  ms

Median goes from ~18 ms to ~66 ms — about 3.5x — purely from housekeeping contention.

Per-invocation syscall profile at steady state, instrumented:

{'os.walk': 20, 'write_private_json': 2, 'retention_lock': 2, 'discover': 2}

Note os.walk: 20. RetentionPolicy.safe_defaults() sets max_total_bytes=512 MiB, so the
default policy opts every invocation into the recursive size-measurement path that
docs/performance.md describes as the byte-policy column of its work-bounds table. The doc does not
point out that the default lands in that column.

Proposal

  1. Make the retention lock non-blocking with a short bounded fallback (LOCK_NB, then a brief
    retry budget). If it cannot be acquired, skip the pass — another invocation is already doing it —
    and log at debug. Retention is idempotent and self-healing by design; skipping is correct.
  2. Do not run a retention pass on every invocation. Gate it on cheap observable state: a stamp file
    with a minimum interval, a bundle-count threshold, or probabilistic sampling. The existing
    policy-debt mechanism already tolerates deferred work.
  3. Do not do the teardown refresh_run_bundle_index() pass under the same lock on every run; fold
    it into the gated pass.
  4. Reconsider whether max_total_bytes belongs in safe_defaults(), given it is the only bound
    that requires recursive walks. If it stays, cache per-bundle sizes in the index and re-measure
    only when run.json mtime changes.

Acceptance criteria

  • No invocation blocks indefinitely on the retention lock; contention degrades to a skipped pass.
  • 12 concurrent invocations of the same CLI stay within a small multiple of the serial cost, and a
    benchmark scenario gates it.
  • Retention still converges: a test that runs N invocations past the cap asserts the bound is
    eventually enforced even though individual passes are skipped.
  • docs/performance.md states that the default policy uses the byte-policy work bounds.

Non-goals

  • Do not weaken the revalidate-under-lock guarantee before destructive work.
  • Do not move retention to a background thread or daemon.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area: runtimeRuntime, lifecycle, execution, or process-boundary ownership.enhancementNew feature or product improvement

Type

No type

Projects

  • Status
    Backlog

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions