Look at the sibling spools again whether or not the last look found a live one

[zaoxing]

Fixes a liveness gap in #163's sibling-spool adoption. The gap matters for the planned multi-rank deployment, where each rank runs an upload-only service.

The defect

adopt_step re-scanned the siblings on adoption_recheck_interval_ns only while its last scan had found a live sibling (storage_service.cpp, recheck_due = live_siblings_ && ...). If the first scan found none, the service never looked again.

A concrete scenario

1. Rank 0's service starts. Its first scan finds no sibling, so live_siblings_ is false.
2. Rank 1 starts later, claims its own rank directory, stages packs, and crashes, or closes without draining.
3. Rank 0 never adopts rank 1's directory while it runs. The packs wait for the next process start on the node.
4. Meanwhile, Spool's charge_dead_siblings keeps charging that directory against rank 0's sink budget. That contradicts spool.h's "the capacity comes back as adoption drains them".

Today this needs a lease takeover to reach. With per-rank services, any rank that starts later than another and then crashes reaches it.

The fix

• Rescan on the recheck interval regardless of what the last look found. A pass lists the parent directory and probes each live sibling's lock without blocking, so the cost is one directory listing per interval (30 s by default).
• live_siblings_ is removed. It had no other reader. The snapshot's live_siblings count is unchanged.
• Comments updated: the config comment, the state comment and spool.h's budget comment now match.

How it was found

The SpoolOwnership TLA+ model on specs/formal-models-archive found it. SpoolOwnership_live_takeover and _live_multi violate adoption liveness. _live_fix, _live_multi_fix and _charge_fix hold once the recheck is unconditional. No specs/ files are added here.

Tests

• New test: test_a_sibling_that_appears_after_start_and_dies_is_adopted in tests/test_native_spool_adoption_live.py. The service's first look finds no sibling. A sibling is then claimed, staged into with the real native pack sink, and released. The test expects it to be adopted, removed, and read back from the catalog.

  • On main it fails, timing out waiting for adoption.
  • With the fix it passes.

• Suites run, against local ClickHouse 25.12 and the fake S3 fixture, with no GPU:

  • tests/test_native_spool_adoption_live.py
  • test_native_spool{,_ownership,_owner_lock,_owner_lock_unit,_reservations}.py
  • test_native_capture_storage_live.py
  • test_native_capture_storage_wiring.py
  • test_native_live_spool.py
  • test_native_sink_release.py
  • test_native_lease_request_bound.py

  All pass, with the sink, store and conformance drivers built.

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused change addresses the liveness gap with targeted regression coverage and no unresolved blocking findings.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes sibling-spool adoption so the storage service discovers directories created after an initial scan found no siblings.

Changes:

• Rechecks siblings periodically regardless of previous scan results.
• Removes unused tracking state and updates adoption comments.
• Adds regression coverage for a late-appearing sibling whose owner exits.

File	Description
tests/​test_native_spool_adoption_live.py	Tests adoption and catalog readback of a late sibling’s packs.
native/​csrc/​store/​spool.h	Clarifies the budget-recovery comment.
native/​csrc/​catalog/​storage_service.h	Updates interval documentation and removes obsolete state.
native/​csrc/​catalog/​storage_service.cpp	Removes the live-sibling prerequisite for periodic scans.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.