Review follow-up: a rolling-upgrade window must not create new compressed multipart objects while pre-fix nodes (whose decompressor is not resumable) may still serve reads. The session marker is now additionally gated on RUSTFS_COMPRESSION_MULTIPART_ENABLED, default off, so the restored capability stays dark until the operator confirms fleet convergence. The default flips per the multipart-compression-default-off-window entry in docs/architecture/compat-cleanup-register.md once the minimum supported direct-upgrade release ships the resumable decoder.
Multipart uploads have bypassed disk compression since #5169 removed the session marker as a stopgap for mid-stream GET failures. The actual root cause was never the multipart layout: the legacy DecompressReader reset its payload consumption state on every poll re-entry, so a Poll::Pending in the middle of a block payload (routine under the erasure duplex) desynchronized the block framing and surfaced as LZ4 frameType errors. This rewrites the decoder as a resumable state machine, restores the multipart session compression marker, reports logical part sizes in ListParts, and makes the rebalance migration read raw stored bytes so compressed and encrypted objects survive migration verbatim.
Fixes#5957. Internal tracking: backlog#1848, backlog#1850.
* test(table-catalog): move the store-side stateful object backend into test_support
First half of the issue's PR2: the store tests' TestCatalogObjectBackend cluster (state/record/locks/pause types, the seed/fail/pause instrumented inherent impl, the TableCatalogObjectBackend trait impl, and the BlockingObjectPublication/UnserializedTestPublication commit-publication fakes — 544 lines) moves verbatim from table_catalog/tests.rs into test_support.rs, with pub(crate) visibility on the items and fields the tests reach directly. Pure move, no behavior change; the admin handler tests' TestTableCatalogObjectBackend union (its put barrier / fail-path / lock-attempt instrumentation folding into this fake) is the second half.
Verification: cargo test -p rustfs --lib table_catalog 481 passed; clippy --lib --tests -D warnings clean; make pre-commit green.
Ref rustfs/backlog#1837 (PR2, part 1).
* test(table-catalog): fold the admin object backend into the shared fake
Second half of PR2: the admin handler tests' TestTableCatalogObjectBackend (struct, inherent impl, trait impl, lock alias — 201 lines) is deleted and its instrumentation folded into the shared TestCatalogObjectBackend, which the admin tests now take through a type alias so no call site is renamed.
Two behavioral differences between the two fakes were found by the test suites rather than assumed away, and both are preserved:
- Lock observability: the admin fake implemented only acquire_write_lock, so the trait's default acquire_read_lock -> acquire_write_lock delegation made read acquisitions visible in lock_attempts. The shared fake implements both independently, so five fence/lock tests timed out until the read path also records attempts.
- Etag generation: the admin fake used content-addressed sha256 etags (its tests observe an etag and expect rewriting identical bytes to reproduce it) while the store fake uses an incrementing counter. Instead of silently picking one, the union carries a content_addressed_etags flag; the 80 admin construction sites go through TestCatalogObjectBackend::content_addressed() and the store tests keep counter semantics.
The six one-shot path-keyed injection knobs (fail/corrupt put, missing/fail read, put barrier) run before the store fake's attempt-indexed injection maps, matching each fake's original ordering.
Verification: cargo test -p rustfs --lib table_catalog 481 passed; --lib admin::handlers::table_catalog 167 passed; clippy --lib --tests -D warnings clean; make pre-commit green.
Ref rustfs/backlog#1837 (PR2, part 2).
* refactor(sse): sink managed-SSE attribution into the shared encryption-keys module
Moves the managed-SSE classifier — stored_managed_encryption_key, contains_managed_encryption_metadata, normalize_managed_metadata — and the SSEType enum from rustfs/src/storage/sse.rs into crates/utils/src/http/object_encryption_keys.rs, the module that already owns every constant they read. This is PR-B0 of rustfs/backlog#1643: crates/scanner must never depend on the rustfs binary crate, so encryption attribution has to live in a shared lower layer before the scanner can report per-scheme coverage without growing a second classifier.
SSEType moves wholesale (option a): its only impl is the dependency-free audit_label(), so the enum relocates verbatim (audit_label becomes pub) and rustfs::storage::sse re-exports it, keeping every existing path compiling. The one piece that cannot move verbatim is normalize_managed_metadata's KMS-context branch, which needs base64 and serde_json — dependencies rustfs-utils does not have and does not gain here. The shared normalizer instead takes an injected Option<fn(&str) -> Option<String>> context recoder; sse.rs passes recode_minio_kms_context, the old inline chain verbatim including the silent skip on decode failure. stored_managed_encryption_key passes no recoder because the context mapping only ever inserts the context key, which the key-id lookup never reads, so its output is identical.
Every metadata lookup stays a case-sensitive exact match (lowercase x-amz-* stored forms, TitleCase MinIO-internal names) per the backlog#1775 trap; new shared-module tests pin that, and a source-scan test in sse.rs asserts the classifier has exactly one definition so a second copy cannot silently return.
* fix(utils): satisfy encryption key test clippy
---------
Co-authored-by: cxymds <cxymds@gmail.com>
Keep S3 CopyObject's real outer owner task alive across caller cancellation so the source/destination bucket guards, same-key copy guard, storage commit, and post-commit publication hooks complete as one request-owned transaction boundary.
Co-authored-by: heihutu <heihutu@gmail.com>
The two table_catalog test files (27.5K lines combined) each maintained a parallel constructor stack for Iceberg metadata JSON and avro manifest-list/manifest bytes. Per the issue's adversarial ruling the parameterized admin variants are canonical (the store file hardcoded sequence 7 / snapshot 20); the two stacks were verified structurally identical first — schemas byte-equal, field lists and values aligned.
New #[cfg(test)] table_catalog/test_support.rs owns the seven constructors (metadata JSON, three manifest-list variants, two manifest variants, nullable_long). The admin tests import them under their old names; the store tests keep their historical signatures as thin delegates passing the fixed values explicitly — every produced byte is identical to the pre-extraction fixtures (the delegate's argument order was cross-checked against the canonical destructuring after an initial swap surfaced as five sequence-bound validation failures).
Ref rustfs/backlog#1837 (PR1).
Use BytesMut spare capacity for direct and pooled small PUT body reads while preserving exact-length validation.
Co-authored-by: heihutu <heihutu@gmail.com>
RemoteAddr, the DependencyReadiness family (DependencyReadiness, ReadinessDegradedReason, DependencyReadinessReport), and convert_ecstore_object_info (with its offset_date_time_to_timestamp helper) are consumed across app, infra, and interface layers but lived under server, so every lower-layer import was an upward app->interface or infra->interface edge the layer guard had to baseline.
They now live in a new layer-neutral rustfs/src/shared_types.rs (classified infra by the guard, making all consumer imports downward or lateral). server::readiness and server::event re-export for their own internals; the eight consumer sites (admin_usecase, bucket_usecase, object_usecase, cluster_snapshot, storage/access, storage/helper, plus the admin handler tests) import from the new home. Pure move: no type, impl, or behavior change.
The regenerated layer-dependency baseline shrinks by exactly eight lines with zero additions — the ratchet's intended direction. The two remaining readiness entries (collect/snapshot fn imports) need the collection machinery itself extracted from server and are left for the issue's PR5 scope.
Ref rustfs/backlog#1834 (PR4).
The 14 tests carried #[ignore = "requires isolated global object layer state"], and the only CI lane that runs ignored tests filters for lifecycle tests — so they executed nowhere. Under nextest, the authoritative runner, every test owns its process and the stale reason no longer applies; all 14 pass.
Ten of them assert the InternalError path taken while the global object layer is uninitialized, a premise a sibling test can destroy under the documented shared-process cargo test fallback. Those ten now start with an explicit premise guard: when a sibling already initialized the store the test skips with a message instead of asserting against a scenario it does not describe. Under nextest the guard never fires and the assertions always run.
Dual-runner evidence: nextest 79 passed; cargo test module-scoped 79 passed; the full storage-tree cargo test sweep returns to its pre-existing baseline (8 unrelated in-process failures, none introduced or worsened here). No test deleted.
Ref rustfs/backlog#1830 (PR1).
* perf(metrics): attribute PUT stage costs
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(ecstore): move PUT metadata during shuffle
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(s3): reuse PUT object lock state
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(ecstore): trim PUT metadata fanout clones
Build per-disk PUT metadata only for committed writer slots, move the response metadata out of the fanout vector, and preserve fresh FileInfo shuffle semantics.
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(metrics): make PUT stage attribution opt-in
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(ecstore): commit inline PUT shards directly
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(ecstore): streamline rename staging cleanup
Use the directory-specific removal operation for rename_data staging parents. This avoids a guaranteed failed file-removal probe on Unix-like hosts and lets Windows remove the empty directory directly while preserving best-effort non-empty handling.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): cover inline PUT rename failures
Cache the detailed stage metrics gate once per PUT and exercise exact-quorum and quorum-minus-one failures after inline shard encoding.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
The peer-edit delivery fence from #5882 treated an equal applied generation as stale. One edit legitimately fans out one delivery per peer record under a single generation (the ILM-expiry edit sends every peer's record), so the receiver applied only the first body, raised its high-water mark, and silently acked-success while dropping the rest — enableILMExpiryReplication never converged on receiving sites and the three-node nightly e2e failed deterministically (issue #5767).
Only a strictly newer applied generation is stale now. Equal generation implies the same logical edit and re-applying a delivery is idempotent (update_peer overwrites the peer record; the mark is raised with max), while strictly older deliveries — the cross-node ordering case the fence exists for — stay rejected.
Adds a composed unit test driving three same-generation bodies through the receiver's fenced sequence, and widens the replication e2e's two site-replication wait helpers from a 10s polling ceiling to the 30s deadline the file's other waits use.
PutBucketVersioning with Status=Suspended on a bucket that carries a replication configuration now fails with InvalidBucketState, matching AWS S3 and MinIO. Suspension would start minting null versions that the versioned replication engine can never converge — the state is unreachable on AWS and MinIO, and the nightly acceptance-matrix e2e that tried to exercise it failed every night since it landed (issue #5767).
The acceptance-matrix test tail now pins the rejection contract (InvalidBucketState) and verifies a fresh matched PUT still replicates with a real version id after the rejected suspension.
The shared module rustfs_utils::http::object_encryption_keys is the single source of truth for encryption metadata key names, but three call sites still carried their own copies or bare literals: crates/kms/src/service.rs (two private constants plus four bare x-rustfs-encryption-* literals on both the write and read path), rustfs/src/app/select_object.rs (six SELECT_* copies), and rustfs/src/storage/options.rs (two private prefix copies now imported from header_compat). All values are unchanged, so the change is a compiler-verified rename.
The reader-only x-rustfs-internal-server-side-encryption- family gets a named constant with the verified judgment recorded on it: no writer emits these keys anywhere in the repo (the SSE writer persists the MinIO-branded keys verbatim for interop), the two comments claiming the dual-key invariant writes this twin were wrong and are corrected, and the defensive redaction/strip readers are kept because removing them is risk-asymmetric.
rustfs-kms's rustfs-utils dependency now declares the http feature it uses instead of relying on feature unification from sibling crates.
Refs rustfs/backlog#1775, rustfs/backlog#1562.
test_format_v1 (ecstore layout::format) only printed its results; the pinned v1 format.json literal never parsed at all because "this": null fails Uuid deserialization, and the Err was silently discarded. Fix the fixture to the real on-disk shape (MinIO and RustFS always write a concrete disk UUID there) and assert a serialize->parse roundtrip identity plus every pinned field of the literal.
test_console_cors_configuration discarded all four parse_cors_origins results; parse_cors_origins returns an opaque CorsLayer, so the test now drives real CORS preflight requests through an axum router and asserts the allow-origin outcomes: wildcard answers any origin with *, a configured list echoes listed origins and refuses unlisted ones, empty/unset configurations allow no cross-origin caller.
test_heal_channel_processor_new only constructed the processor; it now asserts the response channel accepts a send.
Ref rustfs/backlog#1836 (PR1).
select_object.rs re-declared six interop header names as SELECT_* locals (five X-Minio-Internal-Server-Side-Encryption-* markers plus x-rustfs-encryption-key-id). The canonical owners live in rustfs-utils' object_encryption_keys module, which the rustfs crate already depends on with the full feature set. Import them under their canonical names and drop the local copies; SELECT_KMS_ARN_PREFIX stays local because no canonical owner exists for the KMS ARN prefix.
Values are byte-identical, so no behavior change.
Ref rustfs/backlog#1833 (PR3).
A key-marker that does not start with the request prefix is invalid input, not an unimplemented feature.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: houseme <housemecn@gmail.com>
* test(site-replication): pin retry-event lost-update against locked RMW (red)
P1-15 (rustfs/backlog#1675 B2): the site-replication retry-event writers
(enqueue/dequeue, which hang off every hook broadcast path) perform a
load -> mutate -> persist without taking SITE_REPLICATION_STATE_LOCK, so
a single process can lose a concurrent lock-holding writer's update; the
service-side reload path is equally unlocked, and no writer holds a
distributed lock across the read-modify-write, so multi-node RMW loses
updates even where the process lock is held.
Red evidence (current main): replaying enqueue's exact three steps around
a completed mark_pending_rotation_peer_acked commit wipes the rotation
ack — the final state holds the retry event but not the ack.
* fix(site-replication): route state RMW through one locked transaction
P1-15 PR1 (rustfs/backlog#1675 B2). The site-replication state object
(config/site-replication/state.json, which also carries the retry-event
queue) was mutated through read-modify-write sequences with inconsistent
locking: the retry-event writers on every hook broadcast path and the
RPC-driven service reload took no lock at all (single-process lost
updates, pinned by the red commit), and no writer held a distributed lock
across the whole RMW (cross-node lost updates everywhere).
- New admin/site_replication_state module: the state transaction boundary
`with_site_replication_state_lock[_on]` — process mutex plus the
distributed config-object write lock (the pattern proven by the repair
state), with the shared path constant. The process mutex is transitional
until PR2 migrates the remaining ~26 call sites.
- handlers: typed `update_site_replication_state` (no-lock load /
persist-or-clear inside the boundary; normalizes the peer map exactly
once, retiring the double-clone/double-normalize persist path, P2-22).
Migrated: retry-event enqueue (always-write), dequeue (lock-free probe,
transaction on hit), mark_pending_rotation/remove_peer_acked.
- service reload: the tolerant byte-level read->normalize->save now runs
inside the same boundary via no-lock IO — a cluster-wide reload fan-out
can no longer overwrite a concurrent state writer. Normalization
semantics untouched (all six service-side tests unchanged and green).
- Add/PeerJoin/Edit handlers release the state guard before their peer
fan-out: the transport helpers' retry-event bookkeeping now re-enters
the state transaction and must not nest inside the guard (the
adversarial review caught this as a re-entrancy deadlock; the fix
mirrors the Remove/Rotate handlers' existing scope). The Edit non-
refresh branch commits before fanning out — the old fanout-first order
recorded retry events pointing at a state the local site had not saved.
- ecstore: delete_config_no_lock (+ facade/bridge exports) so the clear
half of persist-or-clear works under the held object lock.
Red -> green: the red commit pinned the deterministic lost-update
interleaving (stale retry-event persist wiping a committed rotation ack);
the test now drives the real functions concurrently for 8 rounds and
asserts every retry event and every ack survives. Full
handlers/service site-replication unit suites green (171 + 6); dual-node
site-replication e2e (state edit fresh/stale, object replication) green;
fmt / clippy / logging guardrails clean.
Adversarial review: one blocking finding (the re-entrancy deadlock above)
fixed and re-verified by a full second pass over all 30 lock sites and
the Add/Join/Edit call graphs. Non-blocking notes recorded for PR2:
mark_* now persists on miss (persist-or-clear semantics; a miss-skip
return is a cheap follow-up), Add still holds the guard across the peer
join probe (pre-existing availability debt), and a timeout-guarded
unreachable-peer regression test for the fan-out paths.
* fix(site-replication): keep the state mutex behind an owner helper
CI's architecture migration guard lists SITE_REPLICATION_STATE_LOCK as an
owner-local static, so it may not be `pub(crate)`. Keep it private to the
new module and let the not-yet-migrated RMW call sites take it through
`site_replication_state_process_guard()` — the sanctioned owner-helper
pattern; the helper disappears with the mutex in PR2.
* fix(site-replication): keep peer-edit delivery under the state guard
Review follow-up (#5882).
Releasing the guard before the fan-out (my deadlock fix) traded the
ordering the guard used to provide: edit A could commit and stall while
edit B committed and reached a peer first, then A arrived last and won.
The peer edit handler applies whatever arrives — it has no generation or
updated-at fence — and a successful stale delivery is not repaired by the
retry queue, so the sites diverge silently.
The fan-out is back under the guard. What actually could not run there is
the retry-event bookkeeping, which re-enters the state transaction, so the
edit branch now delivers with the plain transport and settles the retry
queue after the guard is released: successes dequeue, the first failure
enqueues and is returned. Ordering and bookkeeping both preserved. The add
handler keeps its peer-edit finalize fan-out under the guard for the same
reason and releases only before bootstrap/back-fill, which send bucket-ops
(not peer edits) through retry-event transports.
The concurrency test could not tell the two guards apart — both writers
took both locks, so it passed with either removed. Replaced by two tests
that isolate one guard each, both verified by mutation:
- a process-only legacy writer (the shape the not-yet-migrated call sites
still use) racing the transaction: fails when the transaction stops
taking the process mutex;
- two writers that bypass the process mutex, as separate nodes do, driving
the production object-lock path (`with_site_replication_state_object_lock`
factored out for exactly this): fails when the distributed lock is
removed.
Verification: handlers 173 + service 6 unit tests green; site-replication
dual-node and three-node edit e2e green; arch/layer/logging guardrails,
fmt and clippy clean.
* fix(site-replication): fence peer-edit delivery by generation
Review follow-up on the two remaining holes in the edit path.
Ordering was only process-local. `SITE_REPLICATION_STATE_LOCK` is per
node, so holding it across the fan-out orders the edits ONE node accepts
and nothing else: two nodes of the same site can both commit and reach a
peer in the opposite order, and the peer edit handler applied whatever
arrived last. Each edit now takes a generation from
`SiteReplicationState::edit_generation`, allocated in the same commit as
the edit itself — i.e. under the distributed state-object lock, so two
nodes can never share one. The generation rides the peer-edit request as
query parameters and the receiver rejects (acks without applying) a
delivery at or below the mark it already applied for that origin site,
recording the mark in the same commit as the edit it fences. Peers that
predate the fence send no parameters and are applied as before.
Retry settlement could discard a newer failure. After the guard is
released, a success for edit A removed every retry event for
(peer, peer-edit): if edit B committed, failed its own delivery and
enqueued while A was in flight, A erased it — local state B, peer on A,
nothing queued to converge them. Settlement now only removes events whose
recorded generation is not newer than the one being settled, and a later
failure never lowers the fence. Broadcast paths carry no generation and
settle unconditionally as before; their events live under their own
paths and cannot collide with a peer-edit delivery.
A departed peer's mark is dropped on load: a site that leaves drops below
two peers, which clears its state object and restarts its counter at
zero, so a leftover mark would reject every edit it sends after it
rejoins.
Tests: two-node generation uniqueness (drop the object lock and the two
nodes collide), the receiver's staleness predicate and its wiring, the
settlement interleaving (drop the fence and B's retry is erased), and the
rejoin reset.
Refs: rustfs/backlog#1675 (P1-15)
overtrue