refactor(storage): narrow table catalog bounds (#3350)

This commit is contained in:
安正超
2026-06-11 14:32:28 +08:00
committed by GitHub
parent 2ead90d31b
commit dd50359161
4 changed files with 90 additions and 40 deletions
+18
View File
@@ -172,6 +172,23 @@ pub trait HealOperations: Send + Sync + Debug {
async fn check_abandoned_parts(&self, bucket: &str, object: &str, opts: &HealOpts) -> Result<()>; async fn check_abandoned_parts(&self, bucket: &str, object: &str, opts: &HealOpts) -> Result<()>;
} }
/// Namespace lock operations needed by consumers that only coordinate object
/// mutations but do not require the full storage API surface.
#[async_trait::async_trait]
pub trait NamespaceLocking: Send + Sync + Debug + 'static {
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper>;
}
#[async_trait::async_trait]
impl<T> NamespaceLocking for T
where
T: StorageAPI + ?Sized + 'static,
{
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper> {
StorageAPI::new_ns_lock(self, bucket, object).await
}
}
/// Unified storage API combining all operation groups. /// Unified storage API combining all operation groups.
/// ///
/// Consumers can depend on specific sub-traits (e.g., `BucketOperations`) /// Consumers can depend on specific sub-traits (e.g., `BucketOperations`)
@@ -181,5 +198,6 @@ pub trait HealOperations: Send + Sync + Debug {
pub trait StorageAPI: pub trait StorageAPI:
ObjectIO + BucketOperations + ObjectOperations + ListOperations + MultipartOperations + HealOperations + Debug ObjectIO + BucketOperations + ObjectOperations + ListOperations + MultipartOperations + HealOperations + Debug
{ {
// RUSTFS_COMPAT_TODO(API-012): keep old StorageAPI lock callers compiling while namespace-lock-only consumers migrate. Remove after namespace-lock-only consumers depend on NamespaceLocking and StorageAPI no longer owns namespace locking.
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper>; async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper>;
} }
@@ -24,6 +24,12 @@ for later deletion.
- Why: old `ecstore::store_api` bucket DTO import paths must keep compiling while storage API consumers migrate. - Why: old `ecstore::store_api` bucket DTO import paths must keep compiling while storage API consumers migrate.
- Removal condition: remove after all consumers import bucket DTOs from the storage API crate. - Removal condition: remove after all consumers import bucket DTOs from the storage API crate.
- Status: planned cleanup. - Status: planned cleanup.
- `RUSTFS_COMPAT_TODO(API-012)`
- Task: `API-012`
- File: `crates/ecstore/src/store_api/traits.rs`
- Why: old `StorageAPI::new_ns_lock` callers must keep compiling while namespace-lock-only consumers migrate to NamespaceLocking.
- Removal condition: remove after all namespace-lock-only consumers depend on NamespaceLocking and StorageAPI no longer owns namespace lock capability.
- Status: planned cleanup.
## Review Checklist ## Review Checklist
+63 -37
View File
@@ -5,15 +5,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
## Current Context ## Current Context
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
- Branch: `overtrue/arch-storage-api-helper-cleanup` - Branch: `overtrue/arch-table-catalog-bounds`
- Baseline: `origin/main` at `2f7cc7cb9e5891b46db6e686c67636e9343762ce` - Baseline: `origin/main` at `2ead90d31bd713b3b36812ed718bd44e28e522d2`
- PR type for this branch: `dependency-migration` - PR type for this branch: `dependency-migration`
- Runtime behavior changes: none. - Runtime behavior changes: none.
- Rust code changes: narrow scanner data-usage cache load/save helper generic - Rust code changes: add a narrow `NamespaceLocking` operation-group trait and
bounds away from full `StorageAPI` where helpers only need object I/O. narrow the table catalog object backend away from full `StorageAPI` when it
only needs object I/O, object operations, listing, and namespace locking.
- CI/script changes: none. - CI/script changes: none.
- Docs changes: record API-010 completion and the current scanner cache - Docs changes: record API-011 completion, the current table catalog
bound-narrowing context, verification evidence, and expert review outcomes. bound-narrowing context, API-012 compatibility cleanup marker, verification
evidence, and expert review outcomes.
## Phase 0 Tasks ## Phase 0 Tasks
@@ -243,10 +245,10 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
replication, object lookups, and scheduling behavior remain on full replication, object lookups, and scheduling behavior remain on full
`StorageAPI` where needed. `StorageAPI` where needed.
- [~] `API-011` Narrow scanner cache helper storage bounds. - [x] `API-011` Narrow scanner cache helper storage bounds.
- Current branch slice: narrow scanner data-usage cache load/save and cache - Completed slice: `rustfs/rustfs#3348` narrowed scanner data-usage cache
snapshot persistence helper bounds away from full `StorageAPI` when the load/save and cache snapshot persistence helper bounds away from full
helper only needs `ObjectIO`. `StorageAPI` when the helper only needs `ObjectIO`.
- Acceptance: scanner cache persistence helpers express object-I/O-only - Acceptance: scanner cache persistence helpers express object-I/O-only
requirements, while scanner cycle orchestration, bucket scanning, local disk requirements, while scanner cycle orchestration, bucket scanning, local disk
selection, cache publication, and storage hot paths remain unchanged. selection, cache publication, and storage hot paths remain unchanged.
@@ -257,9 +259,31 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
- Risk defense: do not move traits to `rustfs-storage-api`, do not remove - Risk defense: do not move traits to `rustfs-storage-api`, do not remove
`StorageAPI`, do not alter helper bodies, and do not narrow scanner paths `StorageAPI`, do not alter helper bodies, and do not narrow scanner paths
that need bucket operations, disk inventory, or full storage orchestration. that need bucket operations, disk inventory, or full storage orchestration.
- Verification: focused compile/tests, migration guards, and Rust risk scan - Verification: focused compile/tests, migration guards, Rust risk scan, and
passed; required quality/architecture, migration-preservation, and required quality/architecture, migration-preservation, and
testing/verification review is pending before push. testing/verification review passed.
- [~] `API-012` Narrow table catalog object backend bounds.
- Current branch slice: add a narrow `NamespaceLocking` operation-group trait
as a compatibility facade over `StorageAPI::new_ns_lock`, then narrow
`EcStoreTableCatalogObjectBackend` from full `StorageAPI` to `ObjectIO`,
`ObjectOperations`, `ListOperations`, and `NamespaceLocking`.
- Acceptance: table catalog object backend contracts express the actual
object read/write, metadata/delete, list, and namespace-lock capabilities
they need, while table catalog store logic and lock behavior remain
unchanged.
- Must preserve: table catalog object paths, metadata pointer semantics,
optimistic write preconditions, object listing pagination, missing-object
handling, namespace write-lock acquisition, `StorageAPI::new_ns_lock`
compatibility, object APIs, scanner/heal/replication/config persistence,
and storage hot paths.
- Risk defense: do not remove `StorageAPI::new_ns_lock`, do not move traits
into `rustfs-storage-api`, do not change lock implementation code, do not
alter table catalog method bodies, and track the retained old lock method
with `RUSTFS_COMPAT_TODO(API-012)`.
- Verification: focused compile/tests, migration guards, Rust risk scan, and
required quality/architecture, migration-preservation, and
testing/verification review passed.
## Phase 8 Background Controller Tasks ## Phase 8 Background Controller Tasks
@@ -284,8 +308,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
## Next PRs ## Next PRs
1. `dependency-migration`: narrow scanner data-usage cache helper bounds away 1. `dependency-migration`: narrow table catalog object backend bounds away
from full `StorageAPI` where the helper only needs `ObjectIO`. from full `StorageAPI` by depending only on object I/O, object operations,
listing, and namespace locking.
2. `api-extraction`: move only the pure server-config model into 2. `api-extraction`: move only the pure server-config model into
rustfs-config as CFG-003. rustfs-config as CFG-003.
3. `api-extraction`: keep the old rustfs_ecstore::config::* path with 3. `api-extraction`: keep the old rustfs_ecstore::config::* path with
@@ -301,18 +326,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
| Expert | Status | Notes | | Expert | Status | Notes |
|---|---|---| |---|---|---|
| Quality/architecture | pass | Confirmed the Rust diff only narrows scanner cache helper bounds from `StorageAPI` to `ObjectIO`; helper bodies, scanner orchestration, bucket/disk/lifecycle/replication paths, and storage hot paths are unchanged. | | Quality/architecture | pass | Confirmed the Rust diff adds only the narrow `NamespaceLocking` facade, tracks retained `StorageAPI::new_ns_lock` with `RUSTFS_COMPAT_TODO(API-012)`, and narrows table catalog backend bounds; no table catalog method body, lock implementation, object operation, or hot-path behavior changes. |
| Migration preservation | pass | Confirmed `ObjectIO` covers the actual `get_object_reader` and `put_object` calls, while cache wire format, object paths, backup behavior, retry/timeouts, metrics, publish/update flow, scanner scheduling, concurrency, and bucket scanning remain unchanged. | | Migration preservation | pass | Confirmed the blanket facade keeps existing `StorageAPI::new_ns_lock` compatibility and table catalog object paths, optimistic preconditions, pagination, missing-object mapping, and write-lock behavior remain unchanged. |
| Testing/verification | pass | Confirmed the focused compile, `data_usage` and `scanner_io` tests, migration guards, diff hygiene, and added-line risk scan are sufficient for this generic-bound-only slice, so full pre-commit can be skipped under the current larger-granularity instruction. | | Testing/verification | pass | Confirmed focused compile/tests, migration guards, diff hygiene, and added-line risk scan are sufficient for this dependency-boundary slice before push; full pre-commit is skipped under the current larger-granularity instruction. |
## Verification Notes ## Verification Notes
Passed: Passed:
- `cargo fmt --all`. - `cargo fmt --all`.
- `cargo check -p rustfs-scanner -p rustfs-ecstore -p rustfs --lib`. - `cargo check -p rustfs-ecstore -p rustfs --lib`.
- `cargo fmt --all --check`. - `cargo fmt --all --check`.
- `cargo test -p rustfs-scanner data_usage --lib`; 21 passed. - `cargo test -p rustfs table_catalog --lib`; 84 passed.
- `cargo test -p rustfs-scanner scanner_io --lib`; 18 passed.
- `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_architecture_migration_rules.sh`.
- `./scripts/check_layer_dependencies.sh`. - `./scripts/check_layer_dependencies.sh`.
- `./scripts/check_metrics_migration_refs.sh`. - `./scripts/check_metrics_migration_refs.sh`.
@@ -327,25 +351,27 @@ Passed:
Notes: Notes:
- Full pre-commit is intentionally skipped when the focused tests and guards - Full pre-commit is intentionally skipped when the focused tests and guards
pass, per the current instruction to increase PR granularity. pass, per the current instruction to increase PR granularity.
- This slice changes generic bounds and imports only; helper bodies, cache - This slice changes trait contracts, imports, and generic bounds only; table
object paths, backup paths, retry/timeout behavior, metrics, and publish catalog helper bodies, object paths, optimistic write preconditions, list
logic are unchanged. pagination, missing-object mapping, and write-lock behavior are unchanged.
- Scanner orchestration paths intentionally still require full `StorageAPI` - `StorageAPI::new_ns_lock` intentionally remains in place for compatibility;
where they use bucket operations, disk inventory, or broader storage the new `NamespaceLocking` facade only lets narrower consumers state the
orchestration. capability they actually use.
- No temporary compatibility shim was added. - The retained old `StorageAPI::new_ns_lock` surface is marked with
`RUSTFS_COMPAT_TODO(API-012)` and registered in
[`compat-cleanup-register.md`](compat-cleanup-register.md).
## Handoff Notes ## Handoff Notes
- Keep this API-011 slice as a `dependency-migration` PR that only narrows - Keep this API-012 slice as a `dependency-migration` PR that only adds the
scanner data-usage cache helper generic bounds. narrow namespace-locking facade and narrows table catalog backend bounds.
- Do not remove `StorageAPI` itself, object operation traits, or - Do not remove `StorageAPI` itself, object operation traits, or
`new_ns_lock` in this PR. `StorageAPI::new_ns_lock` in this PR.
- Do not move traits into `rustfs-storage-api` or introduce new compatibility - Do not move traits into `rustfs-storage-api` or introduce additional
shims in this PR. compatibility shims in this PR.
- Do not alter data-usage cache wire format, cache object paths, backup paths, - Do not alter table catalog object paths, metadata pointer semantics,
retry/timeout behavior, metrics, update publication, scanner cycle optimistic write preconditions, object listing pagination, missing-object
orchestration, bucket scanning, disk scan concurrency, heal object repair, handling, namespace write-lock acquisition, scanner/heal/replication/config
object APIs, or storage hot-path consumers in this PR. persistence paths, object APIs, or storage hot-path consumers in this PR.
- Do not add temporary compatibility code unless a matching - Do not add temporary compatibility code unless a matching
`RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry are added. `RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry are added.
+3 -3
View File
@@ -38,7 +38,7 @@ use rustfs_ecstore::disk::RUSTFS_META_BUCKET;
use rustfs_ecstore::error::StorageError; use rustfs_ecstore::error::StorageError;
use rustfs_ecstore::{ use rustfs_ecstore::{
set_disk::get_lock_acquire_timeout, set_disk::get_lock_acquire_timeout,
store_api::{HTTPPreconditions, ObjectOptions, PutObjReader, StorageAPI}, store_api::{HTTPPreconditions, ListOperations, NamespaceLocking, ObjectIO, ObjectOperations, ObjectOptions, PutObjReader},
}; };
use serde::{Deserialize, Serialize, de::DeserializeOwned}; use serde::{Deserialize, Serialize, de::DeserializeOwned};
use time::{Duration, OffsetDateTime}; use time::{Duration, OffsetDateTime};
@@ -1364,7 +1364,7 @@ impl<S> Clone for EcStoreTableCatalogObjectBackend<S> {
impl<S> EcStoreTableCatalogObjectBackend<S> impl<S> EcStoreTableCatalogObjectBackend<S>
where where
S: StorageAPI, S: ObjectIO + ObjectOperations + ListOperations + NamespaceLocking,
{ {
pub fn new(store: Arc<S>) -> Self { pub fn new(store: Arc<S>) -> Self {
Self { store } Self { store }
@@ -1376,7 +1376,7 @@ pub(crate) type EcStoreTableCatalogStore<S> = ObjectTableCatalogStore<EcStoreTab
#[async_trait::async_trait] #[async_trait::async_trait]
impl<S> TableCatalogObjectBackend for EcStoreTableCatalogObjectBackend<S> impl<S> TableCatalogObjectBackend for EcStoreTableCatalogObjectBackend<S>
where where
S: StorageAPI, S: ObjectIO + ObjectOperations + ListOperations + NamespaceLocking,
{ {
async fn read_object(&self, bucket: &str, object: &str) -> TableCatalogStoreResult<Option<TableCatalogObject>> { async fn read_object(&self, bucket: &str, object: &str) -> TableCatalogStoreResult<Option<TableCatalogObject>> {
let info = match self.store.get_object_info(bucket, object, &ObjectOptions::default()).await { let info = match self.store.get_object_info(bucket, object, &ObjectOptions::default()).await {