[Store] Preserve ReplicaID during standby restore

[Icedcoco]

Description

Implement PR-I00 standby promotion identity semantics. Restore now preserves descriptor ReplicaID values, rejects invalid or duplicate IDs within one object, and advances the shared Replica::next_id_ high-water mark only after a complete successful restore. Failed restores remain atomic and do not consume IDs.

Module

• Mooncake Store (mooncake-store)

Type of Change

• Bug fix
• Refactor

How Has This Been Tested?

Test commands:

pre-commit run --files mooncake-store/include/replica.h mooncake-store/src/master_service.cpp mooncake-store/tests/ha/master_service_ha_test.cpp

Also compiled the changed production and HA test translation units using the existing CMake compile database.

Test results:

• Focused source compilation passed
• Pre-commit hooks passed
• Full HA test binary run (the current worktree lacks a populated extern/pybind11 submodule, so a clean CMake configure cannot complete)

Checklist

• I have performed a self-review of my own code
• I have formatted my code using the project formatter
• I have run pre-commit on the files changed in this PR and all hooks pass
• I have added tests to prove my changes are effective
• Documentation update is not applicable

AI Assistance Disclosure

• AI tools were used (Codex assisted with implementation, tests, and verification; the human submitter must review every changed line)

[he-yufeng]

Heads up on an overlap you will want to sequence deliberately: #3806 rewrites the validation and construction phases of the same RestoreFromStandbySnapshot for issue #3760 (tolerant per-entry restore with keep-latest overlap resolution), and it lands on the same construction sites this PR edits. One integration point worth deciding before either merges: whether an invalid or duplicate ReplicaID should reject just that object or the whole restore. The tolerant-restore model in #3806 would carry your ID validation naturally as a per-entry rejection with a reason label, keeping the atomicity you want for IDs without reintroducing all-or-nothing for unrelated corruption.

[Icedcoco]

One edge case remains in the current restore path: if a standby snapshot contains ReplicaID 0, UINT64_MAX, or duplicate ReplicaIDs within one object, the pre-scan returns INVALID_PARAMS before any object is committed. Since the supervisor currently treats INVALID_PARAMS as a fatal recovery error, a single malformed object can prevent the promoted primary from serving, and otherwise valid objects in the same snapshot are not restored.

This is known malformed-state behavior and should not occur in normal operation. For I00, we intentionally choose fail-closed over partial recovery: we do not regenerate an ID or arbitrarily select one of the conflicting replicas, because either choice could expose metadata with an incorrect identity. The trade-off is reduced availability, and potentially losing otherwise valid objects from the same malformed snapshot, in exchange for not introducing incorrect data.

A follow-up PR will define object-level/degraded recovery and explicit safe-discard rules, and will narrow the recovery error classification and handling, including the overly broad recovery-failure scope identified in #3774. I00 keeps the current all-or-nothing behavior until that policy is implemented.

[Aionw]

LGTM

[catyans]

issue: UINT64_MAX - 1 still passes the restore pre-scan. It makes desired_next_id equal to UINT64_MAX, so the next normal Replica constructor returns that reserved-invalid ID and wraps next_id_ to 0; the following allocation then returns another invalid ID and can eventually reuse live IDs.

Please make ID exhaustion explicit instead of allowing fetch_add to wrap, and add a boundary test that restores UINT64_MAX - 1 before creating new replicas. The existing test only rejects UINT64_MAX, so it misses the immediate overflow path.

[Icedcoco]

Thanks for catching this edge case. In normal operation, exhausting the full uint64_t ReplicaID space is not a realistic possibility, so we do not plan to add more near-UINT64_MAX boundary restrictions in I00 (for example, rejecting UINT64_MAX - 1, UINT64_MAX - 2, and so on).

We will keep the existing UINT64_MAX check because the current restore high-water update cannot represent max + 1. Handling true ReplicaID allocator exhaustion without wraparound would require a broader change to the normal allocation path and its error handling, which is outside the scope of this PR. We will leave that for a future follow-up if this becomes a practical operational requirement.

[catyans]

Thanks for clarifying. I agree that fully propagating allocator-exhaustion errors may require a broader follow-up, but the restore boundary introduced here should not seed an immediately invalid allocator state. Restoring UINT64_MAX - 1 makes the next constructor emit reserved UINT64_MAX, then wraps next_id_ to 0.

@Aionw @he-yufeng, could you weigh in on whether I00 should at least reject this restored high-water mark, while leaving the broader allocator API redesign to a follow-up?