[Store] Self-heal dangling LOCAL_DISK replicas on the read path

[he-yufeng]

Description

A COMPLETE LOCAL_DISK replica whose backing file physically disappears (manual wipe, RemoveAll broadcast, disk loss) stays registered forever, and every batch_get against it just logs "SSD read failed" and hands back an empty buffer. #3711 already self-heals exactly this on the Put path (probe file existence, EvictDiskReplica, clean retry); the read path had no equivalent, which is the still-open half of #3465 and what #3884 tracks.

On a failed SSD read in batch_get_buffer_internal, the same heal now runs: the offload-file probe proves the backing file gone, the dangling replica is evicted master-side via EvictDiskReplica, and the key is retried once so a surviving replica (or a fresh re-put) can serve it instead of returning empty. When nothing survives, the master stops advertising the key and the miss surfaces honestly. The retry only ever heals distinct dead replicas, so it terminates. Checksum failures keep their existing path (the file exists, just corrupt, so no heal fires). Healthy reads are untouched; this is failure-path only.

healDanglingLocalDiskReplica moves from private to public in client_service.h so the RealClient read path can share the exact Put-side logic instead of growing a second copy.

Refs #3884, #3465, #3711.

Module

• Mooncake Store (mooncake-store)

Type of Change

• Bug fix

How Has This Been Tested?

Test commands:

cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4 pybind_client_test
./mooncake-store/tests/pybind_client_test --gtest_filter='*HealsDangling*:*LocalDisk*:*Offload*:*BatchGet*'
./mooncake-store/tests/pybind_client_test --gtest_filter='*HealsDangling*'

Test results:

• Unit tests pass
• Integration tests pass (if applicable)
• Manual testing done (describe below)

Docker (ubuntu 22.04, gcc) with the in-proc master harness: the new regression test BatchGetBufferHealsDanglingLocalDiskReplica (put, wait for the LOCAL_DISK replica, clear MEMORY, wipe the SSD root, then batch_get) fails on the unpatched tree (the dead replica stays advertised) and passes with the fix (honest miss, replica evicted from metadata). Log sequence observed end to end: SSD read failed ... FILE_OPEN_FAIL -> Evicted dangling LOCAL_DISK replica ... (backing file already gone) -> Object not found. The neighboring local-disk / offload / batch-get tests pass (3/3). The full CTest suite was not run locally; it runs in CI.

Checklist

• I have performed a self-review of my own code
• I have formatted my code using ./scripts/code_format.sh
• I have run pre-commit on the files changed in this PR and all hooks pass
• I have updated the documentation (if applicable)
• I have added tests to prove my changes are effective
• For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

• No AI tools were used
• AI tools were used (specify below)

Prepared with AI assistance (Kimi K3). The investigation, implementation, and tests were reviewed line by line, and the verification above was run in Docker as described.

[fcczzz]

I missed this when reviewing #3711: the existence probe receives the raw object key, but offload storage uses tenant-scoped keys, including for the default tenant. A healthy object can therefore be reported as missing.

This PR exposes that existing bug on reads. I reproduced it locally with two disk-only keys, A and B, in separate bucket files on the same owner:

• Delete only A's file and verify that B still reads correctly.
• Call batch_get_buffer({A, B}). The SSD batch fails, and healing runs for both keys.
• B's replica is evicted from master metadata even though its file is byte-for-byte unchanged. Subsequent reads of B fail.

The new counterexample fails on this PR and passes when I remove the added read-healing block. The PR's existing regression test passes in the same build.

Could we fix the probe before extending its use? It needs both the correct tenant-scoped storage key and a physical-file existence check: BucketStorageBackend::IsExist currently checks only the in-memory index, so fixing the key alone would still miss externally deleted files. Please also add a mixed healthy/missing-key regression to ensure a failed batch preserves healthy replicas.

[he-yufeng]

@fcczzz Fixed, and your counterexample is now a regression test. Three changes:

1. The probe now asks with the tenant-scoped key. The install site in RealClient::setup_internal wraps the raw object key with TenantId(tenant_id).MakeScopedKey(...) before calling FileStorage::Exists. One wrap covers every call site of healDanglingLocalDiskReplica, so the Put, BatchPutStart, and batch-get paths are all fixed by the same edit rather than per-caller.

2. BucketStorageBackend::IsExist now verifies the physical file. Index hit plus fs::exists(GetBucketDataPath(bucket_id)); a miss on either side answers false. The ordering invariant makes this safe for fresh writes: BatchOffload commits the bucket file before the index insert, so an index hit can never precede its file. The per-key backend already answered physically (fs::exists on the resolved path), so the two backends now agree.

3. Your mixed-key scenario is the new regression test (BatchGetBufferWipedSiblingPreservesHealthyReplica in pybind_client_test.cpp): A and B land in separate bucket files, A's files are wiped, and batch_get_buffer({A, B}) must evict A's replica, surface a real miss for A, and return B's data with B's replica untouched.

That test exposed one more real bug in my original read-path code, which I'd rather admit than bury: the SSD batch read dies wholesale on A's missing bucket file, so B's read failed too, and since B's replica was (correctly) not evicted, B's result stayed null. Healing preserved the replica but the caller still got a false miss for a healthy key. So healDanglingLocalDiskReplica now returns a 4-state result (kNotDangling / kUnknown / kPresent / kEvicted) instead of a bool, and the batch-get path retries a key when the file is proven present as well as after an eviction. The Put path contract is unchanged (retries only on kEvicted).

Verified in the dev container on this head: both heal tests pass, and the new test fails on the pre-fix code (B's replica evicted, B's data lost), passes with the fix.

[he-yufeng]

Merged current main to clear the conflict (91d8e78). The disk-read section moved to op_status accounting upstream, so the heal retry now records the batch failure first and writes final_results only on a healed read, leaving the duplicate fan-out and handle assembly untouched. Both heal tests re-pass on the merged tree.