mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 16:28:15 +00:00
0e61ba7c63ca4b0288180c84837ab645ac70d546
4116 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0e61ba7c63 |
test(table-catalog): add scale fault rehearsal (#4359)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> Co-authored-by: houseme <housemecn@gmail.com> |
||
|
|
717cdd2abd |
fix(migration): decrypt MinIO IAM & server config on drop-in migration (#4358)
* fix(migration): decrypt MinIO IAM & server config on drop-in migration MinIO encrypts IAM identity/service-account files and the server config at rest with a key derived from the root credentials. The drop-in migration paths read those blobs from the legacy `.minio.sys` bucket and parsed them as plaintext JSON, so any encrypted blob failed to parse and was silently skipped with "incompatible format". This is why users migrating from MinIO kept their buckets/objects/policies but lost users and access keys (#2212). The IAM load path already knows how to decrypt these blobs (RustFS master keys plus MinIO-compatible legacy keys derived from the root credentials), but that logic lived behind a private method and was never used by the migration paths. Expose it as `rustfs_iam::try_decrypt_iam_blob` and inject it into both migration paths via a `LegacyBlobDecryptFn` callback (ecstore cannot depend on the IAM crate, so the closure is wired in the binary crate). When a blob cannot be decrypted the raw bytes are used as-is, preserving the previous plaintext-only behavior with no regression. Also improve object-layer migration observability without changing control flow: `try_migrate_format` now distinguishes "no legacy format" (a normal fresh install) from "legacy format present but incompatible", and the caller logs a loud error before initializing a fresh format that would leave the existing MinIO objects unreadable. Topology/version skip reasons are promoted from debug to warn. Fixes a pre-existing test isolation race by marking `test_recovery_falls_back_to_default_config_when_blob_stays_corrupt` serial, since it reads a process-wide env var toggled by a sibling test. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(migration): box FormatV3 in LegacyFormatOutcome to satisfy clippy Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): stabilize concurrent multipart resend lock timeout concurrent_resend_same_part_commits_one_generation spawns 6 same-part resends whose cross-disk commits serialize on the per-uploadId commit lock. Under the full nextest suite the parallel disk load pushes those serialized commits past the small default lock-acquire timeout (5s), producing a spurious `Lock(Timeout ...)` unrelated to the property under test (observed on CI at 5.775s vs ~0.5s in isolation). Raise RUSTFS_OBJECT_LOCK_ACQUIRE_TIMEOUT to the production default (30s) for the concurrent-commit section via temp_env, so the regression guard reflects correctness (exactly one intact generation) rather than disk latency under CI load. The meaningful assertions are unchanged, and #[serial] keeps the process-wide env override isolated. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(lock): bound fast-lock notification wait to prevent lost-wakeup stall The real cause of the concurrent_resend_same_part_commits_one_generation failures was a lost wakeup in the fast-lock slow path, not disk latency: raising the acquire timeout to 30s only delayed the failure (it then timed out at 30s), proving a genuine stall rather than overload. In acquire_lock_slow_path a waiter that reaches the notification phase did a single `timeout(remaining, wait_for_write())` spanning the whole acquire budget, and treated that wait's elapse as a hard `Timeout`. But the release path only notifies when `writer_waiters > 0`, so if the holder releases in the gap after the waiter's `try_acquire` fails and before it registers as a waiter, no notification (and no stored permit, since the pooled `Notify` is gated) is produced. The waiter then blocks until the deadline even though the lock is free and stays free — a spurious lock-acquire timeout. The shared process-wide notify pool makes it worse: a wakeup can be consumed by a waiter of a different lock hashing to the same slot. Bound each notification wait (NOTIFY_WAIT_CAP = 50ms) and, on elapse, loop back and re-`try_acquire` instead of returning `Timeout`; the deadline check at the top of the loop is the single source of truth for timing out. A lost/stolen wakeup now degrades to bounded re-polling (acquire within ~50ms of the lock becoming free) instead of stalling for the whole timeout. Correctness (mutual exclusion) is unchanged — acquisition still only happens via `try_acquire_*`. Add a regression test that reproduces the stall (holder + late waiter across many keys): it times out without the fix and passes in ~1s with it. Revert the earlier acquire-timeout workaround in the multipart test now that the underlying stall is fixed, so it runs under the default timeout again. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
796fbb47da | ci: replace docker image build runner with aks dind container (#4361) | ||
|
|
88645c9169 |
fix(server): stop closing idle proxy keep-alive connections + reverse-proxy hardening (#3076) (#4360)
* fix(server): raise default HTTP/1.1 idle keep-alive timeout for proxies In hyper's HTTP/1.1 stack `header_read_timeout` is armed as soon as the connection is ready to read the next request head, so on a keep-alive connection it also bounds how long an idle upstream connection may sit before RustFS closes it. The previous 5s default meant RustFS FIN'd pooled upstream connections while reverse proxies (Caddy ~2min, Nginx 60s) still believed they were alive. The proxy then reused a dead socket and clients saw `socket hang up` / connection reset, most visibly on non-idempotent `PUT` uploads that proxies will not transparently retry (issue #3076). Raise DEFAULT_HTTP1_HEADER_READ_TIMEOUT to 75s (above common proxy idle-keepalive windows) and document the coupling. Still overridable via RUSTFS_HTTP1_HEADER_READ_TIMEOUT for deployments that expose RustFS directly to untrusted slow clients and want tighter slowloris protection. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(operations): add reverse-proxy deployment guide Document the request/body semantics RustFS expects from a proxy layer, the idle keep-alive mismatch that causes `socket hang up` on large PutObject writes, and known-good Caddy/Nginx configs plus Cloudflare caveats. Closes the documentation gap called out in issue #3076 and consolidates the scattered findings from #609/#934/#1492/#1766. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(object): bound stalled PutObject request-body reads with diagnostics When a reverse proxy or CDN forwards a partial request body and then goes silent without closing the connection, the inner body stream neither yields more bytes nor reports EOF, so RustFS waited forever for bytes that never arrive and the client saw a silent hang/abort (issue #3076). A short body that ends with a proper EOF was already rejected promptly; the gap was the no-EOF stall case. Add a single-point request-body guard on the PutObject path that wraps the incoming StreamingBlob with an inter-chunk read timeout. The timer resets on every chunk, so slow-but-progressing uploads are unaffected; it only fires after RUSTFS_HTTP_REQUEST_BODY_READ_TIMEOUT (default 300s, 0 disables) of complete silence. On timeout it logs a structured `put_object_body_read_stalled` event with received/expected byte counts and fails the read with ErrorKind::TimedOut instead of hanging. remaining_length/size_hint are forwarded so wrapping is transparent to downstream length handling. Covered by unit tests for the stall path, length-preserving pass-through, and the disabled (timeout=0) pass-through. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
f73880f354 |
fix(startup): survive slow multi-node cold starts (#4357)
* fix(server): make startup readiness wait configurable and raise default (#4264) The startup runtime-readiness wait was a hardcoded 30s constant with no env override. On slow multi-node cold starts (Docker/K8s/Synology NAS) this window is shorter than the internal startup budgets it depends on — the endpoint DNS-retry window (~90s) and the format-load retry loop (~100s worst case) — so readiness times out and the node exits with `startup readiness timed out after 30s: storage_ready=false, lock_quorum_ready=false` before storage/lock quorum can converge, feeding the restart storm reported in the issue. - Add `RUSTFS_STARTUP_READINESS_MAX_WAIT_SECS` (default 120s), documented in rustfs-config health constants. - Resolve the wait at runtime via `startup_runtime_readiness_max_wait()`; a value of `0` falls back to the default instead of timing out instantly. - Repoint `STARTUP_RUNTIME_READINESS_MAX_WAIT` at the shared config default so there is a single source of truth, and cover the getter with unit tests. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): stop peer/disk background monitors on graceful shutdown (#4264) Long-lived peer health/recovery and remote-disk monitors are detached `tokio::spawn` tasks that each hold a `tracing::Span` via `.instrument(..)` for their whole lifetime. Nothing cancelled them at shutdown, so on the normal return path the Tokio runtime was dropped while they were still alive and their `Span`s were dropped during worker-thread thread-local-storage (TLS) destruction. At that point `tracing-subscriber`'s fmt `on_close` can touch an already-destroyed TLS slot and panic with `cannot access a Thread Local Storage value during or after destruction`, which escalates to a panic-during-panic abort (SIGILL / exit 132) — the crash reported on Synology in issue #4264, amplified by the restart storm. - Add `cluster::rpc::background_monitor` with a process-global shutdown token, `spawn_background_monitor()` (races the monitor future against that token so its span drops while the runtime is alive), and public `shutdown_background_monitors()`. - Route every span-holding peer_s3 / peer_rest / remote_disk monitor spawn through `spawn_background_monitor` instead of `tokio::spawn(..).instrument()`. - Expose `rustfs_ecstore::shutdown_background_monitors()` and call it from the graceful shutdown sequence (right after `ctx.cancel()`, before runtime teardown) via the `storage_api` compatibility boundary. Existing recovery-probe span-context tests still pass, confirming log correlation is preserved. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
9ae233d552 |
test(e2e): add SHA256 write and HEAD checksum regressions (#4354)
test(e2e): add SHA256 verify-on-write & HEAD-checksum regression for #4341 Issue #4341 reported (on beta.8) that RustFS accepted mismatched S3 SHA256 checksums on PutObject (HTTP 200 instead of 400 BadDigest) and did not return ChecksumSHA256 on HeadObject with ChecksumMode=ENABLED. Both behaviors are already correct on current main, but the existing e2e coverage only asserted the happy path (upload succeeds + GetObject content matches). It did not encode the issue's two acceptance criteria, so a future regression would go undetected. Add two focused e2e tests that assert exactly the issue's contract: - test_put_object_rejects_mismatched_sha256: a body that does not match the declared x-amz-checksum-sha256 must be rejected with BadDigest and must not be stored. - test_head_object_returns_stored_sha256: after a correct upload, HeadObject with ChecksumMode=ENABLED must return the stored base64 SHA256 digest. Both tests pass against a real rustfs server (mismatch -> HTTP 400 BadDigest verified). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
58114f49f2 |
perf(ecstore): k-way heap merge for ListObjects, drop clone-to-parse (#4347)
* perf(ecstore): replace linear merge scan with k-way heap and drop clone-to-parse (backlog#874 backlog#875) merge_entry_channels advanced the k-way merge with a linear scan over all channel heads (O(entries x channels)) and allocated two fresh Strings per pairwise comparison via path::clean. Every step also cloned MetaCacheEntry values, including entry.clone().xl_meta() clone-to-parse calls. - Introduce MergeHead with a cached cleaned name (allocated only when the raw name is not already clean) and drive the merge with a BinaryHeap of boxed heads: O(log channels) per entry, allocation-free comparisons. - Move entries through the merge instead of cloning; the winner is sent without an intermediate copy. - Remove the dead merge_file_meta_versions block: it only ran for prefix-dir groups whose entries have empty metadata, so xl_meta() always failed; cross-drive version merging happens in the resolve path. - Keep legacy same-name semantics (dir groups collapse, objects shadow prefix dirs, later object candidate wins) and add regression tests for interleaved ordering, dir/object precedence, uncleaned-name grouping, and prefix-dir collapse. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): honor ascending versions_sort in ListObjects walk (#4348) * fix(ecstore): honor ascending versions_sort in walk and document ordering invariant (backlog#876) The walk loop carried a bare `//TODO: SORT` inside the `WalkVersionsSortOrder::Ascending` branch, so the requested ascending order was silently ignored and versions streamed newest-first (the raw FileMeta order). WalkOptions defaults to Ascending, so every default walker -- notably replication resync, which replays versions and needs oldest-first to preserve the version-stack order -- received the exact opposite of the contract. FileMeta maintains versions newest-first (sort_by_mod_time is descending) and into_file_info_versions preserves that order, so ascending emission is the exact reverse of file_info_versions output. Reverse in place when ascending is requested and add a regression test locking the newest-first invariant plus the reversal contract. Key-ordering audit result (no gap found): per-disk walkers emit sorted streams, merge_entry_channels performs an ordered k-way merge, and gather_results only filters by marker/limit, so ListObjects key order is guaranteed upstream and needs no post-sort. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): enable GET metadata early-stop by default (#4349) * perf(ecstore): enable GET metadata early-stop by default with env opt-out (backlog#872) The metadata early-stop fanout (read_all_fileinfo_early_stop) has been implemented and instrumented for a while but stayed behind an opt-in flag, so default GETs always waited for every disk to answer the metadata read even after quorum agreement was reached. Flip RUSTFS_GET_METADATA_EARLY_STOP_ENABLE to default-on. The gate stays conservative: should_allow_metadata_early_stop only admits metadata-only reads (read_data=false) without version_id, healing, or free-version requirements, everything else falls back to the full-wait fanout, and setting the env var to false restores the old behavior entirely. The version-aware gate (RUSTFS_GET_METADATA_VERSION_EARLY_STOP_ENABLE) remains opt-in because versioned reads carry a higher stale-selection risk profile. Also replace the stale "optimize concurrency" TODO in get_object_fileinfo with a pointer to the early-stop implementation and add regression tests for the new default plus the explicit opt-out path. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): lazily construct codec streaming multipart readers (#4350) * perf(ecstore): lazily construct codec streaming multipart part readers (backlog#871) get_object_decode_reader_with_fileinfo opened shard readers for every part of a multipart object before returning the streaming reader, so TTFB paid for parts x disks file opens up front and an early client disconnect wasted the setup work for every unread part. Replace the eager loop with LazyMultipartCodecStreamingReader: the first part is still built eagerly so the dominant fallback conditions (missing shards / read quorum) are detected before any byte is streamed and the whole request can fall back to the legacy duplex path exactly as before. Each subsequent part is built on demand -- when the previous part hits EOF -- via a spawned task handle owned by the reader; dropping the reader aborts an in-flight build so disconnects stop all further IO. If a later part hits a fallback condition mid-stream (a shard vanished after the request started), the reader surfaces an explicit read error with a pipeline-failure metric instead of silently degrading; the client's retry then detects the condition on the eager first-part setup and takes the legacy path cleanly. Adds unit tests for in-order streaming across lazy boundaries, deferred construction (no build when the client stops within part 1), and the mid-stream fallback error path. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): prefetch next multipart part reader setup during decode (#4351) * perf(ecstore): prefetch next multipart part reader setup during decode (backlog#870) get_object_with_fileinfo processed multipart parts strictly serially: the next part's bitrot reader setup (file opens + read-quorum wait across all disks) only started after the current part finished decoding, so large multipart reads paid full setup latency between every part. Overlap the two stages with a depth-one pipeline: right after the current part's readers are obtained, the next part's setup is spawned (shared inputs behind Arc) and joined when the loop reaches that part. The shared setup_multipart_part_readers helper keeps stage-duration metrics semantics identical for both paths; a failed or stale prefetch falls back to the synchronous setup, and the PrefetchedReaderSetup guard aborts the in-flight task on error returns, early breaks, or caller drop so disconnects stop background disk IO. Gate: RUSTFS_GET_MULTIPART_READER_SETUP_PREFETCH (default on, env opt-out). Adds a three-part end-to-end read test covering the prefetch hit path and cross-part content ordering. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): move FileInfo through GET shuffle instead of cloning (#4352) perf(ecstore): move FileInfo entries through the GET shuffle instead of cloning (backlog#873) shuffle_disks_and_parts_metadata_by_index deep-cloned every valid FileInfo (parts, erasure info, metadata map) once per disk on each GET. Add an ownership-taking variant that runs the same by-index consistency check as a read-only first pass and then moves entries into their shuffled slots with mem::take, and switch get_object_with_fileinfo to it -- that call site already owned the parts metadata vector. Disk handles are Arc clones and stay cheap. Scope notes from the backlog#873 audit: - get_object_fileinfo's disks.clone() stays: DiskStore is Arc<Disk>, so the clone is per-slot refcounting and correctly avoids holding the RwLock read guard across the metadata fanout awaits. - get_object_decode_reader_with_fileinfo keeps the borrowing shuffle: its caller must retain files/disks for the legacy fallback path, so an owned variant would just shift the same clone upstream. - The metadata-cache hit path still clones parts_metadata; sharing the cached entry via Arc changes the read-path return types and is left as a follow-up. Equivalence tests cover both the by-index placement and the mod-time fallback against the borrowing variant. Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> * fix(ecstore): gate merge emission on cleaned key and clear clippy redundant_clone Address review + CI findings on the ListObjects/GET optimization PR: - merge_entry_channels gated emission on the raw entry name while the heap orders by the cleaned key, so entries whose cleaned order and raw byte order disagree (e.g. redundant slashes) could be dropped. Gate on the same cleaned sort key the heap uses; add a regression test (`a//c` after `a/b`). - Drop three redundant `.clone()` calls in test code flagged by clippy::redundant_clone (owned-shuffle equivalence tests and the walk ascending-versions contract test) that failed the CI clippy gate. - Document the known mid-stream fallback limitation of the opt-in multipart codec streaming reader (default off) and mark the in-place per-part legacy degradation as a follow-up. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): force full metadata fanout for object tagging writes (backlog#872) put_object_tags reads the object fileinfo with read_data=false and then writes the updated tags to the online-disk set that read returned. With metadata early-stop enabled by default, that read now returns as soon as read quorum is reached, so the online-disk set is only a read-quorum subset. Writing tags to that subset fails write quorum -> ErasureWriteQuorum -> S3 SlowDown, which is exactly the s3-tests tagging failures (PutObjectTagging/DeleteObjectTagging, reached max retries). Thread a caller-controlled `allow_early_stop` gate through read_all_fileinfo_observed/_inner and add get_object_fileinfo_gated; put_object_tags calls it with allow_early_stop=false so the metadata read does the full quorum fanout and returns the complete online-disk set as the write target. Pure-read callers (GET/HEAD/tag read) keep the early-stop fast path unchanged. Extract metadata_early_stop_permitted() as the single gate and add a unit test locking the invariant: caller opt-out (and observe=false, and data reads) never early-stop even with the env flags on. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
c9b976ad46 |
feat(ecstore): reclaim orphaned data dirs during heal (#4356)
* feat(ecstore): reclaim orphaned data dirs during heal (backlog #3231/#3191) Pre-#3510, an unversioned overwrite leaked one UUID-named data dir per PUT. The write path now cleans up going forward, but pre-existing strays stay on disk forever: heal's dangling logic only removes whole objects whose data is missing, never surplus data dirs of an otherwise-healthy object. That leaked space is what eventually made ListObjects scan tens of GB and time out. Add SetDisks::reclaim_orphan_data_dirs, a fail-closed, quorum-safe sweep that mirrors purge_orphan_dir_object: - referenced set is the UNION of get_data_dirs() across every online replica's xl.meta, so a dir named by any replica is kept; - if a disk holds the object dir but its xl.meta is missing or unparseable the object is treated as degraded and nothing is removed; - only UUID-named subdirectories are ever considered. Wire it into heal_object's healthy tail (under the object write lock) as a best-effort step so the background heal scanner and admin heal recover the leaked space automatically; a reclaim failure never fails the heal. Covered by four unit tests: removal of an unreferenced dir, no-op when all dirs are referenced, fail-closed abort on missing metadata, and cross-replica union preservation. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): fix typo unparseable -> unparsable Satisfies the repository typos CI check. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
0ce0388fc0 |
fix(ecstore): fsync inline rename_data rollback backup (#4346)
* fix(ecstore): fsync inline rename_data rollback backup The inline branch of LocalDisk::rename_data writes the previous version's xl.meta rollback backup (<old_data_dir>/xl.meta.bkp) with a bare std::fs::write, fsyncing neither the file nor its new directory entry, even when drive_sync_enabled() is true. The same closure already syncs the new xl.meta and the commit rename directory, and the non-inline branch persists its backup durably. That backup is the sole restore source for delete_version(undo_write=true) on a set-level write-quorum failure. With sync enabled, an inline overwrite plus power loss plus a quorum failure that triggers undo_write could restore a lost or truncated backup and fail to roll back to the prior committed object. Write the inline backup via OpenOptions + write_all and, when sync is enabled, sync_data() it and fsync_dir_std its parent, mirroring the non-inline branch. Extend the inline backup test to assert the .bkp contents equal the previous metadata bytes. Refs backlog#868 (disk-durability-01). * fix(ecstore): preserve to_file_error mapping on inline backup write Address review: the rewritten inline rollback-backup write must keep the same DiskError classification as the previous std::fs::write(...).map_err( to_file_error)? call. Without it, io errors (PermissionDenied/NotFound/ StorageFull/...) would surface as DiskError::Io(_) instead of the specific DiskError variants (FileAccessDenied/FileNotFound/DiskFull/...), since From<io::Error> for DiskError only recovers variants that were wrapped via to_file_error. Map open/write_all/sync_data/fsync_dir_std through to_file_error, matching write_all_internal. --------- Co-authored-by: houseme <housemecn@gmail.com> |
||
|
|
e4ecb1bce5 | fix: ci failed (#4353) | ||
|
|
85ba51ce72 | fix(ci): use direct hash comparison for sha256 verification on Windows (#4345) | ||
|
|
073bc96756 |
fix(filemeta): compute header signature instead of hardcoding zero (#4343)
The xl.meta version header `signature` was hardcoded to `[0,0,0,0]` on the write path (`From<FileMetaVersion> for FileMetaVersionHeader`), and the existing `get_signature` implementations only hashed `version_id` + `mod_time` (+ `size`), so two versions that share a version_id and mod_time but differ in body (e.g. a PutObjectTagging that reached only some disks) produced identical headers. Such divergence was undetectable and unhealable in the strict merge path (backlog#861, B12). Compute a real signature over the full version body, mirroring MinIO's `xlMetaV2Version.getSignature` semantics: - Fill the signature on the write path via the `From` impl (covers add_version / set_idx / update_object_version). - `MetaObject::get_signature` / `MetaDeleteMarker::get_signature` now clone the body, zero the per-disk `erasure_index`, fold `meta_user` / `meta_sys` in with order-independent hashes (msgpack map order is not stable across disks), marshal the rest and xxh64 it, then fold 64->32 bits. Empty vs all-empty PartETags are normalized alike. - Invalid/bodyless versions return an `err` sentinel rather than all-zero, so they never collide with a real all-zero legacy signature. Backward compatibility: the decode path preserves stored signature bytes, and the normal (non-strict) merge path already zeroes signatures before grouping, so existing all-zero xl.meta stays consistent across disks. Only strict merge/heal compares signatures, where legacy data is uniformly zero (no split) and genuine partial-write divergence is now correctly detected. Adds unit tests covering: non-zero signature on write, erasure_index independence, tag/metadata/size divergence detection, map-order independence, PartETags normalization, delete-marker divergence, and the err sentinel. |
||
|
|
5594b18912 |
fix(ecstore): make delete_volume non-recursive by default to prevent bucket-heal wipe (backlog#799 B1) (#4339)
* fix(ecstore): make delete_volume non-recursive by default to prevent bucket-heal wipe (backlog#799 B1) `delete_volume` unconditionally `remove_dir_all`'d the whole bucket tree, and the bucket-heal "remove" branch called it fire-and-forget on every local disk. A mis-classified "dangling" bucket (or a non-force S3 DeleteBucket on a populated bucket) was therefore recursively wiped — a potential whole-bucket data loss. The `VolumeNotEmpty` -> recreate/`BucketNotEmpty` handling already present in both delete_bucket paths was dead code because the primitive never refused. Add an explicit `force_delete` flag to `DiskAPI::delete_volume` and default the non-force path to a non-recursive `remove_dir` (rmdir), which fails atomically with `VolumeNotEmpty` if the bucket still holds any object data. Only an explicit force delete (S3 force bucket delete) removes recursively. Mirrors MinIO's `xlStorage.DeleteVol` (`Remove` vs `RemoveAll`). - Trait + all impls (local behavior, dispatch, disk_store, remote RPC) take the flag; the gRPC `DeleteVolumeRequest` gains a `force` field (proto3 default false → old peers get the safe non-recursive behavior on rolling upgrade). - Heal remove branch passes `false` and no longer discards the result: a `VolumeNotEmpty` refusal is logged (the bucket is not dangling) instead of wiping data. - Both `delete_bucket` paths pass `opts.force`, activating the previously-dead `VolumeNotEmpty` -> `BucketNotEmpty`/recreate handling (correct S3 semantics). Adds a regression test: non-force delete of a non-empty bucket returns VolumeNotEmpty and preserves the data; force delete removes it. Design converged by two independent expert reviews (MinIO-fidelity + defense-in-depth) referencing MinIO xl-storage.go. Refs backlog#799 (B1), issue rustfs/backlog#850. The safety expert's deeper hardening (typed capability instead of a bool, trash-instead-of-in-place for force, quorum re-verification of dangling) is noted on #850 as follow-up. * fix(ecstore): reword 'mis-classified' -> 'misclassified' to satisfy typos (backlog#799 B1) * fix(rustfs): thread force_delete through StorageDiskRpcExt::delete_volume + test literal (backlog#799 B1) |
||
|
|
28c0543f3c |
fix(filemeta): resolve core-storage audit P2 items B15/B16/B18 (backlog#863) (#4344)
fix(filemeta): resolve P2 core-storage audit items B15/B16/B18 (backlog#863) Fixes the three remaining actionable items from the core-storage reliability audit (rustfs/backlog#863). All three are metadata-layer correctness defects in the filemeta crate. B15 — version sort tie-break consistency `FileMeta::sort_by_mod_time` and the delete path used a hand-rolled comparator whose version_type tie-break for equal mod_time was the OPPOSITE of the canonical `FileMetaVersionHeader::sorts_before` (used by the metacache merge and latest-version selection). At equal mod_time it sorted a delete marker before its object, so the per-disk read path could treat an object as deleted while the quorum-merge path treated it as present. Both sort sites now derive their ordering from `sorts_before`, matching MinIO (object-first). B16 — replication reset state persistence The three sites that flush `reset_statuses_map` into `meta_sys` inserted each key verbatim. A bare-ARN key (produced by `ObjectInfo::replication_state`) has no internal prefix, so read-back — which only recognizes prefixed keys — silently dropped the reset state; a rustfs-only key was invisible to MinIO-compatible readers. New `persist_reset_statuses` normalizes every entry to the canonical `replication-reset-<arn>` suffix and writes both the `x-rustfs-internal-*` and `x-minio-internal-*` prefixes. B18 — Legacy (V1Obj) body round trip `FileMetaVersion::encode_to` never emitted the `V1Obj` field, so re-encoding a Legacy version silently dropped its entire body. Adds symmetric `encode_to` for `MetaObjectV1`/Stat/Erasure/ChecksumInfo/Part and a `write_msgp_time` helper (ext8/type-5/12-byte, matching the decoder). `Mode` uses the strict `write_u32` marker the decoder requires, and `Stat.ModTime` is written only when present so a `None` never round-trips to `Some(UNIX_EPOCH)`. Adds regression tests for each fix (138 filemeta tests pass). Verified with an adversarial multi-expert review that could not break any of the three. Refs rustfs/backlog#863 (B15, B16, B18). |
||
|
|
b813fc7739 |
fix(ecstore): reject zero erasure block_size in codec streaming read (#4340)
The codec streaming GET reader divides by erasure.block_size in build_codec_streaming_part_reader without validating the erasure dimensions, unlike the legacy multipart path which already rejects block_size==0 / data_shards==0. FileInfo::is_valid() does not check block_size, so corrupted on-disk metadata (block_size==0, data_blocks>0) passes validation and panics the read task with a divide-by-zero. Add Erasure::has_valid_dimensions() and reject invalid dimensions at the codec streaming entry before any disk access, mirroring the legacy guard (which now reuses the same predicate). Refs backlog#868 (868-1). |
||
|
|
8b2c67a100 | ci: cancel superseded main release builds (#4342) | ||
|
|
6f613317f6 |
feat(internode): optimize gRPC transport (#4337)
* feat(internode): P0 gRPC transport tuning, message limits, payload metrics Land the P0 subtask from docs/grpc-optimization: close the client-vs-server transport gaps and add instrumentation to size which unary RPCs need channel isolation in P1. Transport tuning (G3): the client `Endpoint` now disables Nagle and raises the HTTP/2 stream/connection flow-control windows to mirror the server socket, so small lock/health RPCs are not batched and larger metadata responses are not throttled by the 64KiB default window. All env-overridable, 0 opts out. Message-size limits (G1): both `NodeServiceClient` and `NodeServiceServer` set max decode/encode size (default 100MiB) instead of tonic's silent 4MiB cap, so a large multi-version xl.meta or aggregated ReadMultiple no longer fails out_of_range. The server limit is set on `NodeServiceServer` before wrapping in the auth `InterceptedService` (the interceptor type does not expose it). Payload instrumentation (P1 prep): ReadAll/ReadMultiple record a payload-size histogram plus a large-payload counter when a response crosses the configured threshold (default 8MiB), feeding alerting on paths that contend with latency-sensitive control-plane traffic on the shared channel. Threshold-only counter, no per-call hot-path log. Verification: cargo check/test on config, io-metrics, ecstore, rustfs; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P1 control/bulk gRPC channel isolation (opt-in) Land the P1 subtask from docs/grpc-optimization: physically separate large bytes-carrying unary RPCs from latency-sensitive control-plane RPCs so a big transfer can no longer head-of-line block a lock/health RPC on the shared HTTP/2 connection (G2/G5). Introduce ChannelClass { Control, Bulk } and get_channel_for_class in protos. Control RPCs keep the per-peer connection keyed by the bare address; Bulk RPCs (ReadAll/WriteAll/ReadMultiple/BatchReadVersion, via a new get_bulk_client) are round-robined across a small per-peer bulk pool. Rather than restructuring the global GLOBAL_CONN_MAP (and every consumer), bulk channels are cached under a composite key (addr\0bulk\0idx). The NUL separator cannot appear in a URL, so bulk keys never collide with the control key. This keeps the blast radius small on a consistency-sensitive path. create_new_channel is refactored into build_channel(dial_addr, cache_key) so several physically distinct channels to one peer cache independently while dialing/TLS still use the real address. Gated by RUSTFS_INTERNODE_CHANNEL_ISOLATION (default OFF) so the default build is byte-for-byte the pre-P1 behavior: bulk resolves to the control channel and the switch is a single-env rollback. RUSTFS_INTERNODE_BULK_CHANNELS (default 2, clamped >=1) sizes the pool. On failure, evict_failed_connection drops the whole bulk pool for the peer (round-robin hides which index was used), avoiding half-dead cached channels. Lock RPCs (remote_locker) already use the default Control path, so lock semantics and retry behavior are unchanged. Verification: cargo check/test on config, protos, ecstore, rustfs; new protos tests for bulk key routing and isolation-off passthrough; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P2 msgpack/JSON codec observability + encode buffer presizing Land the safe, wire-compatible slice of P2 from docs/grpc-optimization: the observability prerequisite for retiring the redundant JSON fields, plus a codec micro-optimization. No proto/wire-format change; JSON is still dual-written. Internode RPCs today dual-encode each metadata value as both msgpack (`*_bin`) and a JSON compatibility string, and decoders prefer `_bin` with a JSON fallback. Before the JSON fields can ever be dropped (a cross-version change), that fallback must be proven unused in production. Add rustfs_system_network_internode_msgpack_json_fallback_total{direction, message}: incremented whenever a decode falls back to the JSON field because the msgpack payload was absent. Wired into both directions — the client decoding peer responses (remote_disk.rs, incl. the list-level read_multiple/batch fallbacks) and the server decoding peer requests (node_service/disk.rs). This counter must read zero across a release window before send paths stop writing JSON and the proto text fields are reserved/removed (the deferred P2-1 steps). Also pre-size the msgpack encode buffers (Vec::with_capacity(512)) on both sides, eliminating the repeated growth reallocations for typical FileInfo payloads with zero added copy. Full thread_local buffer pooling is deferred: it needs either an extra copy (unclear net win) or a send-path buffer-return lifecycle, to be justified by a codec microbenchmark first. Verification: cargo check/test on io-metrics, ecstore, rustfs; new fallback counter smoke test; existing codec decode tests green; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(internode): add msgpack/JSON convergence observation runbook Runbook driving the observation-gated retirement of the redundant JSON compatibility fields on internode gRPC metadata RPCs (grpc-optimization P2-1). Documents the shipped fallback counter (rustfs_system_network_internode_msgpack_json_fallback_total{direction,message}), the PromQL to confirm it reads zero across a release window, a standing alert, and the staged flip/rollback procedure (env-gated msgpack-only send, then proto field removal in N+1). Includes the verified field -> peer-decoder audit: only fields whose peer decodes _bin first may be converged. Notes DeleteVersion.opts (DeleteOptions) is NOT convergence-ready — its server handler is not _bin-first and must gain a decode_msgpack_or_json path first. This gates the send-side change so it cannot empty a JSON field an old peer still needs. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): env-gated msgpack-only send + DeleteVersion _bin support (P2-1) Implements the send-side lever for retiring the redundant JSON compatibility fields on internode gRPC metadata RPCs, plus the missing `_bin` support on the delete path that it depends on (grpc-optimization P2-1). Default-off: the base build is byte-for-byte the prior dual-write behavior. Gated msgpack-only send (RUSTFS_INTERNODE_RPC_MSGPACK_ONLY, default false): - New rustfs_protos::internode_rpc_msgpack_only() reads the flag. - Client (remote_disk.rs) compat_json() and server (node_service/disk.rs) compat_response_json() emit an empty JSON string when the flag is on, so only the msgpack _bin payload is sent. The _bin field is always sent; decoders keep the JSON read fallback. Applied only to fields with a confirmed _bin-first peer decoder (WriteMetadata/UpdateMetadata/RenameData file_info, UpdateMetadata opts, ReadOptions, ReadMultipleReq, BatchReadVersionReq; ReadVersion/ReadXL/RenameData responses and the ReadMultiple/BatchReadVersion response lists). - Only enable after the P2 fallback counter has read zero across a release window (see docs/operations/internode-msgpack-json-convergence-runbook.md). Single-env rollback; no wire-format break. DeleteVersion(s) _bin support (prerequisite): - The DeleteVersion/DeleteVersions protos had NO _bin fields. Add additive (backward-compatible) bytes file_info_bin/opts_bin (DeleteVersion) and repeated bytes versions_bin + bytes opts_bin (DeleteVersions); regenerate the checked-in prost struct. - Client dual-writes them; server decodes them _bin-first with JSON fallback. - These delete fields are kept OUT of the msgpack-only set (always dual-write) until their own fallback counter reads zero across a window with the new decoders fully deployed. DeleteVersion.raw_file_info stays JSON-only (no _bin field yet). Verification: cargo check/test on protos, config, ecstore, rustfs (incl. the six delete request handler tests and a compat_json default-path test); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3 cluster peer online/offline health metric Land the safe observability core of P3 (grpc-optimization G6/G8): track each internode peer's reachability and expose the offline count, for parity with MinIO's minio_cluster_servers_offline_total. Pure instrumentation — peer selection and quorum are unchanged. - io-metrics: per-peer PeerHealthState { online, consecutive_failures } registry plus record_peer_reachable/record_peer_unreachable. A peer flips offline after N consecutive failures (dial failures or RPC-triggered evictions) and back online on the next successful dial; the count of offline peers is published to the rustfs_cluster_servers_offline_total gauge. - config: RUSTFS_INTERNODE_OFFLINE_FAILURE_THRESHOLD (default 3, clamped >= 1). - protos: build_channel marks the peer reachable on a successful dial and unreachable on a dial failure; evict_failed_connection feeds the failure signal too. Keyed by the real peer address, so control and bulk channels to one peer share health state. Deferred (documented in docs/grpc-optimization P3): startup prewarm (no clean topology-ready hook yet), the offline fast-bypass in peer routing (consistency- sensitive; must not change quorum), and idempotent-read-only retry. This commit is observability only. Verification: cargo check/test on io-metrics, config, protos (new peer-health state-machine and threshold-clamp tests); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3 control-channel prewarm + self-healing offline bypass Add the remaining P3 connection-lifecycle levers (grpc-optimization G6/G8), both env-gated and default-off so the base build is unchanged. Prewarm (RUSTFS_INTERNODE_PREWARM, default off): RemoteDisk::new spawns a best-effort background dial of the peer's control channel, deduped per peer address, moving the connect cost off the first RPC. Failures fall through to the existing lazy connect + recovery monitor. Offline bypass (RUSTFS_INTERNODE_OFFLINE_BYPASS, default off): remote_disk get_client/get_bulk_client fast-fail a peer already marked offline instead of paying the connect timeout, so the erasure layer proceeds on quorum sooner. This does NOT change quorum. It is self-healing: cluster_peer_should_bypass lets one request per RUSTFS_INTERNODE_OFFLINE_REPROBE_SECS (default 5s) through to recover the peer even with no background monitor, and the recovery monitor's own probe path calls the client directly so it is never bypassed. io-metrics gains cluster_peer_is_offline / cluster_peer_should_bypass (with a per-peer re-probe timestamp). Scope: data path only — remote_locker (lock RPCs, most consistency-sensitive) is left dual-writing/unbypassed as a follow-up. Verification: cargo check/test on io-metrics, config, ecstore (new self-healing bypass tests; all 105 rpc tests green); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(internode): add A/B benchmark runbook for gRPC optimization stages Reproducible before/after collection procedure for grpc-optimization P0–P3. Since every stage is env-gated, before/after is the same binary with different env — no rebuild. Documents, per stage: the exact env toggles (baseline vs enabled column), which existing bench script to run (run_internode_transport_baseline.sh / run_four_node_cluster_failover_bench.sh), the Prometheus metrics to capture, and the acceptance gates from the design docs (e.g. lock p99 down >= 20% for P1, msgpack fallback counter = 0 before enabling P2, correct rustfs_cluster_servers_offline_total for P3). Live runs require a multi-node cluster + load tool + Prometheus scrape and cannot be produced in a single-process sandbox; artifacts land under target/bench (gitignored) and attach to the PR. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3-2 lock-path offline bypass + P3-3 idempotent read retry Extend the offline bypass to the lock path and add opt-in retries for idempotent reads (grpc-optimization P3-2/P3-3). Both env-gated and default-off/zero. Offline bypass (lock path): factor the bypass decision into a shared pub(crate) internode_offline_bypass_reason(addr) and call it from remote_locker::get_client too, so lock RPCs to an offline peer fast-fail (letting dsync reach quorum sooner) instead of paying the connect timeout. Does not change quorum; the self-healing re-probe keeps peers recoverable. Gated by RUSTFS_INTERNODE_OFFLINE_BYPASS (default off). Idempotent read retry (P3-3): add execute_read_with_retry — a bounded, exponential-backoff retry for read-only/reentrant RPCs on transient network errors — and route disk_info through it. RUSTFS_INTERNODE_IDEMPOTENT_READ_RETRIES defaults to 0 (disabled). Write/lock RPCs are never retried (quorum/idempotency safety, per CLAUDE.md); the wrapper requires an Fn closure so only reads that rebuild their request from borrowed inputs qualify. Deferred: grpc.health.v1 (optional ecosystem-compat only; needs a new tonic-health dep and 3-way hybrid-service wiring — internal needs are met by the existing Ping RPC). Verification: cargo check/test on config, ecstore (105 rpc tests green incl. disk_info now via the retry wrapper); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(scripts): one-click internode gRPC A/B benchmark driver Wrap the per-stage env matrix from the benchmark runbook into scripts/run_internode_grpc_ab_bench.sh: given --stage <p0|p1|p2|p3> and --phase <before|after>, it emits the stage/phase RUSTFS_INTERNODE_* server env to <out-dir>/server-env.sh and runs the right underlying bench (run_internode_transport_baseline.sh for p0/p1/p2, run_four_node_cluster_failover_bench.sh for p3) into a labeled target/bench/internode-transport/<stage>-<phase>/. Passthrough args after `--` reach the underlying bench; --dry-run previews the env + command. The script is explicit that RUSTFS_INTERNODE_* are server env, so for the load-driven stages the operator must restart rustfs with the emitted env before the run; the docker four-node (p3) path exports them for a forwarding compose. shellcheck-clean. Runbook updated with a "One-click driver" section. Co-Authored-By: heihutu <heihutu@gmail.com> * chore(compose): forward RUSTFS_INTERNODE_* into the four-node cluster The four-node local-build compose only forwarded a fixed whitelist of env, so the internode gRPC knobs (grpc-optimization P0-P3) never reached the containers and the A/B bench driver's "after" phase was a no-op. Forward the full RUSTFS_INTERNODE_* set with defaults matching the binary defaults, so leaving them unset is a no-op and the A/B driver can toggle a stage per phase. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(internode): address Copilot review — retry health action + poison-safe peer health Two review nits on #4337: - P3-3 idempotent read retry (remote_disk.rs): execute_read_with_retry ran every attempt through execute_with_timeout_for_op, which hardcodes FailureHealthAction::MarkFailure. So the first transient error could flip the disk faulty and short-circuit the remaining retries, and each attempt over-counted the failure. Route all but the final attempt through execute_with_timeout_for_op_and_health_action with IgnoreFailure; only the last attempt marks faulty/evicts. No default impact (retries default 0). - Peer-health helpers (io-metrics): record_peer_reachable/record_peer_unreachable, cluster_peer_is_offline and cluster_peer_should_bypass early-returned on a poisoned mutex, permanently stalling the offline gauge and bypass state after a single panic. Recover via PoisonError::into_inner(). Verification: cargo check/test on io-metrics + ecstore (105 rpc tests green); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
8f11222a63 |
feat(admin): add MinIO-compatible IAM and IDP endpoints (#4334)
feat(admin): add MinIO-compatible IAM/IDP admin endpoints
Register and implement MinIO admin API compatibility for IAM/IDP:
- PUT /v3/import-iam-v2 and POST /v3/revoke-tokens/{userProvider}
- generic /v3/idp-config/{type}[/{name}] CRUD mapped onto existing config
- LDAP/OpenID service-account, policy-entities, and list-access-keys flows
Adds IamSys::delete_temp_account primitive to back STS token revocation.
revoke-tokens requires the broader ListUsers admin capability for
cross-user revocation (self-revocation only needs RemoveServiceAccount),
mirroring the cross-user guard used by the service-account handlers.
Registers admin route-policy inventory entries for every new route.
Unsupported LDAP/OpenID backends return honest compatibility errors.
Refs rustfs/backlog#609 #610 #616
|
||
|
|
3420f28c17 |
feat(admin): add MinIO-compatible diagnostics endpoints (#4333)
feat(admin): add MinIO-compatible diagnostics and service-control endpoints Register and implement MinIO admin API compatibility routes: - POST /v3/service: restart/stop trigger the existing graceful-shutdown path; freeze/unfreeze record advisory state (request admission is not yet gated, surfaced honestly via effective=false) - profiling/trace and healthinfo/obdinfo/log diagnostics endpoints - GET /v3/top/locks and POST /v3/force-unlock, backed by new FastObjectLockManager::list_locks()/force_unlock() introspection - speedtest routes where feasible Refs rustfs/backlog#604 #606 #607 #615 |
||
|
|
68f048b8fe |
fix(replication): persist delete marker mtime in MRF entries (#4331)
fix(replication): persist original mtime in MRF entries (backlog#867) MRF delete entries did not persist the original delete-marker mtime, so after a restart the recovery replay path reconstructed the delete without a source timestamp. Downstream the replica delete-marker was stamped with the replay time (now()) instead of the source mtime, causing delete-marker timestamp divergence across clusters. Extend the MrfReplicateEntry disk format with an optional deleteMarkerMtime field (persisted as Unix nanoseconds) in both duplicate struct definitions (rustfs-replication and rustfs-filemeta). DeletedObjectReplicationInfo now persists delete_marker_mtime, and start_mrf_processor restores it onto the reconstructed delete so the replica keeps the source timestamp. Backward compatibility: the new key uses skip_serializing_if + serde default, so historical MRF files without it decode to None and replay falls back to the current time (pre-#867 behaviour). No panic or entry loss on old files. Closes rustfs/backlog#867 |
||
|
|
b60686eb6f |
feat(admin): add replication diff/mrf and batch job admin endpoints (#4332)
Add MinIO-compatible admin endpoints under the /v3 prefix, wired to real RustFS subsystems where they exist and documented as compatibility-only where they do not. Replication (backlog#611): - POST /v3/replication/diff: bounded on-demand scan of object versions, returning versions whose replication status is PENDING or FAILED. Wired to list_object_versions and the per-version replication status. Requires the bucket to exist and have a replication configuration. The scan is capped (REPLICATION_DIFF_MAX_SCAN) and reports IsTruncated when partial. - GET /v3/replication/mrf: reports the failed-replication backlog (MinIO's MRF concept) from the replication stats handle: aggregate failed count and size per target plus in-queue counters. RustFS records failed replications as aggregate counters and persists MRF entries without a runtime query API, so per-object MRF rows are not enumerable; PerObjectEntriesAvailable is always false to make that explicit. Batch jobs (backlog#613): - POST /v3/start-job, GET /v3/list-jobs, GET /v3/status-job, GET /v3/describe-job, DELETE /v3/cancel-job. - RustFS has no batch-job execution engine (no scheduler, queue, or worker), so these implement MinIO request/response/error semantics only and never fake execution or success: start-job parses and validates the job definition then returns NotImplemented for known job types and InvalidRequest for unknown ones; list-jobs returns an empty list; status/describe/cancel validate jobId and return a no-such-job error. All routes are registered with admin auth using existing AdminAction variants (ReplicationDiff, GetReplicationMetricsAction, StartBatchJobAction, ListBatchJobsAction, DescribeBatchJobAction, CancelBatchJobAction) and are covered by the admin route-registration matrix and new handler unit tests. Refs rustfs/backlog#611 #613 |
||
|
|
511ad31ba0 |
fix(s3): normalize root double-slash ListBuckets requests (#4336)
test(s3): MinIO parity for double-slash ListBuckets, invalid copy-source date, region-aware CreateBucket - Add DoubleSlashListBucketsCompatLayer: rewrite `GET //` to `GET /` so it routes to ListBuckets, matching MinIO browser-client behavior (#601) - Add e2e regression coverage for invalid copy-source conditional date headers (#618) and region-aware MakeBucket(us-east-1) (#629) Refs rustfs/backlog#601 #618 #629 |
||
|
|
eb02486574 |
chore(swift): remove placeholder SSE module (#4330)
security(swift): remove placeholder SSE module (backlog#646)
The Swift `encryption` module was a non-functional stub: `encrypt_data`
returned the plaintext unchanged while labeling it AES-256-GCM in the
object metadata, and `generate_iv` derived the IV from a timestamp
rather than a CSPRNG. It had no production caller (only a `pub mod`
declaration and one integration test), so wiring it in as-is would have
silently shipped plaintext advertised as ciphertext.
We are not supporting Swift server-side encryption for now, so remove
the module outright rather than keep a dangerous stub around:
- delete crates/protocols/src/swift/encryption.rs
- drop `pub mod encryption;` from swift/mod.rs
- remove the encryption case (and unused import) from the swift
integration test
The module reached main dubiously: it was introduced together with the
whole Swift API in commit
|
||
|
|
3bc8d79fe5 |
fix(multipart): serialize put_object_part per uploadId (#4329)
fix(multipart): serialize the part commit per uploadId to prevent mixed-generation shards (backlog#853) Concurrently re-transmitting the same part could land two shard generations across different disks: each shard is individually bitrot-valid, so the corruption surfaces only as a silent mix at read time. The hazard is confined to rename_part, where two temp parts are moved cross-disk onto the SAME final part path — interleaving there can leave shards from two generations. Each concurrent stream already writes to its own unique temp dir, so the encode/stream phase never conflicts and must stay lock-free: holding a lock across it would serialize slow re-transmits of the same part, break the S3 'last finisher wins' semantics, and cause UploadPart lock-acquire timeouts (ServiceUnavailable). Scope a write lock to the uploadId namespace around only the rename_part commit, so each commit is atomic across disks and the last committer wins consistently. Mirrors MinIO's per-uploadID NS lock; disjoint from the object lock held by complete_multipart_upload (no lock-ordering cycle). Honors opts.no_lock. The pre-delete-before-rename half of backlog#853 was already fixed in #4316; this completes the issue. Adds an end-to-end regression test: six concurrent resends of the same part must all succeed (no lock-acquire timeout), exactly one generation stays visible, and the reassembled object equals one intact generation. Closes rustfs/backlog#853 |
||
|
|
6d0583872b |
test(ecstore): serialize list_objects mutation-sequence tests (#4338)
Three tests share the global list-objects mutation-sequence state (the LIST_OBJECTS_MUTATION_SEQUENCE atomic and the per-bucket sequence map) and all call reset_list_objects_mutation_sequences_for_test(), but only observed_mutation_persists_namespace_journal_high_water was marked #[serial_test::serial]. The serial lock only excludes tests that also carry the attribute, so the unmarked tests still ran concurrently with it: the journal test's persist of "bucket" -> 11 could land after the recovery test's reset, making it observe 11 instead of the expected 9 (flaky under the default multi-threaded test runner, green with --test-threads=1). Add #[serial_test::serial] to the two unmarked tests: - persistent_key_only_index_load_recovers_mutation_sequence_from_journal - list_objects_mutation_sequence_advances_per_bucket Unique bucket names alone would not fix this: the per-bucket assertions in list_objects_mutation_sequence_advances_per_bucket check exact values produced by the single global atomic counter, which any concurrent observe call perturbs, and every reset zeroes that global state for whichever peer is mid-flight. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
75fab0a842 |
fix(filemeta): satisfy clippy::field_reassign_with_default in replication_info_equals test (#4335)
fix(filemeta): use struct-init to satisfy clippy::field_reassign_with_default The replication_info_equals test constructed FileInfo via `let mut x = FileInfo::default()` followed by field reassignment, which clippy::field_reassign_with_default (deny-by-default under -D warnings) rejects on the lib-test target, breaking `Test and Lint` CI on every PR. Rewrite the three constructions using struct-init + ..Default::default(). Test-only change; behavior unchanged. |
||
|
|
25193ee6cf |
feat(listobjects): add guarded metadata-fast listing (#4311)
* feat(listobjects): add phase 0 observability metrics * perf(listobjects): reduce gather metadata decoding * test(listobjects): add adversarial listing coverage * feat(listobjects): add source-aware cursor fields * feat(listobjects): define index source modes * feat(listobjects): model index rebuild health * feat(listobjects): add opt-in index fallback gate * feat(listobjects): verify index candidate pages * docs(listobjects): add rollout runbook * docs(listobjects): untrack baseline fixtures * feat(listobjects): add opt-in key-only provider * feat(listobjects): add index verification metrics * perf(listobjects): gate list metrics on get stage switch * feat(listobjects): bind provider health generation * feat(listobjects): add persistent key-only provider * feat(listobjects): bind persistent provider lifecycle * feat(listobjects): track persistent index mutations * feat(listobjects): persist index mutation checkpoints * fix(listobjects): aggregate benchmark metric snapshots * feat(listobjects): store journal in system namespace * chore(listobjects): report fallback reasons in bench * test(listobjects): verify metadata-fast stale fallbacks * feat(listobjects): serve metadata-fast snapshots behind guardrails * test(listobjects): add metadata-fast chaos bench * fix(listobjects): attribute metadata-fast fallback metrics * test(listobjects): add metadata-fast fallback chaos probes * test(listobjects): add quorum journal chaos guardrails * fix(listobjects): satisfy pre-pr clippy gate * docs(listobjects): untrack rollout runbook * fix(ecstore): remove redundant heal error conversion * test(filemeta): satisfy replication info clippy ---- Co-Authored-By: heihutu <heihutu@gmail.com> |
||
|
|
0271abc14d |
docs: add MinIO compatibility router + file-format docs (#4328)
Add two durable architecture references grounded in current code: - minio-rustfs-router-compatibility.md: MinIO cmd/api-router.go (S3 object/bucket) and cmd/admin-router.go (admin /v3, /v4) vs RustFS implementation status, with per-row landing points and a gaps-only checklist of still-missing admin endpoints (profiling, healthinfo, LDAP IDP config, replication diff, MRF, batch, locks, speedtest, log stream, top, trace). - minio-file-format-compat.md: xl.meta (meta_ver 3 write, <=3 read, XL2 magic, rs-vandermonde, HighwayHash256 bitrot, inline data) and .metadata.bin bucket-metadata interop matrix for the backlog#580 items, plus old-RustFS -> new migration and a phased plan. Cross-links s3-compatibility-matrix.md and admin-route-action-snapshot.md and indexes both docs in docs/architecture/README.md. Refs rustfs/backlog#596 rustfs/backlog#603 rustfs/backlog#580 |
||
|
|
a2d679cdb6 |
fix(bucket): propagate bucket deletion to peer metadata caches (backlog#646) (#4326)
The peer `DeleteBucketMetadata` RPC handler was a stub that returned success without doing anything, and the delete-bucket flow never sent the notification in the first place. As a result, after a bucket was deleted other nodes kept serving its stale cached metadata. Wire the whole path end to end: - ecstore: add an in-memory `remove_bucket_metadata` (free fn) and `BucketMetadataSys::remove`, the counterpart to `set_bucket_metadata`, and export it through the `api::bucket::metadata_sys` facade. - node_service: `handle_delete_bucket_metadata` now validates the bucket name and actually drops the cached metadata for it. - bucket_usecase: after a successful delete_bucket, notify peers via `notification_sys.delete_bucket_metadata` in the background, symmetric to the existing `notify_bucket_metadata_reload` path. Also update the delete-bucket-metadata unit test to assert the empty-bucket rejection instead of the old always-success stub, and drop an unused `tracing::debug` test import left over from #4322. Verified: cargo fmt; cargo check -p rustfs-ecstore; cargo test -p rustfs --lib --features rio-v2 test_delete_bucket_metadata_empty_bucket; arch guardrail scripts pass. |
||
|
|
655c5cb403 |
fix(ecstore): reject short-read shards and advance ParallelReader per stripe (backlog#799 B2) (#4327)
Two compounding defects let a truncated shard corrupt a GET (ranged GET could return HTTP 200 with wrong bytes): - `BitrotReader::read` returned `Ok(short_len)` when the shard stream hit EOF before filling the caller's buffer. With `skip_verify` / `HashAlgorithm::None` / parity=0 there is no hash to catch it, so the short shard was accepted and every downstream byte shifted. - `ParallelReader.offset` was set once and never advanced per stripe, so every stripe after the first reused the first stripe's geometry and the last-stripe length clamp was wrong. Fix both (they are mutually required): - `BitrotReader::read` now errors (`UnexpectedEof`) on a short read, before and independent of the bitrot hash check, so it fires under skip-verify/no-hash too. The caller sizes the buffer to the expected per-stripe shard length, so "buffer not filled" == "shard truncated". A short read routes through the existing `errs[i]` path, dropping that reader from the stripe so parity reconstruction engages; with parity=0 the stripe fails read quorum and the GET errors loudly instead of streaming shifted bytes. Mirrors MinIO's `parallelReader.Read` (`n != shardSize` -> reader failed). - `ParallelReader::read` / `read_lockstep` advance `self.offset += shard_size` per stripe so the per-stripe expected length (incl. the shorter final stripe) is exact, matching the correct pattern already used by heal. Updates three bitrot tests that used an obsolete oversized-buffer pattern to size per-stripe (as the real decode/heal paths do), and adds a regression test that a truncated shard errors under None/HighwayHash + skip_verify. Refs backlog#799 (B2), issue rustfs/backlog#851. Design converged by two independent expert reviews referencing MinIO cmd/erasure-decode.go. |
||
|
|
6297d112c3 |
fix: auto-repair breaking changes from 1b3727e2 (#4324)
fix(heal): remove useless .into() conversion in heal error path Clippy flagged a useless_conversion lint at heal.rs:491 where is redundant because is already of type . |
||
|
|
92596da7fa |
fix(filemeta): key round-tripped replication reset state by canonical header (backlog#799 B16) (#4325)
fix(filemeta): key round-tripped replication reset state by canonical header (backlog#799 B19->B16) `get_internal_replication_state` stored the reset status under the bare ARN after stripping the internal prefix, while the write and lookup sides (`get_replication_state` / `ReplicationState::target_state`) use the full `target_reset_header(arn)` key. A `target_state` fallback masked the lookup miss, but the map stayed keyed inconsistently (bare on read, full on write), which can drop reset state across merge/reflatten cycles. Store the canonical `target_reset_header(arn)` key on read so the map is consistent everywhere; the lookup fallback becomes belt-and-suspenders rather than load-bearing. Adds a round-trip regression test. Refs backlog#799 (B16), tracked in rustfs/backlog#863. |
||
|
|
31c6859965 |
chore: converge stale TODOs and apply safe fills (backlog#646) (#4322)
Second TODO-convergence round over the current tree (backlog#646). All line numbers in the old inventory had gone stale after the set_disk / diagnostics / cluster refactors, so this re-scans and reduces the marker count from 144 to 99. STALE removals (comment describes already-implemented behavior, or dead commented-out blocks) across ecstore (set_disk ops/core, store, cluster/rpc, bucket/metadata_sys, services), iam, filemeta, s3select and rustfs auth/object_usecase. No behavior change. Safe fills, each verified: - filemeta: replication_info_equals now also compares replication_state_internal (function currently has no callers; adds a regression test). - bitrot: drop the confirmed-unused `_want` parameter from bitrot_verify and the now-unused `sum` on LocalDisk::bitrot_verify, removing a Bytes::copy_from_slice allocation. Streaming verify uses the file's embedded per-shard hash, never the passed sum. - signer: rename v4_ignored_headers -> V4_IGNORED_HEADERS and drop the non_upper_case_globals allow. - admin/heal: test_decode was #[ignore]d and used serde_urlencoded on a JSON body (would panic); rewire to serde_json::from_slice to match the production decode path, add assertions, un-ignore. Verified: cargo fmt; cargo check on touched crates; tests pass (filemeta, signer, bitrot, heal::test_decode); arch guardrail scripts pass. |
||
|
|
5f1759eb3c | ci: cancel PR workflows on close (#4323) | ||
|
|
83edafb11f |
fix(ecstore): reject renewing a mis-mounted drive into the wrong set (backlog#799 B19) (#4321)
fix(ecstore): reject renewing a misplaced drive into the wrong set (backlog#799 B19) `renew_disk` located the drive's (set, disk) position from its own format via `find_disk_index`, but never checked that the resolved `set_idx` matched this `SetDisks`' own `set_index` before inserting the drive into `self.disks`. A drive whose format places it in another set would be claimed by this set too, so two sets could manage the same drive and degrade together. Skip (with a warning) when `set_idx != self.set_index`. Refs backlog#799 (B19), tracked in rustfs/backlog#863. |
||
|
|
5c5f8063d8 |
fix(replication): log (not swallow) resync status persistence failure (backlog#799 B23) (#4320)
fix(replication): don't silently swallow resync status persistence failure (backlog#799 B23) After a resync computes a new replication status, it persists it via `put_object_metadata` but discarded the `Err` case (`if let Ok(u) = ...`). A failure left the object's on-disk replication status disagreeing with the resync result with no signal at all. Log the failure at warn level instead. Refs backlog#799 (B23), tracked in rustfs/backlog#863. |
||
|
|
1b3727e2c4 |
fix(heal): clean up .rustfs/tmp healed shards on all failure paths (backlog#799 B20) (#4319)
heal_object wrote healed shards to .rustfs/tmp/<uuid>/ and only removed that tmp dir on the success path (the final delete_all). Three early exits after the tmp shards were written leaked the dir: - `erasure.heal(...)?` failing midway, - the `disks_to_heal_count == 0` early return, and - the `?` on the post-rename remote data-dir delete. Add the tmp cleanup before the first two, and downgrade the remote data-dir cleanup failure to a warning (the healed shard is already renamed into place, so that cleanup failing must not abort the heal or leak the tmp shards). Refs backlog#799 (B20), tracked in rustfs/backlog#863. |
||
|
|
849ea9a122 | fix(site-replication): align IAM and bucket metadata replication (#4318) | ||
|
|
5c67c508cb |
fix(filemeta): skip unknown fields when decoding MetaDeleteMarker (backlog#799 B17) (#4317)
`MetaDeleteMarker::decode_from` returned an error on any field key it didn't recognize, unlike `MetaObject::decode_from` / `MetaObjectV1::decode_from`, which skip unknown fields. That breaks forward compatibility: a delete marker written by a newer version with an extra key fails to decode on an older binary. Skip the unknown field's value (`skip_msgp_value`) and continue, matching the object decoders. Adds a regression test. Refs backlog#799 (B17), tracked in rustfs/backlog#863. |
||
|
|
8ffe230a89 |
fix(multipart): don't pre-delete the committed part before rename_part (backlog#853) (#4316)
`rename_part` unconditionally ran `cleanup_multipart_path([part.N, part.N.meta])` on every disk *before* the per-disk rename fan-out. The per-disk `rename_part` already replaces `part.N` atomically (std::fs::rename) and rewrites `part.N.meta`, so the pre-delete is redundant — and destructive: it removed an already-committed (ACKed) part on all disks before the new rename landed, so a re-upload of the same part that then failed write quorum destroyed the committed part outright. Remove the pre-rename cleanup and rely on the atomic rename to overwrite in place; the quorum-failure rollback below is unchanged. No behaviour change on the success paths (first upload / retry both end with part.N replaced), verified by the existing multipart suite. Refs backlog#799 (B4). The remaining half of B4 — serializing concurrent same-part uploads with an uploadId-scoped lock so two generations can't be mixed across disks — is a larger concurrency change tracked as a follow-up on the issue. |
||
|
|
5e73d59eb6 |
fix(replication): re-derive replication decision when replaying MRF delete entries (backlog#858) (#4315)
MRF delete replay reconstructed the delete with `..Default::default()`, leaving `replication_state = None`. `replicate_delete` derives its target set purely from `replication_state.replicate_decision_str`, so an empty state produced an empty decision -> zero targets: the replayed delete contacted no remote at all, a silent no-op that left replicas permanently diverged after a restart. The MRF entry doesn't persist the decision and the source object is already gone, so re-derive it from the live bucket config at replay time via `check_replicate_delete` — mirroring the object heal path (`get_heal_replicate_object_info`) — and set it on the reconstructed delete's `replication_state`. This is a contained fix with no change to the on-disk MRF format. Refs backlog#799 (B9). Note: the source delete-marker mtime is still not persisted in the MRF entry, so a replayed delete marker is stamped with the replay time on the target. That is a separate, minor consistency nuance (the delete now propagates correctly) and can be addressed by extending the MRF entry format in a follow-up. |
||
|
|
f33ae8e9af |
fix(replication): keep MRF persister backlog cumulative so flushes don't drop entries (backlog#859) (#4314)
The MRF persister accumulated overflow entries in `pending`, flushed them with `flush_mrf_to_disk`, and cleared `pending` on success. But `flush_mrf_to_disk` *overwrites* the whole MRF file with exactly the entries passed. After flushing batch A (file = A) and clearing, the next flush wrote batch B and thereby overwrote the file to contain only B — and the MRF file is only replayed (and cleared) at startup, never during the run, so batch A's entries were silently lost. A crash after the B flush lost all of batch A's pending replications. Keep `pending` cumulative (the file must hold the full set of overflow entries for the run) and rewrite the whole set on each flush instead of clearing after success: - flush eagerly once 1 000 *new* entries accumulate since the last write (measured against the flushed length, so a large backlog isn't rewritten on every add), and on the 10s tick when dirty; - bound the in-memory/on-disk backlog with `MRF_PENDING_CAP` (200 000) and log once when the cap is hit rather than growing without limit. Refs backlog#799 (B10). |
||
|
|
650a3e5734 |
fix(replication): classify resync verification HEAD errors instead of miscounting (backlog#862) (#4312)
The resync result verification HEADed the target after replicating and counted the outcome with inverted error handling: - for a delete marker, ANY HEAD error (timeout, 5xx, auth, malformed) was counted as replicated (success); - for a versioned object against an AWS-style target, HEAD was sent with the RustFS UUID versionId, which AWS rejects with 400, so a well-replicated object was counted as failed. Classify the error before counting: - delete marker: only a definitive 404/NoSuchKey or 405/MethodNotAllowed confirms the marker propagated (`is_retryable_delete_replication_head_error` == false); any retryable/ambiguous error now counts as failed; - versioned object with a version-id-format rejection: re-verify via `head_object_fallback` (versionId-less HEAD) before deciding — present -> replicated, absent/error -> failed; - all other errors: failed, as before. Reuses the existing, unit-tested classifier helpers. Verified against the existing resyncer suite (24 tests). Refs backlog#799 (B13). |
||
|
|
32845e03b0 |
fix(replication): mark version-id fallback ETag match as Completed, not Failed (backlog#860) (#4310)
During resync against an AWS-style target that rejects RustFS UUID versionIds,
the code retries HEAD without a versionId and compares ETags. On a match it set
`replication_action = None` ("already in sync, nothing to copy") but did not
return, so control fell through to `if replication_action != ReplicationAction::All`
— a branch meant only for the unsupported metadata-only case — and stamped the
object FAILED with "metadata-only replication is not implemented". The target
already held an identical object, yet the source recorded FAILED forever, so
AWS-style targets never converged and the MRF kept re-queuing.
Handle `ReplicationAction::None` explicitly before that branch: record it as
Completed (with the resync timestamp/`replication_resynced` bookkeeping for
ExistingObject + reset_id, mirroring the HEAD-success None path) and return.
Only `ReplicationAction::Metadata` now takes the metadata-unsupported failure
branch; `All` still proceeds to the copy. This path is the only way `None`
reaches that point (the HEAD-success None case already returns earlier).
Refs backlog#799 (B11).
|
||
|
|
6c9efc8064 | fix(heal): treat no-heal-required format results as noop (#4301) | ||
|
|
1cc1fc0f83 |
fix(ecstore): exclude failed-shard disks from commit so write quorum isn't inflated (backlog#852) (#4309)
`MultiWriter` recorded per-writer failures only in a private `errs` vector and nulled the failed writer locally, but `put_object` never used that to prune the disk set: `rename_data` ran over the full `shuffle_disks`, so a disk that took a short/failed write still had its truncated shard renamed into place and counted as an online disk. The object then claimed N good shards while one was short/corrupt, so a single later disk failure could drop it below reconstructable quorum — silent data loss. - `write_shard`: null the writer on a generic write error too (not only on `ShortWrite`), so a failed writer is uniformly represented as `None` — matching `shutdown_writer`, which already does this. - `put_object`: after encode, drop every disk whose writer failed (`drop_failed_writer_disks`) from `shuffle_disks` before `rename_data`, and re-check write quorum over the survivors. `rename_data` already re-checks quorum and rolls back if too few disks remain, so excluded disks are neither committed nor counted (MinIO sets failed writers to nil before `renameData`). Adds unit tests for the exclusion/quorum accounting. Refs backlog#799 (B3). |
||
|
|
c3f54f95ed |
fix(heal): preserve resume state and retry when heal objects fail (backlog#855) (#4308)
fix(heal): don't mark set healed / discard resume state when objects failed (backlog#855)
`execute_heal_with_resume` unconditionally called `mark_completed()` and
returned `Ok(())` after the bucket loop, even when objects failed. The caller
then treats `Ok` as success and cleans up the resume + checkpoint state, so a
heal run with per-object failures was reported as a clean completion and its
state (including the failed set) was destroyed with no retry.
Finalize based on the failure count instead:
- failed_objects == 0 -> mark completed (unchanged);
- failed_objects > 0 with retry budget left -> `schedule_retry()` (bump the
bounded retry counter + reset per-pass progress for a full re-scan) and
return Err, so `heal_erasure_set` preserves the resume/checkpoint state and
the caller keeps the healing markers for the next run;
- failed_objects > 0 with retries exhausted -> drop the resume state (no
zombie task) but still return Err so the markers survive for a later heal
cycle / the background scanner. Never silently claim a clean completion.
The failure identities are not persisted across pages (the per-page sets are
pruned in `complete_page`), so a retry is a bounded full re-scan; healed
objects are skipped quickly on the normal-scan pass.
Adds `ResumeState::reset_for_retry`, `ResumeManager::schedule_retry`,
`ResumeCheckpoint::reset_for_retry`, `CheckpointManager::reset_for_retry`, and
regression tests. This activates the failure path the caller already documents
at task.rs ("Keep the markers on failure ... the next run re-marks and
eventually clears them"), which was previously dead because heal never
returned Err on object failures.
Refs backlog#799 (B6).
|
||
|
|
7975f26b90 |
fix(ecstore): reach orphan-dir purge on nil-version delete miss (#4189) (#4307)
PR #4220 added a purge for orphan empty-directory trees (folder keys with no xl.meta) on the delete path, but the guard only accepted object-not-found. Over the real HTTP DELETE path the guard is never reached: `del_opts` pins `version_id = Uuid::nil()` for directory keys, so the missing dir object fails the specific-version lookup with version-not-found (FileVersionNotFound), not object-not-found. The guard short-circuits, the store returns the error, and the API layer turns it into a fake 204 — the ghost folder survives, exactly the #4189 symptom. The existing unit tests passed because they call `purge_orphan_dir_object` directly, bypassing the nil-version lookup. Accept both misses in the guard (extracted as `should_purge_orphan_dir_on_missing`) so the folder delete actually takes effect. Non-directory keys and non-miss errors (e.g. quorum failures) are unaffected; the cross-disk data-safety refusal in `purge_orphan_dir_object` is unchanged. Verified end-to-end against a running 4-disk erasure server: DELETE of a planted orphan `ghost/` tree now purges it on all drives and clears the listing, a real object under the same prefix is untouched, and a prefix holding real data on any drive is refused (all drives preserved). Adds predicate regression tests covering version-not-found / object-not-found on directory keys, and the negative cases. |
||
|
|
be52e35a1f |
test(table-catalog): add vendor compatibility audit (#4299)
* test(table-catalog): add vendor compatibility audit * fix(table-catalog): include OSS data-plane endpoint --------- Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
|
b09526c0f5 | feat(helm): allow overriding Kubernetes cluster domain (#4259) |