From 69719c257e648dedf2195b06bc41ff97e6baa121 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 14 Aug 2026 21:19:06 +0800 Subject: [PATCH] chore(ecstore): remove the pool-level ListObjects pagination copy (#6078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(ecstore): remove the pool-level ListObjects pagination copy The ListObjects pagination pipeline existed in three near-copies in one file; production listing never reaches the Sets copy, which ECStore bypasses by expanding straight to per-set disks. This removes it: impl ListOperations for Sets (61 lines of pure forwarding in core/sets.rs) and the impl Sets pagination block (826 lines of inner_list_objects_v2 / list_objects_generic / inner_list_object_versions / list_path / list_merged / walk_internal in store/list_objects.rs). Two preconditions verified before deleting rather than taken on faith: the architecture guard pins only set_disks_implements_storage_list_operations_contract, so nothing requires the Sets trait impl; and the four Sets pagination methods had no cross-file caller besides that trait impl. The single test consumer moves to the surviving pipeline instead of being deleted: writes still go through the pool, and the listing assertion now targets the set-level implementation. It is renamed accordingly so the name still describes what it covers. The logging guardrail's TRACE-only requirement for Sets::list_objects_v2 retires in the same diff — the wrapper it pinned no longer exists. The ECStore and SetDisks entries are untouched. The SetDisks copy stays for now: its trait impl is guard-pinned, so replacing the duplicate pipeline behind it needs the generic helper the issue schedules for post-1.0. Verification: cargo nextest run -p rustfs-ecstore 4020 passed; check_architecture_migration_rules.sh and check_logging_guardrails.sh pass; clippy --lib --tests -D warnings clean; make pre-commit green. Ref rustfs/backlog#1821 (PR1). * chore(ecstore): fold the ListObjects forwarders into the ECStore impl store/list.rs held two thin forwarders, handle_list_objects_v2 and handle_list_object_versions, that only re-entered the inner_* implementations. The ListOperations impl now calls those directly and the file goes away. The logging guardrail's trace_hot_spans list pinned handle_list_objects_v2 as TRACE-only; that entry is retired in the same diff, adjacent to the sets.rs entry retired by the preceding commit. Ref rustfs/backlog#1821. * chore(ecstore): drop the type aliases orphaned by the pagination removal core/sets.rs declared four local type aliases — ListObjectsV2Info, ListObjectVersionsInfo, ObjectInfoOrErr and WalkOptions — used only by the pool-level pagination pipeline removed earlier in this branch. store/list_objects.rs keeps its own live copies of the same aliases. They only surface now that #6087 removed the core module's dead_code blanket: on that older base each PR was warning-free on its own, and the combination is what exposes them. Their storage_api_contracts imports go with them. Ref rustfs/backlog#1823, rustfs/backlog#1821. * fix(ecstore): preserve Sets listing compatibility --- crates/ecstore/src/store/list.rs | 81 ------------------- crates/ecstore/src/store/list_objects.rs | 2 +- crates/ecstore/src/store/mod.rs | 7 +- .../tests/ecstore_contract_compat_test.rs | 12 +++ scripts/check_logging_guardrails.sh | 4 +- 5 files changed, 19 insertions(+), 87 deletions(-) delete mode 100644 crates/ecstore/src/store/list.rs diff --git a/crates/ecstore/src/store/list.rs b/crates/ecstore/src/store/list.rs deleted file mode 100644 index 5f7f480b5..000000000 --- a/crates/ecstore/src/store/list.rs +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2024 RustFS Team -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use super::*; - -impl ECStore { - #[instrument(level = "trace", skip(self))] - #[allow(clippy::too_many_arguments)] - pub(super) async fn handle_list_objects_v2( - self: Arc, - bucket: &str, - prefix: &str, - continuation_token: Option, - delimiter: Option, - max_keys: i32, - fetch_owner: bool, - start_after: Option, - incl_deleted: bool, - ) -> Result { - self.inner_list_objects_v2( - bucket, - prefix, - continuation_token, - delimiter, - max_keys, - fetch_owner, - start_after, - incl_deleted, - ) - .await - } - - #[instrument(skip(self))] - pub(super) async fn handle_list_object_versions( - self: Arc, - bucket: &str, - prefix: &str, - marker: Option, - version_marker: Option, - delimiter: Option, - max_keys: i32, - ) -> Result { - self.inner_list_object_versions(bucket, prefix, marker, version_marker, delimiter, max_keys) - .await - } - - pub(crate) async fn list_object_versions_for_lifecycle( - self: Arc, - bucket: &str, - prefix: &str, - marker: Option, - version_marker: Option, - delimiter: Option, - max_keys: i32, - ) -> Result { - self.inner_list_object_versions_for_lifecycle(bucket, prefix, marker, version_marker, delimiter, max_keys) - .await - } - - pub(super) async fn handle_walk( - self: Arc, - rx: CancellationToken, - bucket: &str, - prefix: &str, - result: tokio::sync::mpsc::Sender, - opts: WalkOptions, - ) -> Result<()> { - self.walk_internal(rx, bucket, prefix, result, opts).await - } -} diff --git a/crates/ecstore/src/store/list_objects.rs b/crates/ecstore/src/store/list_objects.rs index 10fc766a2..558bb7c94 100644 --- a/crates/ecstore/src/store/list_objects.rs +++ b/crates/ecstore/src/store/list_objects.rs @@ -3845,7 +3845,7 @@ impl ECStore { .await } - pub(crate) async fn inner_list_object_versions_for_lifecycle( + pub(crate) async fn list_object_versions_for_lifecycle( self: Arc, bucket: &str, prefix: &str, diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index 30a76bb69..f51fa6df5 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -148,7 +148,6 @@ mod heal_walk; pub use heal_walk::HealWalkVersion; mod init; pub(crate) mod init_format; -mod list; pub(crate) mod list_objects; mod multipart; mod object; @@ -601,7 +600,7 @@ impl crate::storage_api_contracts::list::ListOperations for ECStore { start_after: Option, incl_deleted: bool, ) -> Result { - self.handle_list_objects_v2( + self.inner_list_objects_v2( bucket, prefix, continuation_token, @@ -624,7 +623,7 @@ impl crate::storage_api_contracts::list::ListOperations for ECStore { delimiter: Option, max_keys: i32, ) -> Result { - self.handle_list_object_versions(bucket, prefix, marker, version_marker, delimiter, max_keys) + self.inner_list_object_versions(bucket, prefix, marker, version_marker, delimiter, max_keys) .await } @@ -636,7 +635,7 @@ impl crate::storage_api_contracts::list::ListOperations for ECStore { result: tokio::sync::mpsc::Sender, opts: WalkOptions, ) -> Result<()> { - self.handle_walk(rx, bucket, prefix, result, opts).await + self.walk_internal(rx, bucket, prefix, result, opts).await } } diff --git a/crates/ecstore/tests/ecstore_contract_compat_test.rs b/crates/ecstore/tests/ecstore_contract_compat_test.rs index 496eab291..d1d303f7a 100644 --- a/crates/ecstore/tests/ecstore_contract_compat_test.rs +++ b/crates/ecstore/tests/ecstore_contract_compat_test.rs @@ -157,6 +157,18 @@ fn ecstore_implements_storage_list_operations_contract() { assert!(storage_list_operations_type_name::().ends_with("::ECStore")); } +#[test] +fn ecstore_pools_expose_storage_list_operations_contract() { + fn assert_contract(store: &ECStore) { + let future = store.pools[0] + .clone() + .list_objects_v2("bucket", "", None, None, 1, false, None, false); + drop(future); + } + + let _ = assert_contract; +} + #[test] fn ecstore_implements_storage_multipart_operations_contract() { assert!(storage_multipart_operations_type_name::().ends_with("::ECStore")); diff --git a/scripts/check_logging_guardrails.sh b/scripts/check_logging_guardrails.sh index a5095d8b3..eb0640635 100755 --- a/scripts/check_logging_guardrails.sh +++ b/scripts/check_logging_guardrails.sh @@ -984,7 +984,9 @@ trace_hot_spans=( "crates/ecstore/src/store/object.rs:handle_get_object_info" "crates/ecstore/src/set_disk/ops/object.rs:get_object_info" "crates/ecstore/src/store/mod.rs:list_objects_v2" - "crates/ecstore/src/store/list.rs:handle_list_objects_v2" + # The ECStore handle_list_objects_v2 forwarder was folded into the trait impl + # above, so store/mod.rs now carries this hot path's TRACE requirement + # directly (backlog#1821). "crates/ecstore/src/core/sets.rs:list_objects_v2" "crates/ecstore/src/set_disk/ops/list.rs:list_objects_v2" "rustfs/src/app/bucket_usecase.rs:execute_list_objects_v2"