docs(architecture): correct erasure-coding spec statements that contradict main (#5012)

The normative erasure-coding spec stated several target behaviors and known baseline defects as active invariants, and cited two symbols that do not exist on main. Each correction below was verified against the code.

- §4.1 shard size: legacy even-padding is RustFS-legacy-only and is NOT MinIO's sizing. MinIO (and the modern GF(2⁸) path) use plain div_ceil; for 1 MiB / 6 data shards that is 174763 bytes versus the legacy even-padded 174764. MinIO-migrated data is decoded by the modern path, so the legacy formula never applies to it.
- §6.3 / §11 MTime: the MTime key is always written (a None mod_time encodes as UNIX_EPOCH = 0), not omitted. Round-trip safety to None is enforced on the decode side, not by write omission. Only the legacy StatInfo.ModTime field is omitted-when-None.
- §7 commit: rollback after a missed write quorum is best-effort — undo failures are counted and warn!-logged, never propagated or retried — so a partially-failed rollback can leave shards behind; softened "never left partially committed" accordingly.
- §7 data_dir: reduce_common_data_dir votes over each disk's old_data_dir (a GC input for reclaiming the superseded dir), not the newly committed data_dir.
- §7 convergence: classify_rename_convergence is consumed only by the multipart-complete path (convergence.needs_heal()); the regular put_object path discards it.
- §7 write layout: removed the WriteLayout / resolve_write_layout citation — neither symbol exists on main (the write layout is computed inline), which violated the spec's own cite-real-symbols rule.
- §6.6 inline: inline presence is read from the meta_sys[inline-data] body marker alone; the header InlineData flag is written but not consulted on read, and disagreement is tolerated (MinIO may write the flag unset with inline data present). Removed the false "must agree" invariant.
- §11 UUID tolerance: version_id / data_dir (ID/DDir) decode as a fixed 16-byte Uuid::from_bytes and fail closed on a present-but-non-16-byte value; only transitioned-versionID uses the tolerant Uuid::from_slice(...).ok().filter(!nil). Split the previously-conflated bullet.
- §11 decode blanket: narrowed "everything else must degrade to a tolerant default" to enumerate the legitimate structural/geometry/version/length fail-closed guards (header array length, unknown header_ver, version > max, versions_len / bin_len bounds, non-16-byte ID/DDir).
- §8 / §13 codec guard: has_valid_dimensions() is a &self method, so it runs after construction and reliably covers only block_size == 0; a data_blocks == 0 geometry with parity > 0 panics in the .expect constructor before the guard runs. Noted the fallible-constructor fix.

Refuted and intentionally left unchanged: the "accepts container major == 1 with any minor" statement is correct — the reader compares only major (rejects major > 1) and never gates on minor, so minor 4 is accepted.
This commit is contained in:
Zhengchao An
2026-07-18 21:05:22 +08:00
committed by GitHub
parent 53728a03d3
commit 9b197fc1c2
+13 -13
View File
@@ -115,8 +115,8 @@ distribution[i-1] = 1 + ((start + i) % N) for i in 1..=N // a cyclic rotatio
- **INVARIANT — block size.** New objects use `BLOCK_SIZE_V2 = 1 MiB` ([object_api/mod.rs](../../crates/ecstore/src/object_api/mod.rs)); it is stored per version in `ErasureInfo.block_size` and must be read back from metadata, never assumed.
- **INVARIANT — shard-size formulas (frozen for on-disk compatibility).** Two formulas exist and must both be preserved:
- Modern ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)): `calc_shard_size(block_size, data_shards) = block_size.div_ceil(data_shards)`.
- Legacy ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)): `calc_shard_size_legacy = (block_size.div_ceil(data_shards) + 1) & !1` — round the ceiling **up to the nearest even number**, byte-identical to MinIO's even-padding and to `filemeta`'s own `calc_shard_size` ([fileinfo.rs](../../crates/filemeta/src/fileinfo.rs)).
- Modern ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)): `calc_shard_size(block_size, data_shards) = block_size.div_ceil(data_shards)` — plain ceiling division, no even rounding. This is the MinIO-compatible sizing (MinIO's `Erasure.ShardSize` is the same plain `ceil(block_size / data_shards)`); the modern GF(2⁸) path uses it for all new and MinIO-migrated data.
- Legacy ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)): `calc_shard_size_legacy = (block_size.div_ceil(data_shards) + 1) & !1` — round the ceiling **up to the nearest even number**. This even-padded form is **RustFS-legacy-only** and matches `filemeta`'s own even-padded `calc_shard_size` ([fileinfo.rs](../../crates/filemeta/src/fileinfo.rs)); it is **not** MinIO's sizing and the two differ by one byte at typical geometries (for `block_size = 1 MiB`, `data_shards = 6`, plain `div_ceil` yields `174763` bytes whereas the legacy even-padded form yields `174764`). Because MinIO-migrated data is decoded by the modern path (§1), this legacy even-padding never applies to MinIO objects.
- The runtime codec picks the formula from `uses_legacy` ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)); the metadata layer `ErasureInfo::shard_size` always uses the even-padded form. Modern reads/writes drive off `Erasure::shard_size`.
- Whole-file logical shard size — `shard_file_size(total_length)` ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)): `0` for empty, pass-through for negative, else `full_blocks * shard_size() + shard_size_fn(last_block_size, data_shards)`. This is the pre-bitrot size; the on-disk file is larger by the per-block hash bytes (§5).
@@ -194,7 +194,7 @@ Fields: `version_id`, `mod_time`, `signature: [u8;4]`, `version_type`, `flags: u
**`MetaObject` (V2Obj)** — msgpack map, keys (**INVARIANT**): `ID`, `DDir`, `EcAlgo`, `EcM`, `EcN`, `EcBSize`, `EcIndex`, `EcDist`, `CSumAlgo`, `PartNums`, `PartETags`, `PartSizes`, `PartASizes`, `PartIdx`, `Size`, `MTime`, `MetaSys`, `MetaUsr` ([version.rs](../../crates/filemeta/src/filemeta/version.rs)). Notes:
- `ID` / `DDir` are **16 raw UUID bytes**; nil ⇒ `None` on decode, `None` ⇒ 16 zero bytes on write.
- `MTime` is **unix-nanos** sint; `UNIX_EPOCH``None`, and `None` is omitted on write so it never round-trips to `Some(epoch)`.
- `MTime` is **unix-nanos** sint. The `MTime` key is **always written** — both `MetaObject` (V2Obj) and `MetaDeleteMarker` (DelObj) emit it unconditionally — and a `None` mod_time is encoded as `0` (`UNIX_EPOCH` nanos), **not** omitted. Round-trip safety is enforced on the **decode** side: `UNIX_EPOCH``None` on read, so a `None` never resurfaces as `Some(epoch)`. (The only mod-time field actually omitted-when-`None` on write is the legacy `StatInfo.ModTime`, a different field.)
- `EcDist` is an **array** of per-shard slot values (not a bin blob). V2Obj does **not** store per-part bitrot checksums (only legacy V1 does).
- `PartETags` / `PartASizes` / `MetaSys` / `MetaUsr` are written as msgpack nil when empty; **a reader must treat nil and empty identically**. `PartIdx` is omitted entirely when empty.
- Part arrays are **parallel and index-aligned**: when parts are materialized (the `all_parts` decode path), `PartNums` / `PartSizes` / `PartASizes` must be equal length (mismatch ⇒ `FileCorrupt`, because indexing would panic or miscompute Content-Length/Range); `PartETags` / `PartIdx` are soft-guarded (applied only if length matches, empty index ⇒ None). When parts are not materialized the arrays are not cross-checked.
@@ -216,7 +216,7 @@ Fields: `version_id`, `mod_time`, `signature: [u8;4]`, `version_type`, `flags: u
### 6.6 Inline data
Small objects store their payload inline after the container CRC ([filemeta_inline.rs](../../crates/filemeta/src/filemeta_inline.rs)): 1 version byte (`INLINE_DATA_VER = 1`) then a msgpack map of `version-key → bin`. **INVARIANT — the map key** is the version-id string, `"null"` (`NULL_VERSION_ID`) for the null/None version, else the lowercase hyphenated UUID. Presence is signalled by both `meta_sys[inline-data]` and the header `InlineData` flag, which must agree. The inline threshold is `should_inline` ([storageclass.rs](../../crates/ecstore/src/config/storageclass.rs)): inline if `shard_size ≤ inline_block/8` for versioned buckets, else `≤ inline_block`; `DEFAULT_INLINE_BLOCK = 128 KiB`.
Small objects store their payload inline after the container CRC ([filemeta_inline.rs](../../crates/filemeta/src/filemeta_inline.rs)): 1 version byte (`INLINE_DATA_VER = 1`) then a msgpack map of `version-key → bin`. **INVARIANT — the map key** is the version-id string, `"null"` (`NULL_VERSION_ID`) for the null/None version, else the lowercase hyphenated UUID. Presence is determined **on read** solely by the `meta_sys[inline-data]` body marker (`FileInfo::inline_data`); the read path gates inline extraction on that marker alone. The header `InlineData` flag is **written** (mirrored from the body on marshal) but is **not** consulted on read, and a disagreement is tolerated — MinIO may leave the header flag unset while inline data is present, so a reader must **not** require the flag and the marker to agree. The inline threshold is `should_inline` ([storageclass.rs](../../crates/ecstore/src/config/storageclass.rs)): inline if `shard_size ≤ inline_block/8` for versioned buckets, else `≤ inline_block`; `DEFAULT_INLINE_BLOCK = 128 KiB`.
---
@@ -225,9 +225,9 @@ Small objects store their payload inline after the container CRC ([filemeta_inli
`put_object` and multipart resolve the write layout (§2.2), encode (§4), write shards with bitrot (§5), then commit atomically.
- Encode-time gates: writable disks `< write_quorum``ErasureWriteQuorum`; committed shards `< write_quorum` after encode ⇒ error ([set_disk/ops/object.rs](../../crates/ecstore/src/set_disk/ops/object.rs)).
- **INVARIANT — atomic commit with rollback.** Commit is `rename_data` (per-disk temp → final) fanned across all disks ([core/io_primitives.rs](../../crates/ecstore/src/set_disk/core/io_primitives.rs)). If write quorum is not met (`reduce_write_quorum_errs`), every successful disk is undone (`delete_version{undo_write:true}`) and the write fails. A write that misses quorum is rolled back, never left partially committed.
- On success, `data_dir` is the value voted by ≥ write_quorum disks (`reduce_common_data_dir`); a `classify_rename_convergence` result drives post-commit heal.
- The write layout is centralized (a `WriteLayout` / `resolve_write_layout` abstraction resolves per-pool parity, storage class, and `max_parity`); the baseline computes it inline in the write path.
- **INVARIANT — atomic commit with best-effort rollback.** Commit is `rename_data` (per-disk temp → final) fanned across all disks ([core/io_primitives.rs](../../crates/ecstore/src/set_disk/core/io_primitives.rs)). If write quorum is not met (`reduce_write_quorum_errs`), every successful disk is undone (`delete_version{undo_write:true}`) and the original quorum error is returned. **Baseline:** the rollback is **best-effort** — undo failures are counted and `warn!`-logged, never propagated or retried — so a write that both misses quorum *and* whose rollback partially fails can leave shards on some disks; that partial residue is reconciled later by heal/scanner, not by the commit path. The guarantee the commit path enforces is "never *reports* success below quorum", not "never leaves any bytes behind".
- On success the newly committed dir is `fi.data_dir`. Separately, `reduce_common_data_dir` votes over each disk's **`old_data_dir`** (the *superseded* dir being dereferenced) and returns it when it reaches write_quorum, so the old dir can be reclaimed (`commit_rename_data_dir`) — it is a GC input, **not** the new `data_dir`. `classify_rename_convergence` classifies the commit (`PartialCommit` / `SignatureDivergent`), but **only the multipart-complete path consumes it** (`convergence.needs_heal()``send_heal_request`); the regular `put_object` path discards the convergence result and relies on the old-data-dir cleanup / `add_partial` heal enqueue instead.
- The write layout (per-pool parity, storage class, `max_parity`) is computed **inline** in the write path ([set_disk/ops/object.rs](../../crates/ecstore/src/set_disk/ops/object.rs)); on `main` there is **no** `WriteLayout` type or `resolve_write_layout` function — do not cite either as if it exists (§13's symbol-citation rule). A future refactor may centralize this; add the symbol to the spec only once it lands in code.
---
@@ -235,7 +235,7 @@ Small objects store their payload inline after the container CRC ([filemeta_inli
- **INVARIANT — read quorum = `data_blocks`.** `object_quorum_from_meta` returns `(read_quorum = data_blocks, write_quorum)` ([set_disk/metadata.rs](../../crates/ecstore/src/set_disk/metadata.rs)); `parity_blocks = common_parity(...)` is the parity value held by the most disks that still reaches its own read quorum. When `default_parity_count == 0`, read = write = all shards.
- Authoritative FileInfo selection — `find_file_info_in_quorum` ([set_disk/metadata.rs](../../crates/ecstore/src/set_disk/metadata.rs)) groups valid metas by a content-identity SHA-256 (`file_info_quorum_hash`) that hashes size/flags/mod_time/transition/version_id/data_dir/parts and, for real objects, data/parity/distribution — **excluding replication-status keys** so replication noise never splits quorum. A meta counts only if its mod_time equals the common mod_time (or etag matches when mod_time is absent). The winning hash must reach quorum, else `ErasureReadQuorum`. Latest-version reads may escalate to write_quorum to avoid resurrecting a partially-overwritten version.
- **INVARIANT — decode needs ≥ `data_blocks` shards.** The stripe reader requires `available_shards ≥ data_shards`; below that the read fails closed with a read-quorum error (never silent truncation) ([set_disk/read.rs](../../crates/ecstore/src/set_disk/read.rs), [set_disk/shard_source.rs](../../crates/ecstore/src/set_disk/shard_source.rs)). Before any `block_size` / `data_shards` division, `has_valid_dimensions()` must hold (`block_size > 0 && data_shards > 0`) or the read fails instead of dividing by zero ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)).
- **INVARIANT — decode needs ≥ `data_blocks` shards.** The stripe reader requires `available_shards ≥ data_shards`; below that the read fails closed with a read-quorum error (never silent truncation) ([set_disk/read.rs](../../crates/ecstore/src/set_disk/read.rs), [set_disk/shard_source.rs](../../crates/ecstore/src/set_disk/shard_source.rs)). Before any `block_size` / `data_shards` division, `has_valid_dimensions()` must hold (`block_size > 0 && data_shards > 0`) or the read fails instead of dividing by zero ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)); note this guard runs *after* codec construction and fully covers only `block_size == 0` — a `data_blocks == 0` geometry panics earlier in the constructor (§13).
- If `available ≥ data_blocks` but some shards are missing, the read is served **and** a background read-repair heal is enqueued.
- **INVARIANT — cross-stripe read verification.** When a data shard is missing and `available > data_blocks`, reconstruction regenerates parity and compares it to the surviving parity; a mismatch is `InvalidData "inconsistent read source shards"` (backlog#832), catching corruption that passed per-shard bitrot but disagrees across the stripe ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)).
@@ -275,8 +275,8 @@ RustFS is a **read-forward-compatible consumer**: it writes the current format a
**Decode-tolerance invariants (be liberal in what you accept on decode).** These are the contract this document newly codifies. Metadata read from disk or a peer is untrusted input, but decode must not reject shapes that legitimate older / foreign writers produce. Validation may be added, but only if it never rejects data the current or any prior RustFS/MinIO writer can legitimately produce:
- **Nil UUID ⇒ None.** `version_id`, `data_dir`, `transitioned-versionID` read as `Uuid::from_slice(...).ok().filter(!nil)` absent, empty, and nil all mean "no value", never `Uuid::nil()` ([version.rs](../../crates/filemeta/src/filemeta/version.rs); [AGENTS.md](../../AGENTS.md) Cross-Cutting Domain Invariants).
- **Epoch mod_time ⇒ None**, and `None` is omitted on write.
- **Nil UUID ⇒ None (value-level; required identifiers are fixed-width).** `version_id` and `data_dir` (`ID`/`DDir`) are decoded as a **fixed 16-byte** field via `Uuid::from_bytes`: a nil 16-byte value ⇒ `None` (and `None` ⇒ 16 zero bytes on write), but a *present-but-non-16-byte* value is **not** tolerated — it fails closed (the sibling extractors `decode_data_dir_from_v2_object` / `parse_legacy_uuid_bytes` return an explicit `must be 16 bytes` error). The v3 shallow header deliberately keeps a nil `version_id` as `Some(nil)` (null-version disambiguation), not `None`. Only **`transitioned-versionID`** uses the fully tolerant `Uuid::from_slice(...).ok().filter(!nil)`, where absent / empty / nil / any non-16-byte / foreign-string value all decode to `None`. Do not conflate the required identifiers with this optional tier id ([version.rs](../../crates/filemeta/src/filemeta/version.rs); [AGENTS.md](../../AGENTS.md) Cross-Cutting Domain Invariants).
- **Epoch mod_time ⇒ None** on decode. (The `MTime` key is always written and a `None` is encoded as `UNIX_EPOCH`, not omitted; this decode-side rule is what keeps `None` from round-tripping to `Some(epoch)`. Only the legacy `StatInfo.ModTime` field is omitted-when-`None` on write.)
- **Skip unknown msgpack fields** for forward-compat (Go `dc.Skip()` parity) at every decoder.
- **Hard-guard only length-critical arrays** (`PartNums` / `PartSizes` / `PartASizes` must match), **soft-guard recomputable ones** (`PartETags` / `PartIdx` applied only if length matches; empty ⇒ default). All-empty `PartETags` must equal absent.
- **Negative `part.actual_size` is a valid compressed-unknown sentinel** and must be tolerated on decode (the read path's `get_actual_size` already relies on it). Do **not** reject `actual_size < 0`.
@@ -284,7 +284,7 @@ RustFS is a **read-forward-compatible consumer**: it writes the current format a
- **Dual-key read fallback** (RustFS prefix, then MinIO prefix).
- **Container/header/meta versions**: accept `≤` current, reject only `>`.
Two related hardening exceptions that are legitimate (fail-closed is correct there): a length mismatch between required parallel part arrays is `FileCorrupt` (indexing would corrupt returned data), and a CRC / bitrot mismatch is fatal (the bytes are provably wrong). Everything else on the decode path must degrade to a tolerant default, not an error.
Structural, geometry, and version guards legitimately **fail closed**, and turning them into tolerant defaults would hide corruption. Fail-closed is correct for: a length mismatch between required parallel part arrays (`FileCorrupt` indexing would corrupt returned data); a CRC / bitrot mismatch (the bytes are provably wrong); a container/header/meta version *greater* than the current max; a header array length that does not match its `header_ver` (4/5/7), or an unknown `header_ver`; `versions_len` or a per-version `bin_len` exceeding the blob size; and a present-but-non-16-byte `ID`/`DDir` (decoded as a fixed 16-byte `Uuid::from_bytes` — see the corrected UUID bullet above). Tolerance applies to *recoverable, value-level* shapes — nil/absent UUIDs, epoch mod_time, unknown map keys, empty-vs-absent collections, a foreign or oversized `transitioned-versionID`**not** to structural corruption. (Enum decode is a middle case: `VersionType::from_u8` degrades an unknown value to `Invalid` at the decode step, and the object is then rejected by the higher-level `valid()` check.)
---
@@ -296,7 +296,7 @@ Geometry & math
- Set size `N ∈ 2..=16` for multi-drive layouts (single-drive deployments run at `N = 1`, parity 0); `N = data_blocks + parity_blocks`.
- `parity ≤ N/2`; `STANDARD parity ≥ RRS parity`; parity resolved **per pool** for its own `N`.
- `read_quorum = data_blocks`; `write_quorum = data_blocks` (`+1` iff `data == parity`); delete-marker quorum `N/2 + 1`.
- Decode requires `≥ data_blocks` shards; below that, fail closed. Writes missing quorum roll back.
- Decode requires `≥ data_blocks` shards; below that, fail closed. Writes missing quorum roll back (best-effort — see §7).
Algorithm
- Modern RS over GF(2⁸) (`rs-vandermonde`) for new writes; legacy GF(2¹⁶) for old files, selected by `uses_legacy_checksum`.
@@ -325,7 +325,7 @@ Decode tolerance
- **Keep this document in sync.** A change to any governed behavior, formula, format field, or invariant must update this spec in the same PR; renaming a cited symbol must update its reference here. The spec is normative and is the checklist the next change is reviewed against, so drift is a correctness defect. References are symbol-based (not line numbers) specifically so ordinary refactors do not invalidate them — but semantic changes still must.
- **Adding an on-disk field** must be additive: new msgpack key or a `minor`/`meta_ver` bump with a read path for the old value; keep decoders skipping unknown keys; write both internal-key prefixes; never repurpose or reorder existing keys or header array positions.
- **Never make a decode boundary stricter** than what §11 allows without (a) proving no legitimate older-RustFS or MinIO-migrated shape is rejected, and (b) a regression test against real on-disk and MinIO fixtures. New validation belongs at the trust boundary and must fail *open to a tolerant default*, not closed to `FileCorrupt`, for anything recoverable. (Concretely: rejecting a negative `actual_size`, or hard-failing a non-16-byte `transitioned-versionID`, breaks existing data — see §11.)
- **Codec construction.** The baseline exposes panicking `Erasure::new` / `new_with_options` (the codec's shard-count validation surfaces as an `.expect` panic), plus `has_valid_dimensions()` as the read-path preflight ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs)). Production paths that build a codec from on-disk geometry must guard with `has_valid_dimensions()` first; a fallible constructor is preferred over panicking in any path reachable from untrusted metadata.
- **Codec construction (baseline gap).** The baseline exposes panicking `Erasure::new` / `new_with_options`: the codec's shard-count validation surfaces as an `.expect` panic when `data_shards == 0 && parity_shards > 0` (`ReedSolomon::new``TooFewDataShards`). `has_valid_dimensions()` (`block_size > 0 && data_shards > 0`) is a **`&self`** method, so it can only run *after* construction — the read path builds the codec from on-disk geometry first and checks the guard second ([erasure.rs](../../crates/ecstore/src/erasure/coding/erasure.rs), [set_disk/read.rs](../../crates/ecstore/src/set_disk/read.rs)). It therefore reliably catches only the `block_size == 0` case (block size is never passed to `ReedSolomon::new`, so construction succeeds and the guard rejects it before any division); a `data_blocks == 0` xl.meta with `parity > 0` **panics in the constructor before the guard can run**. The correct fix is a **fallible constructor** (returning `Result`, not `.expect`) on any path reachable from untrusted metadata; until then `has_valid_dimensions()` is a partial preflight, not a complete guard.
- **Guardrail scripts** (part of `make pre-commit` / `make pre-pr`):
- [check_architecture_migration_rules.sh](../../scripts/check_architecture_migration_rules.sh) keeps the erasure engine crate-private and under its owner module, and keeps erasure-cache / `GLOBAL_IS_ERASURE*` access behind ecstore helpers.
- [check_doc_paths.sh](../../scripts/check_doc_paths.sh) validates that every repo path this document cites exists — keep citations to real paths.