Check the lease before a cycle's first chunk too, not only the later ones

[zaoxing]

Closes a gap in decision 7 (#150: no uploads while the service cannot index). The first chunk of every upload cycle went out without a fresh lease check.

What was wrong

run_cycle checks the publisher lease at the top of the cycle (ensure_publisher_lease()), then calls spool_.ListPending, which hashes every staged pack. Over a backlog that takes seconds; it took about 1 s per GiB here. Before each chunk the loop re-checked the lease under LeaseScope, but only when next != 0. The first chunk relied on the check made before the listing.

The interleaving:

1. A loop cycle takes the lease, and nothing is owed.
2. ListPending starts hashing the backlog.
3. Meanwhile the lease thread's renewal fails with an unknown outcome. The writer quarantines and drops the lease.
4. The listing returns. With next == 0 no check runs, and upload_chunk uploads up to indexer.max_packs packs (64 by default). The uploader then removes them from the spool.
5. Indexing fails without a lease, so the chunk is owed in pending_index_. For the quarantine no cycle has the catalog, so those packs live only in memory. A crash with reconcile_on_start=False leaves them in the bucket and never in the catalog.

That is the orphaning #150 forbids, and storage_service.h claimed the opposite ("A cycle checks the lease before each chunk it uploads").

The fix

The LeaseScope + writer_.held_lease() check now runs before every chunk, the first included, so it comes after the listing. It sends no request. The owed and cancel checks stay where they were, for later chunks only. The header comment now says the first chunk is checked after the listing.

A lease lost while a chunk is in flight is still possible. That is the window the header already documents, at most one chunk.

Tests

• New: test_a_lease_lost_while_the_spool_is_listed_uploads_nothing in tests/test_native_capture_storage_live.py (manual, needs ClickHouse). It uses a sparse backlog of 64 × 128 MiB, which takes the listing about 8 s. The catalog is cut while the first cycle hashes, and the test asserts the lease quarantined before the listing ended. On main the cycle then uploads one pack: it fails with uploaded_packs == 1. With the fix it uploads nothing and every pack stays staged.
• Run against a local ClickHouse after building cpu-goals:
  • test_native_capture_storage_live.py, test_native_lease_request_bound.py, test_native_catalog_lease_live.py, test_native_capture_storage_wiring.py and the spool, ownership, adoption and sink-release suites all pass.
  • test_native_spool_adoption_live.py::test_a_flush_does_not_wait_for_an_adoption_to_list_a_dead_backlog timed out once under load. It passed in isolation and in three full re-runs of its file, and its path doesn't go through the changed loop.
• pytest -m cpu: 2660 passed, 1 skipped.

How it was found

The PackPipeline TLA+ model on the specs/formal-models-archive branch found it (specs/tla/PackPipeline_firstchunk.cfg violates ChunkStartsWithLease). The same check with the first chunk checked, PackPipeline_firstchunk_fixed.cfg, holds. The specs are not part of this PR.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The newly detected lease loss is not propagated to catalog, allowing reconciliation to run without a lease.

Review effort: Balanced
Findings: None

What changed in this PR

Ensures every upload chunk receives a fresh publisher-lease check after spool listing.

Changes:

• Checks the lease before the first chunk.
• Adds a live regression test.
• Clarifies lease behavior documentation.

File	Description
native/​csrc/​catalog/​storage_service.cpp	Moves lease validation outside the later-chunk condition.
native/​csrc/​catalog/​storage_service.h	Documents first-chunk validation timing.
tests/​test_native_capture_storage_live.py	Tests lease loss during slow spool listing.

---

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