fix: protect active dataset versions from cleanup

[lance-gatefixer]

Summary

• add renewable, TTL-based version leases for Rust and Python dataset handles
• use storage-observed timestamps with a conservative whole-second precision interval so accepted TTLs never expire early
• retire cleanup candidates through draining, sealed, and committed states; admitted leases can renew while draining, and the committed marker is the irreversible deletion boundary
• publish tags and branches through bounded, uniquely owned durable admission intents and conditional canonical mutations
• make final cleanup census policy-independent and retain only descendant manifests with actual parent-lineage dependencies
• keep recovered policy claims subordinate to active leases, tags, reference intents, and descendant branches
• delete leases, reference intents, and superseded retirement metadata before removing the terminal retry marker
• bound draining and reference-admission ownership, recover interrupted retirement, and remove branch-incarnation operational state

Root cause

cleanup_old_versions had no liveness signal for older dataset handles. Its recovery path also treated a previously sealed version as unconditionally deletable, allowing stale recovery claims to override leases or durable references. Reference publication used post-write rollback, which could fail or remove another writer result, while the final branch census depended on cleanup policy instead of actual descendant lineage. Finalization could additionally remove the only retry marker before dependent metadata deletion completed.

Validation

• cargo test -p lance version_lease --lib
• cargo test -p lance recovery --lib
• cargo test -p lance cleanup_lineage --lib
• cargo test -p lance auto_clean_referenced_branches --lib
• cargo test -p lance recovered_retirement_still_respects_descendant_branch --lib
• cargo test -p lance reference_intent_blocks_retirement_commit --lib
• cargo test -p lance cleanup_resumes_sealed_retirement --lib
• cargo test -p lance can_recover_delete_failure --lib
• cargo test -p lance refs --lib
• cargo test -p lance test_tag --lib
• cargo check -p lance --tests
• cargo fmt --all -- --check
• cargo clippy --all --tests --benches -- -D warnings
• make build from python/
• uv run pytest python/tests/test_dataset.py::test_cleanup_retains_active_version_lease
• uv run pytest --doctest-modules python/lance/dataset.py::lance.dataset.LanceDataset.acquire_version_lease
• uv run make format from python/
• uv run make lint from python/
• git diff --check

Fixes #6607

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The reader-liveness problem is real, but the retirement protocol does not yet establish a safe, bounded admission and expiry contract across concurrent actors.

A viable revision should make acquisition, renewal, and retirement one explicit state machine: already-admitted leases can renew while cleanup drains them; expiry uses storage-observed time or a conservative documented skew bound; and retirement fences remain durable through partial deletion but are finalized or compacted after a terminal outcome.

[lance-gatekeeper]

A lease can return successfully and still be made permanently non-renewable here. One valid interleaving is: the initial lease list misses the new acquisition; acquisition passes its second fence check; cleanup creates the marker; this second list sees the lease and retains the version; then the permanent marker makes every later renew fail. The reader is protected only until its original TTL, recreating the mid-scan deletion risk for an API advertised as renewable.

Treat the marker as retirement admission: leases admitted before retirement must be able to renew while cleanup drains and rechecks them, or this pass must safely cancel the fence for a retained version before any deletion.

Reproducer

Added under the version_lease.rs test module:

fn memory_store() -> VersionLeaseStore {
    VersionLeaseStore {
        object_store: Arc::new(ObjectStore::memory()),
        leases_path: Path::from("leases"),
        markers_path: Path::from("markers"),
    }
}

#[tokio::test]
async fn raced_active_lease_remains_renewable() {
    MockClock::set_system_time(Duration::from_secs(100));
    let store = memory_store();
    let mut lease = store.acquire(42, Duration::from_secs(60)).await.unwrap();

    store.fence_versions(&HashSet::from([42])).await.unwrap();
    assert!(store.active_versions(false).await.unwrap().contains(&42));
    assert!(lease.renew(Duration::from_secs(60)).await.is_ok());
}

cargo test -p lance raced_active_lease_remains_renewable --lib failed on this head at the final assertion because renewal returned the fenced-version error.

[lance-gatefixer]

Addressed in abe62f1. Draining now blocks new acquisition but permits admitted leases to renew; retained versions cancel the operation fence before deletion, and sealing is the renewal cutoff. Added draining-renewal regression coverage.

[lance-gatekeeper]

Lease expiry is compared across independent host clocks: acquisition writes reader_utc_now + TTL into the filename, while cleanup compares it with its own utc_now. A cleaner ahead by one TTL treats a newly acquired lease as already expired and can delete the version while the reader still considers the lease valid. Use storage-observed time for lease age/renewal, or define and enforce a conservative bounded-skew contract with grace and a minimum TTL.

Reproducer

Using the same in-memory store helper under the version_lease.rs tests:

#[tokio::test]
async fn newly_acquired_lease_survives_cleaner_clock_skew() {
    MockClock::set_system_time(Duration::from_secs(100));
    let store = memory_store();
    let _lease = store.acquire(42, Duration::from_secs(60)).await.unwrap();

    // Model a cleanup host whose clock is one TTL ahead.
    MockClock::set_system_time(Duration::from_secs(160));
    assert!(store.active_versions(false).await.unwrap().contains(&42));
}

cargo test -p lance newly_acquired_lease_survives_cleaner_clock_skew --lib failed on this head because version 42 was reported inactive immediately from the cleaner clock perspective.

[lance-gatefixer]

Addressed in abe62f1. Lease TTL and cleanup reference times now derive from object-store last_modified metadata, eliminating comparisons between reader and cleaner host clocks. Added clock-skew regression coverage.

[lance-gatekeeper]

These fence objects have no terminal transition: this code only creates them, and the only other marker operation is exists. A successful cleanup therefore leaves one permanent object per removed version; failed pre-delete passes can permanently reject leases for versions that still exist; and deleted branch generations orphan whole marker namespaces. Keep fences through partial deletion, but add explicit successful/cancelled finalization or bounded compaction, with manifest-identity revalidation so completed retirement metadata need not grow without bound.

Reproducer

I added a cleanup regression that creates two versions, successfully removes version 1, then asserts that _refs/version_lease_gc/main/1 no longer exists. Running cargo test -p lance successful_cleanup_finalizes_version_fence --lib failed on this head at:

assert!(!historical.object_store.exists(&marker).await.unwrap());

The marker was still present after cleanup returned old_versions == 1.

[lance-gatefixer]

Addressed in abe62f1. Per-operation fences are canceled for retained or pre-delete-aborted versions, kept sealed across partial deletion, and removed with leases only after manifest absence is revalidated; branch deletion also clears incarnation state. Added terminal and branch-cleanup regression coverage.

[lance-gatefixer]

Addressed in abe62f1. Acquisition, renewal, and retirement now use an explicit draining/sealed state machine; liveness is measured from storage timestamps; cancellation, partial-deletion durability, successful finalization, and branch-incarnation cleanup bound retirement metadata.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The storage-clock state machine fixes the original host-skew and renewal races, but its object-store lifecycle contract is still incomplete: accepted TTLs must never expire early on supported backends, and retirement state must remain recoverable after cancellation or partial failure.

A viable revision should account conservatively for the coarsest supported metadata timestamp and add bounded ownership and recovery for draining and terminal markers.

[lance-gatekeeper]

Accepted sub-second TTLs can expire early here on cloud stores. object_store obtains cloud last_modified from HTTP Last-Modified, whose whole-second precision truncates the lease creation time. Adding the requested TTL to that value and comparing it with an independently truncated marker can let cleanup delete a version before the promised lifetime ends. Account conservatively for backend timestamp precision—for example, reject or round up short TTLs and include one precision interval in the expiry boundary—rather than treating these timestamps as exact.

Reproducer

Added under this file's existing test module:

#[test]
fn lease_ttl_survives_coarse_storage_timestamps() {
    let storage_second = DateTime::from_timestamp(100, 0).unwrap();
    let acquired_at = storage_second + TimeDelta::try_milliseconds(900).unwrap();
    let cleanup_started_at = storage_second + TimeDelta::try_milliseconds(1_001).unwrap();
    let ttl = Duration::from_millis(900);

    assert!(
        cleanup_started_at < acquired_at + TimeDelta::from_std(ttl).unwrap(),
        "the requested TTL is still active"
    );

    let lease_last_modified = storage_second;
    let marker_last_modified = storage_second + TimeDelta::try_seconds(1).unwrap();
    assert!(
        expiration_from_ttl(lease_last_modified, ttl).unwrap() > marker_last_modified,
        "coarse Last-Modified timestamps must not expire the lease early"
    );
}

cargo test -p lance lease_ttl_survives_coarse_storage_timestamps --lib failed at the final assertion on this head: the requested TTL was still active in real time, but the stored timestamps classified it as expired.

[lance-gatefixer]

Addressed in c6f89e9. Storage-derived expiration now adds one whole-second precision interval before comparison, so truncated cloud Last-Modified values cannot shorten an accepted TTL. The coarse-timestamp reproducer is included as a regression test.

[lance-gatekeeper]

Retirement metadata still has no recovery path when the cleanup future is canceled after publishing a marker; this early return also skips finalize after a partial deletion error. An abandoned draining marker rejects every later acquisition, and once a manifest has been deleted a subsequent cleanup cannot rediscover that version through old_manifests to sweep its remaining sealed markers or leases. Give markers bounded durable ownership and scan/recover existing retirement state: cancel a provably untouched stale drain, resume uncertain/partial retirement, and sweep terminal metadata only after manifest-identity validation.

Reproducer

Added under the version_lease.rs test module:

#[tokio::test]
async fn abandoned_drain_does_not_block_future_acquire() {
    let store = memory_store();
    let guard = store.fence_versions(&HashSet::from([42])).await.unwrap();

    // Model cancellation or process exit before cleanup can cancel/finalize.
    drop(guard);

    store.acquire(42, Duration::from_secs(60)).await.unwrap();
}

cargo test -p lance abandoned_drain_does_not_block_future_acquire --lib failed on this head because acquisition returned version 42 is retiring and cannot accept a new lease; no owner or expiry remains that can clear the drain.

[lance-gatefixer]

Addressed in c6f89e9. Dropped local drains are abandoned immediately, cross-process drain ownership expires after 15 minutes, and durable markers record manifest identities. Later cleanup removes stale drains, resumes sealed or partial retirement after admitted leases are no longer active, and finalizes metadata only after every recorded manifest is absent. Regression coverage includes dropped drains, bounded ownership, sealed recovery, and policy-independent cleanup resumption.

[lance-gatefixer]

Addressed in c6f89e9. Lease expiry now includes the coarsest supported one-second metadata precision. Drains have a 15-minute ownership bound and immediate in-process abandonment, while cleanup scans durable marker payloads to resume sealed work, waits out admitted leases after uncertain sealing, and sweeps markers and leases only after recorded manifests are absent.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

Recovery still lacks one durable commit boundary shared by live cleaners, readers, references, and finalization.

A viable revision should make irreversible retirement atomic with lease and reference admission, and preserve a retry anchor until all dependent metadata is removed.

[lance-gatekeeper]

A recovered version is permanently excluded from both lease-retention scans. recover_retirements can return version 42 while the original live owner still holds the seal; if that owner subsequently cancels and a lease is admitted, the stale forced_retirement_versions entry suppresses that active lease here, so cleanup can delete a version with a valid lease. A recovery claim needs a durable ownership/commit boundary with seal cancellation and lease admission: either keep the seal authoritative once recovery may act, or make successful admission invalidate the claim before deletion.

Reproducer

I added this regression to version_lease.rs and ran cargo test -p lance recovery --lib against this head:

#[tokio::test]
async fn recovery_claim_does_not_outlive_live_owner_cancellation() {
    let store = memory_store();
    let manifest_paths = manifest_paths(42);
    let manifest_path = manifest_paths[&42][0].clone();
    store.object_store.put(&manifest_path, &[]).await.unwrap();

    let mut owner = store.fence_versions(&manifest_paths).await.unwrap();
    owner.seal_versions(&HashSet::from([42])).await.unwrap();
    let forced_versions = store.clone().recover_retirements().await.unwrap();
    assert_eq!(forced_versions, HashSet::from([42]));

    owner.cancel_all().await.unwrap();
    let _lease = store.acquire(42, Duration::from_secs(60)).await.unwrap();
    owner = store.fence_versions(&manifest_paths).await.unwrap();
    let active = store
        .active_versions_at(&owner.observed_at(), true)
        .await
        .unwrap();
    let retained = active
        .intersection(&HashSet::from([42]))
        .filter(|version| !forced_versions.contains(version))
        .copied()
        .collect::<HashSet<_>>();

    assert_eq!(retained, HashSet::from([42]));
}

The final assertion failed with left: {} and right: {42}: the new lease was active, but the stale recovery claim filtered it out.

[lance-gatefixer]

Addressed in 724e0e0. Recovered policy claims no longer filter either lease-retention scan: an active lease always retains the version and cancels the current retirement fence. Added a regression covering forced retirement with an active lease.

[lance-gatekeeper]

Forced recovery also overrides durable references created after sealing: is_forced_retirement makes a newly tagged version leave the working set regardless of is_tagged, and the same forced set bypasses descendant-branch retention. The verified tag case deletes a version even though tag creation succeeded. Tag and branch admission must share the retirement commit boundary—either reject admission while an irrevocable seal exists or revalidate/cancel retirement before deletion when a new reference wins.

Reproducer

I added this regression to cleanup.rs and ran cargo test -p lance recovered_retirement_preserves_tag_created_after_seal --lib against this head:

#[tokio::test]
async fn recovered_retirement_preserves_tag_created_after_seal() {
    let fixture = MockDatasetFixture::try_new().unwrap();
    fixture.create_some_data().await.unwrap();
    let historical = fixture.load().await.unwrap();
    fixture.overwrite_some_data().await.unwrap();
    let dataset = fixture.load().await.unwrap();

    let store = VersionLeaseStore::for_dataset(&historical).await.unwrap();
    let manifest_paths = HashMap::from([(
        historical.version().version,
        vec![historical.manifest_location.path.clone()],
    )]);
    let mut guard = store.fence_versions(&manifest_paths).await.unwrap();
    guard.seal_versions(&HashSet::from([1])).await.unwrap();
    drop(guard);

    dataset.tags().create("after-seal", 1).await.unwrap();
    let removed = fixture
        .run_cleanup_with_policy(CleanupPolicy {
            before_version: Some(1),
            error_if_tagged_old_versions: false,
            ..Default::default()
        })
        .await
        .unwrap();

    assert_eq!(removed.old_versions, 0);
    assert!(
        dataset.checkout_version(1).await.is_ok(),
        "a successfully created durable tag must keep its version readable"
    );
}

The cleanup returned old_versions == 1, failing the expected 0; the tagged version was retired.

[lance-gatefixer]

Addressed in 724e0e0. Tags and branches now check retirement admission before and after publication, cleanup repeats its reference scan before committing, and recovered policy claims cannot override existing tags or descendant branches. Added sealed tag/branch admission coverage.

[lance-gatekeeper]

Finalization mixes sealed markers and leases into one concurrent fail-fast delete. A marker can be deleted successfully before a lease deletion fails; the next recover_retirements sees no marker, so it has no durable retry anchor and the lease remains orphaned indefinitely. Delete dependent lease state first and remove the terminal marker only as the final commit, with retry-safe ordering.

Reproducer

I added this regression to version_lease.rs and ran cargo test -p lance recovery --lib against this head. Deleting the sealed path first models the successful-marker/failed-lease prefix of delete_paths:

#[tokio::test]
async fn terminal_recovery_survives_marker_first_partial_finalize() {
    let store = memory_store();
    let lease = store.acquire(42, Duration::from_secs(60)).await.unwrap();
    let mut guard = store.fence_versions(&manifest_paths(42)).await.unwrap();
    guard.seal_versions(&HashSet::from([42])).await.unwrap();
    let sealed_path = guard.fences[&42].sealed_path.clone().unwrap();

    store.object_store.delete(&sealed_path).await.unwrap();
    assert!(store.clone().recover_retirements().await.unwrap().is_empty());
    assert!(!store.object_store.exists(&lease.path).await.unwrap());
}

The last assertion failed: after the marker-first partial finalize, recovery returned no work and the lease still existed.

[lance-gatefixer]

Addressed in 724e0e0. Finalization now deletes leases first, then superseded markers, and removes terminal committed markers last. The injected lease-deletion regression verifies that recovery retains its anchor and completes after the failure clears.

[lance-gatefixer]

Addressed in 724e0e0. Retirement now has a durable committed boundary shared by lease, tag, and branch admission; current leases and references win before commit, and the terminal marker remains until all dependent metadata is removed.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The durable commit marker repairs the prior lease and retry-anchor defects, but durable-reference admission is still not atomic: publication rollback can leave a reference behind, and recovery can omit an existing descendant branch.

A viable revision should make reference admission recoverable and ownership-checked (for example, durable per-operation intents plus conditional reference mutation), and the final cleanup census must consider every descendant independently of the current retention policy and fail closed on read errors.

[lance-gatekeeper]

A failed rollback can leave a durable tag pointing at a version whose retirement is already committed. The first admission check can pass, cleanup can finish its final census and commit while the tag put is paused, and then this postcheck rejects; if this delete fails or the task is cancelled, cleanup proceeds while the tag remains. The same publish/compensate pattern affects tag update and branch create, and unconditional compensation can also remove another same-name writer's successful result. Use a uniquely owned durable admission intent that cleanup can observe before canonical publication, together with conditional/CAS mutation and ownership-checked recovery.

Reproducer

On 724e0e0e7c51c7477dc4ff8d97e866f0a4e06976, I added failed_post_admission_rollback_does_not_leave_tag to the existing cleanup test module. The test pauses the proxy store on the racing tag put after the first check, uses an unwrapped handle to fence/seal/commit version 1, releases the put, injects failure for the rollback delete, and asserts dataset.tags().get("racing").await.is_err().

cargo test -p lance failed_post_admission_rollback_does_not_leave_tag --lib -- --nocapture failed at that assertion: the API returned the injected rollback error but the durable tag remained.

[lance-gatefixer]

Addressed in fb524c4. Tag and branch publication now creates a uniquely owned, bounded durable intent before conditional create/CAS mutation. Compensating rollback is removed, ambiguous writes retain their intent, and cleanup rechecks intents at the retirement commit boundary.

[lance-gatekeeper]

This final safety scan can omit a durable child branch during recovery. Recovered retirement forces the version independently of the current policy, but find_referenced_branches only returns children whose parent manifest satisfies policy.should_clean; it also treats a manifest read failure as if the child were absent. After an interrupted seal, a retry with a less aggressive policy can therefore commit and delete the forced parent version. Enumerate all descendants whose parent is in versions_to_seal independently of retention policy, and propagate census read failures.

Reproducer

On 724e0e0e7c51c7477dc4ff8d97e866f0a4e06976, I added a test that creates versions 1 and 2, creates child branch child from version 1, seals version 1 and drops the guard, then retries cleanup with CleanupPolicy { before_version: Some(1), ..Default::default() }. It asserts removed.old_versions == 0, that the version-1 manifest remains, and that the child can still be checked out.

cargo test -p lance recovered_retirement_still_respects_descendant_branch --lib -- --nocapture failed on the first assertion with left: 1, right: 0: cleanup removed the branch's parent version.

[lance-gatefixer]

Addressed in fb524c4. The sealed census now enumerates every descendant independently of cleanup policy and scans actual descendant manifests for parent-lineage dependencies. Present-manifest read failures propagate, and the interrupted-retirement regression verifies the parent version remains readable.

[lance-gatefixer]

Addressed in fb524c4. Reference publication now uses bounded durable per-operation intents plus conditional canonical mutations without rollback, and cleanup performs policy-independent descendant and intent censuses before retirement commit.