kvcache-ai / kvcache-ai/Mooncake
[RFC]: Collision-safe file-per-key paths and legacy migration
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
## 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.
Contributor guide
Research direction
Start with the #2987 ScanMeta() baseline and the #2984 implementation of ResolveFilePerKeyPathFromKey(), then review the existing ResolvePathFromKey() behavior. Run storage_backend_test and file_storage_test first; done requires the listed collision, migration, recovery, quota, and unchanged master-DISK-path cases to pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100