[GH-ISSUE #3034] [RFC]: Collision-safe file-per-key paths and legacy migration #778

New issue

Open

opened 2026-10-29 21:19:26 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-29 21:19:26 +00:00

Owner

Originally created by @feichai0017 on GitHub (Jul 21, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3034

Changes proposed

This RFC extends the fix for #2980 and the implementation in #2984. The current file-per-key path uses a sanitized raw key as its filename, so different keys can alias after sanitization or path normalization, and long keys can exceed filesystem filename limits. Backward-compatible migration must also preserve eviction, accounting, and recovery invariants, so the implementation exceeds 500 non-test lines.

Design

Keep the master-managed DISK layout stable. ResolvePathFromKey() retains the existing sanitized-key layout because master replica metadata stores those paths. Only the file-per-key backend uses ResolveFilePerKeyPathFromKey().
Use a bounded canonical file-per-key name. The canonical filename is an XXH3-128 digest split into two directory levels. Loads deserialize and validate the embedded key, so a mismatched record is rejected rather than returned for another key.
Preserve legacy compatibility. Lookups prefer the canonical path and fall back to the old path only after validating the serialized key. Legacy paths that cannot represent a valid key (ENAMETOOLONG, or . / .. resolving to a directory) are treated as absent; other filesystem errors fail closed.
Migrate without losing the authoritative copy. Rewrites are staged in the reserved .staging directory and committed with a same-filesystem rename. Canonical and legacy paths are protected from quota and watermark eviction for the transaction. A matching legacy file is removed only after validation; path locks, FIFO records, and used-space accounting are updated together.
Recover conservatively. Startup removes uncommitted staging files before quota reconstruction. ScanMeta() accepts only the observed canonical or validated legacy layout, rejects symlinks and unexpected entries, and leaves recovery gated after traversal, status, decoding, callback, or cleanup failure.
Commit recovered state once. Scan results, file records, and per-key accounting are staged in temporary structures. Canonical and legacy records for one embedded key are deduplicated regardless of traversal order. Only after traversal, all callbacks, and validated legacy cleanup succeed does recovery reconcile FIFO/map membership, pending eviction state, used space, counters, and readiness.

Implementation split and merge order

#2987 provides the fail-closed ScanMeta() state machine, strict traversal/path binding, exact configured fsdir, and atomic recovery reconciliation.
#2984 adds the digest path, validated legacy fallback, atomic rewrite, legacy retirement, and compatibility accounting on top of that baseline.
Review and merge #2987 first, then #2984. The #2984 branch contains the exact #2987 implementation so both can be tested together; its displayed diff will shrink after #2987 lands.

Ordering and failure contract

After the staged write is complete and closed, a successful same-filesystem rename() makes the canonical entry namespace-visible before the matching legacy shadow is unlinked. This is a namespace-ordering guarantee, not an fsync-backed power-loss durability guarantee.

Before canonical rename, a failure leaves the old canonical/legacy authority intact; an uncommitted staging file is removed immediately or on recovery.
After canonical rename, a legacy unlink failure leaves the canonical copy authoritative. Eviction mode retains physical-byte accounting and disarms the legacy record's master notification; no-eviction mode accounts for both copies. A later scan retries validated cleanup.
A scan traversal, decode, path-binding, callback, or cleanup failure does not publish the staged FIFO/counter/readiness state. Earlier idempotent master callbacks may have succeeded, but the adaptor remains gated and a full retry is required.
A racing compatibility read can still observe a transient miss or I/O error; embedded-key validation prevents returning another key's data.

Compatibility and operational behavior

Existing master-managed DISK replica paths do not change.
Existing file-per-key legacy files remain readable and are migrated lazily on rewrite or reconciled by ScanMeta().
Atomic replacement reserves the full staged object size. If quota cannot provide transient headroom without evicting the canonical or legacy target, the write fails before either target is modified.
The reserved .staging namespace is owned by the file-per-key backend and is cleaned after an interrupted write.
ScanMeta() callbacks must not synchronously re-enter adaptor mutation or watermark-eviction APIs while the exclusive recovery lock is held.

Alternatives considered

Escaping the raw key was rejected because filename length and . / .. normalization remain problematic.
Changing the shared resolver was rejected because it would invalidate existing master DISK replica descriptors.
Eagerly rewriting every legacy file at startup was rejected in favor of validated fallback plus lazy migration, which bounds restart work and keeps mixed-version data readable.

Validation

Exact Ubuntu head 1714b3d88bccae1b1ffbac5bf31d384e512766fa passes:

storage_backend_test: 110/110
master_service_ssd_test: 18/18
utils_test: 20/20
file_storage_test: 23/23
client_storage_backend_test: 3/3

Coverage includes constructed legacy-path collisions, long and dot keys, canonical/legacy traversal, malformed and unexpected entries, callback failure and full retry, crash staging cleanup, legacy-shadow retirement, FIFO/space transfer, tight-quota rollback, exact fsdir handling, and unchanged master DISK descriptor paths.

Out of scope

This RFC does not claim hostile-filesystem containment, protection against ancestor symlink/hard-link/TOCTOU mutation, or a reliable tenant-scoped RemoveAll acknowledgement protocol. Those authority and cleanup concerns are tracked in #3231.

Related: #2980, #2982, #2984, #2987, #3231

Before submitting a new issue...

Searched for relevant issues and read the contribution guidance.

Originally created by @feichai0017 on GitHub (Jul 21, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3034 ## Changes proposed This RFC extends the fix for #2980 and the implementation in #2984. The current file-per-key path uses a sanitized raw key as its filename, so different keys can alias after sanitization or path normalization, and long keys can exceed filesystem filename limits. Backward-compatible migration must also preserve eviction, accounting, and recovery invariants, so the implementation exceeds 500 non-test lines. ### Design 1. **Keep the master-managed DISK layout stable.** `ResolvePathFromKey()` retains the existing sanitized-key layout because master replica metadata stores those paths. Only the file-per-key backend uses `ResolveFilePerKeyPathFromKey()`. 2. **Use a bounded canonical file-per-key name.** The canonical filename is an XXH3-128 digest split into two directory levels. Loads deserialize and validate the embedded key, so a mismatched record is rejected rather than returned for another key. 3. **Preserve legacy compatibility.** Lookups prefer the canonical path and fall back to the old path only after validating the serialized key. Legacy paths that cannot represent a valid key (`ENAMETOOLONG`, or `.` / `..` resolving to a directory) are treated as absent; other filesystem errors fail closed. 4. **Migrate without losing the authoritative copy.** Rewrites are staged in the reserved `.staging` directory and committed with a same-filesystem rename. Canonical and legacy paths are protected from quota and watermark eviction for the transaction. A matching legacy file is removed only after validation; path locks, FIFO records, and used-space accounting are updated together. 5. **Recover conservatively.** Startup removes uncommitted staging files before quota reconstruction. `ScanMeta()` accepts only the observed canonical or validated legacy layout, rejects symlinks and unexpected entries, and leaves recovery gated after traversal, status, decoding, callback, or cleanup failure. 6. **Commit recovered state once.** Scan results, file records, and per-key accounting are staged in temporary structures. Canonical and legacy records for one embedded key are deduplicated regardless of traversal order. Only after traversal, all callbacks, and validated legacy cleanup succeed does recovery reconcile FIFO/map membership, pending eviction state, used space, counters, and readiness. ### Implementation split and merge order - #2987 provides the fail-closed `ScanMeta()` state machine, strict traversal/path binding, exact configured `fsdir`, and atomic recovery reconciliation. - #2984 adds the digest path, validated legacy fallback, atomic rewrite, legacy retirement, and compatibility accounting on top of that baseline. - Review and merge #2987 first, then #2984. The #2984 branch contains the exact #2987 implementation so both can be tested together; its displayed diff will shrink after #2987 lands. ### Ordering and failure contract After the staged write is complete and closed, a successful same-filesystem `rename()` makes the canonical entry namespace-visible before the matching legacy shadow is unlinked. This is a namespace-ordering guarantee, not an `fsync`-backed power-loss durability guarantee. - Before canonical rename, a failure leaves the old canonical/legacy authority intact; an uncommitted staging file is removed immediately or on recovery. - After canonical rename, a legacy unlink failure leaves the canonical copy authoritative. Eviction mode retains physical-byte accounting and disarms the legacy record's master notification; no-eviction mode accounts for both copies. A later scan retries validated cleanup. - A scan traversal, decode, path-binding, callback, or cleanup failure does not publish the staged FIFO/counter/readiness state. Earlier idempotent master callbacks may have succeeded, but the adaptor remains gated and a full retry is required. - A racing compatibility read can still observe a transient miss or I/O error; embedded-key validation prevents returning another key's data. ### Compatibility and operational behavior - Existing master-managed DISK replica paths do not change. - Existing file-per-key legacy files remain readable and are migrated lazily on rewrite or reconciled by `ScanMeta()`. - Atomic replacement reserves the full staged object size. If quota cannot provide transient headroom without evicting the canonical or legacy target, the write fails before either target is modified. - The reserved `.staging` namespace is owned by the file-per-key backend and is cleaned after an interrupted write. - `ScanMeta()` callbacks must not synchronously re-enter adaptor mutation or watermark-eviction APIs while the exclusive recovery lock is held. ### Alternatives considered - Escaping the raw key was rejected because filename length and `.` / `..` normalization remain problematic. - Changing the shared resolver was rejected because it would invalidate existing master DISK replica descriptors. - Eagerly rewriting every legacy file at startup was rejected in favor of validated fallback plus lazy migration, which bounds restart work and keeps mixed-version data readable. ### Validation Exact Ubuntu head `1714b3d88bccae1b1ffbac5bf31d384e512766fa` passes: - `storage_backend_test`: 110/110 - `master_service_ssd_test`: 18/18 - `utils_test`: 20/20 - `file_storage_test`: 23/23 - `client_storage_backend_test`: 3/3 Coverage includes constructed legacy-path collisions, long and dot keys, canonical/legacy traversal, malformed and unexpected entries, callback failure and full retry, crash staging cleanup, legacy-shadow retirement, FIFO/space transfer, tight-quota rollback, exact `fsdir` handling, and unchanged master DISK descriptor paths. ### Out of scope This RFC does not claim hostile-filesystem containment, protection against ancestor symlink/hard-link/TOCTOU mutation, or a reliable tenant-scoped `RemoveAll` acknowledgement protocol. Those authority and cleanup concerns are tracked in #3231. Related: #2980, #2982, #2984, #2987, #3231 ### Before submitting a new issue... - [x] Searched for relevant issues and read the contribution guidance.

bsakaguchi commented

2026-10-29 21:19:27 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Jul 21, 2026):

Thanks for opening this issue, @feichai0017!

Field	Value
Issue	#3034
GitHub user ID	`132118805`
Reporter	@feichai0017

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Jul 21, 2026): Thanks for opening this issue, @feichai0017! | Field | Value | |-------|-------| | **Issue** | #3034 | | **GitHub user ID** | `132118805` | | **Reporter** | @feichai0017 | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:19:27 +00:00

Author

Owner

@feichai0017 commented on GitHub (Aug 2, 2026):

@he-yufeng, I updated the RFC to make the ordering boundary explicit: the staged canonical file is closed and renamed into place before a validated legacy shadow is unlinked. This guarantees namespace ordering, not fsync-backed power-loss durability.

The RFC now also documents the failure matrix and the implementation split: #2987 provides fail-closed scan/reconciliation, while #2984 layers digest paths and legacy migration on top.

Could you confirm whether this direction and the proposed merge order (#2987 first, then #2984) match what you had in mind?

@feichai0017 commented on GitHub (Aug 2, 2026): @he-yufeng, I updated the RFC to make the ordering boundary explicit: the staged canonical file is closed and renamed into place before a validated legacy shadow is unlinked. This guarantees namespace ordering, not `fsync`-backed power-loss durability. The RFC now also documents the failure matrix and the implementation split: #2987 provides fail-closed scan/reconciliation, while #2984 layers digest paths and legacy migration on top. Could you confirm whether this direction and the proposed merge order (#2987 first, then #2984) match what you had in mind?

bsakaguchi referenced this issue

2026-10-29 21:30:35 +00:00

[PR #778] [MERGED] [Store] Check If Get Completed Within Lease #1817