Job retention

[airhorns]

Summary

• Removes DeleteTerminalJob task variant, cleanup_buffer from the broker, and all per-job cleanup task scheduling/clearing from cancel, lease, restart, and import paths
• Adds a periodic background scanner (spawn_retention_scanner) that finds and deletes expired terminal jobs by scanning the 0x03 status/time index
• Upgrades config to use Duration-based fields with duration_str parsing (SlateDB pattern) for default_terminal_retention and retention_scan_interval

The scanner runs at a configurable interval (default 12h), skips non-terminal entries via key-only parsing, and deletes expired jobs in serializable transactions with concurrent-restart guards. Net -122 lines.

Test plan

• cargo test --test job_retention_tests — all 10 retention-specific tests pass (scanner-based deletion, restart race, explicit delete, import retention)
• cargo test --test job_store_shard_import_tests — all 80 import tests pass (no cleanup task assertions)
• cargo test -- --skip k8s_ --skip etcd_ --skip coordination_split_tests --skip turmoil — full test suite passes
• cargo clippy — no warnings

🤖 Generated with Claude Code

---

Note

Medium Risk
Adds a background retention scanner that deletes terminal jobs and changes job storage/proto/config surfaces to include retention; mistakes could lead to unintended job deletions or extra load from periodic scans.

Overview
Adds terminal job retention: jobs now persist a terminal_retention_ms value at enqueue/import time (defaulting from shard config) and the shard spawns a periodic retention scanner that scans the status/time index and deletes expired terminal jobs transactionally.

Extends the API/storage surface to carry retention (proto enqueue/import + JobInfo/flatbuffers/codec, job query schema, and gRPC GetJob responses), updates config to support Duration-based default_terminal_retention and retention_scan_interval parsed via duration-str, and introduces retention Prometheus metrics. Manual delete_job is refactored to reuse the same transactional record-deletion helper to keep indexes/counters consistent.

^{Reviewed by Cursor Bugbot for commit b9aafc2. Bugbot is set up for automated code reviews on this repo. Configure here.}

[airhorns]

• add counter metrics for each shard that count how many retention scans have been done and how many jobs have been cleaned up by the retention scanner, document in the metrics docs

[airhorns]

this seems like it duplicates the logic we already have for job deletion, figure out how to DRY this up

[airhorns]

this is still duplicative with the other delete path, which is very risky where we might let the bookkeeping diverge. unify the two paths

[airhorns]

we dont actually need to await this here, its ok, if this doesnt commit the next scan will pick it up

[cursor]

Serde default conflicts with custom deserializer for Duration

High Severity

The default_terminal_retention and retention_scan_interval fields use both #[serde(default = "...")] and #[serde(deserialize_with = "duration_str::deserialize_duration")]. When the field is absent from config, serde uses the default function (returning a Duration), so that works. But when the field is present, duration_str::deserialize_duration parses a string like "7d". The PR reviewer flagged this — the old numeric test defaults (like Duration::from_secs(86400)) work programmatically but a TOML config with a plain number (e.g., default_terminal_retention = 0) would fail deserialization since duration_str expects a string, not a number. This is a breaking change for users who might try numeric values in config.

Additional Locations (1)

• src/settings.rs#L483-L493

 

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b9aafc2. Configure here.}

[cursor]

Duration as_millis cast to i64 silently overflows

Low Severity

default_terminal_retention.as_millis() returns u128, and casting it to i64 with as silently truncates/wraps on overflow. A user-configured retention exceeding ~292 million years would wrap to a negative value, which effective_terminal_retention_ms would then reject. More practically, any Duration exceeding i64::MAX milliseconds (~106 days worth of u128 range beyond i64::MAX) would produce an incorrect retention value without any error.

 

^{Reviewed by Cursor Bugbot for commit b9aafc2. Configure here.}

[cursor]

serialize_duration loses sub-millisecond Duration precision on roundtrip

Low Severity

serialize_duration checks subsec_millis() == 0 to decide between seconds and milliseconds format, but subsec_millis() only returns the millisecond component (0–999) of the sub-second portion. A duration with only microsecond or nanosecond precision (e.g. from a round-trip through duration_str parsing) would have subsec_millis() == 0 yet non-zero sub-second content, causing it to serialize as whole seconds and lose precision. The condition likely intended subsec_nanos() == 0.

 

^{Reviewed by Cursor Bugbot for commit b9aafc2. Configure here.}