[Bugfix] Register SSD replicas per owner after client restart

[yyaoling]

Description

Fix a bug where a restarted client's SSD (local-disk) replica registration could be silently dropped, leaving offloaded objects with no disk replica once the old owner is swept.

When a client crashes and restarts, it comes back with a new UUID but the same on-disk files and RPC endpoint. On restart it rescans its SSD objects and re-reports them to the master (via
NotifyOffloadSuccess, or AddReplicaForRetainedClient on the retained-client path). At that moment the old owner's disk replicas may still be registered, because owner cleanup happens
through the heartbeat/liveness lifecycle rather than immediately.

Both registration paths deduplicated existing disk replicas by storage medium (Replica::fn_is_local_disk_replica) — i.e. "does this object already have any local-disk replica?". So while
the old owner's replica was still present, the new owner's report was treated as already-existing and effectively discarded. Once the old owner was later swept, the object was left with no disk
replica at all, even though the new client had already reported it — forcing another rescan (or a memory-replica fallback) to recover.

Fix

Deduplicate by owner (client_id) instead of by medium, in both paths (master_service.cpp):

• AddReplicaForRetainedClient
• NotifyOffloadSuccess

A disk replica is now treated as "already registered" only when the same owner already has one:

• Repeated rescan batches from the same owner stay idempotent (the descriptor's endpoint/size is refreshed in place).
• Replicas from different owners coexist until the old owner is removed by its normal lifecycle cleanup.
• The restarted owner's registration is no longer lost.

A key or RPC endpoint cannot prove that two owners share a disk, so owner identity (client_id) is the only safe dedup key.

Module

• Transfer Engine (mooncake-transfer-engine)
• Mooncake Store (mooncake-store)
• Mooncake Conductor (mooncake-conductor)
• Reshard (mooncake-reshard)
• Mooncake EP (mooncake-ep)
• Mooncake PG (mooncake-pg)
• Integration (mooncake-integration)
• P2P Store (mooncake-p2p-store)
• Python Wheel (mooncake-wheel)
• Common (mooncake-common)
• Mooncake RL (mooncake-rl)
• CI/CD
• Docs
• Other

Type of Change

• Bug fix
• New feature
• Refactor
• Breaking change
• Documentation update
• Performance improvement
• Other

How Has This Been Tested?

Test commands:

# Example: bash scripts/run_ci_test.sh

Test results:

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

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)

  Claude Code (Opus 4.8) and gpt-6-astra helped analyze the offload/registration code paths, articulate the root cause, and draft the two regression tests and this description. The human submitter has reviewed
  every changed line and can defend the change end-to-end.

[he-yufeng]

Mechanism review of the three changed files (static, line level; no local build, per my review scope):

The defect is real and the fix is shaped correctly. Both call sites deduplicated by medium: AddReplicaForRetainedClient treated any existing local-disk replica as "replacing existing", and NotifyOffloadSuccess skipped registration whenever a disk replica was present. A restarted client with a fresh UUID rescanning its SSD objects therefore had a perfectly good registration silently dropped, and the master kept advertising only the dead owner's copy. Deduplicating by owner is the right predicate, and the inline comment says why (a key or an endpoint does not prove two owners share a disk).

The quota change is consistent with it, and arguably the more important half: LocalSsdManager tracks used_bytes per owner and feeds ssd_free_ratio_first placement, so charging the second owner for its own physical copy keeps placement accounting honest instead of misreporting its disk as empty. The scenario test pins both registration branches plus the unmount sweep, and the quota test now asserts both replicas stay visible.

One window worth a thought, not a blocker: between the new owner's registration and the old owner's lifecycle sweep, the key advertises two COMPLETE disk replicas. If a reader gets the stale owner's endpoint first, it pays an SSD read failure before finding the live one (or returns empty if that was the only advertised copy it tried). That is exactly the read-side dangling-replica gap #3889 heals with the probe + evict + retry path, so the two changes compose: yours stops the false refusal at registration time, #3889 cleans up the stale-read window until the sweeper runs. Does the read path today order or filter COMPLETE disk replicas by owner liveness, or is it first-come?

Boundary note for the queue: this does not overlap #3806 (standby index restore after a master restart); different layer, no conflict either way.

[yyaoling]

Mechanism review of the three changed files (static, line level; no local build, per my review scope):

The defect is real and the fix is shaped correctly. Both call sites deduplicated by medium: AddReplicaForRetainedClient treated any existing local-disk replica as "replacing existing", and NotifyOffloadSuccess skipped registration whenever a disk replica was present. A restarted client with a fresh UUID rescanning its SSD objects therefore had a perfectly good registration silently dropped, and the master kept advertising only the dead owner's copy. Deduplicating by owner is the right predicate, and the inline comment says why (a key or an endpoint does not prove two owners share a disk).

The quota change is consistent with it, and arguably the more important half: LocalSsdManager tracks used_bytes per owner and feeds ssd_free_ratio_first placement, so charging the second owner for its own physical copy keeps placement accounting honest instead of misreporting its disk as empty. The scenario test pins both registration branches plus the unmount sweep, and the quota test now asserts both replicas stay visible.

One window worth a thought, not a blocker: between the new owner's registration and the old owner's lifecycle sweep, the key advertises two COMPLETE disk replicas. If a reader gets the stale owner's endpoint first, it pays an SSD read failure before finding the live one (or returns empty if that was the only advertised copy it tried). That is exactly the read-side dangling-replica gap #3889 heals with the probe + evict + retry path, so the two changes compose: yours stops the false refusal at registration time, #3889 cleans up the stale-read window until the sweeper runs. Does the read path today order or filter COMPLETE disk replicas by owner liveness, or is it first-come?

Boundary note for the queue: this does not overlap #3806 (standby index restore after a master restart); different layer, no conflict either way.

[yyaoling]

Mechanism review of the three changed files (static, line level; no local build, per my review scope):

The defect is real and the fix is shaped correctly. Both call sites deduplicated by medium: AddReplicaForRetainedClient treated any existing local-disk replica as "replacing existing", and NotifyOffloadSuccess skipped registration whenever a disk replica was present. A restarted client with a fresh UUID rescanning its SSD objects therefore had a perfectly good registration silently dropped, and the master kept advertising only the dead owner's copy. Deduplicating by owner is the right predicate, and the inline comment says why (a key or an endpoint does not prove two owners share a disk).

The quota change is consistent with it, and arguably the more important half: LocalSsdManager tracks used_bytes per owner and feeds ssd_free_ratio_first placement, so charging the second owner for its own physical copy keeps placement accounting honest instead of misreporting its disk as empty. The scenario test pins both registration branches plus the unmount sweep, and the quota test now asserts both replicas stay visible.

One window worth a thought, not a blocker: between the new owner's registration and the old owner's lifecycle sweep, the key advertises two COMPLETE disk replicas. If a reader gets the stale owner's endpoint first, it pays an SSD read failure before finding the live one (or returns empty if that was the only advertised copy it tried). That is exactly the read-side dangling-replica gap #3889 heals with the probe + evict + retry path, so the two changes compose: yours stops the false refusal at registration time, #3889 cleans up the stale-read window until the sweeper runs. Does the read path today order or filter COMPLETE disk replicas by owner liveness, or is it first-come?

Boundary note for the queue: this does not overlap #3806 (standby index restore after a master restart); different layer, no conflict either way.

This bug arises because the primary key for local_disk within the replica context is subject to change. Potential solutions include: 1. Deduplicating and updating local_disk_replica entries based on the endpoint; 2. Persisting the client_id and using it for reporting after data flushing (though this disrupts the existing design model, which assumes that memory and disk states cannot be distinguished by the same client_id); or 3. Scanning disk data upon startup and using a dedicated master interface to update replica data, rather than reusing the standard interface intended for data flushing notifications.
@he-yufeng

[he-yufeng]

On the three directions: the endpoint-keyed dedup (1) collapses exactly the case your own inline comment calls out — after a disk swap or a remount on the same box, two owners legitimately share one endpoint, and dedup by endpoint would again drop the live registration, this time with a false positive instead of a false negative. The dedicated rescan interface (3) is the cleanest long-term shape (registration and flush notification are different verbs), but it is a new master surface with its own auth and lifecycle story. Your per-owner dedup is the only one of the three that lands without either of those costs, which is why it reads as the right first step rather than a compromise.

The stale-read window is orthogonal to all three: whatever stops the false refusal at registration, a dead owner's COMPLETE replica still advertises until the sweeper runs. That stays true under endpoint dedup or a rescan interface too, so the read-side heal in #3889 composes with whichever direction the queue settles on.

[yyaoling]

On the three directions: the endpoint-keyed dedup (1) collapses exactly the case your own inline comment calls out — after a disk swap or a remount on the same box, two owners legitimately share one endpoint, and dedup by endpoint would again drop the live registration, this time with a false positive instead of a false negative. The dedicated rescan interface (3) is the cleanest long-term shape (registration and flush notification are different verbs), but it is a new master surface with its own auth and lifecycle story. Your per-owner dedup is the only one of the three that lands without either of those costs, which is why it reads as the right first step rather than a compromise.

The stale-read window is orthogonal to all three: whatever stops the false refusal at registration, a dead owner's COMPLETE replica still advertises until the sweeper runs. That stays true under endpoint dedup or a rescan interface too, so the read-side heal in #3889 composes with whichever direction the queue settles on.

On the three directions: the endpoint-keyed dedup (1) collapses exactly the case your own inline comment calls out — after a disk swap or a remount on the same box, two owners legitimately share one endpoint, and dedup by endpoint would again drop the live registration, this time with a false positive instead of a false negative. The dedicated rescan interface (3) is the cleanest long-term shape (registration and flush notification are different verbs), but it is a new master surface with its own auth and lifecycle story. Your per-owner dedup is the only one of the three that lands without either of those costs, which is why it reads as the right first step rather than a compromise.

The stale-read window is orthogonal to all three: whatever stops the false refusal at registration, a dead owner's COMPLETE replica still advertises until the sweeper runs. That stays true under endpoint dedup or a rescan interface too, so the read-side heal in #3889 composes with whichever direction the queue settles on.

The current read path already filters LOCAL_DISK replicas by owner liveness: Replica::getDescriptorIfAvailable() checks the bound ClientLivenessRecord::IsServing(). Therefore, an old owner’s replica can stop being returned before the metadata sweep removes it. Also, in the restart scenario covered here, the endpoint and backing files remain unchanged. Even if the old owner’s descriptor is returned before failure detection, the SSD read can still succeed: the read RPC uses the endpoint, keys, and sizes, without validating the descriptor’s owner UUID against the serving process. The actual bug is that the new owner’s registration was dropped. Once the old owner becomes non-serving or is swept, the master loses the usable replica despite the files still being available. This PR preserves the new owner’s registration so it survives that cleanup. I’ll keep this PR scoped to the registration fix.

I’ll keep #4130 scoped to preserving the new owner’s registration. Existing liveness filtering and lifecycle cleanup already handle the old owner. If we want to retire stale owner records earlier during restart takeover, we can discuss that in a separate PR. That is distinct from #3889, which handles dangling replicas whose backing files are confirmed missing. @he-yufeng