diff --git a/docs/architecture/s3-tables-support-matrix.md b/docs/architecture/s3-tables-support-matrix.md index a2c18d6fd..87fe01a0e 100644 --- a/docs/architecture/s3-tables-support-matrix.md +++ b/docs/architecture/s3-tables-support-matrix.md @@ -26,7 +26,7 @@ RustFS S3 Tables is an Iceberg REST Catalog and table-bucket implementation on t | `/_iceberg/v1` | Supported compatibility alias | MinIO AIStor-style alias; smoke profile defaults to signing name `s3tables`. | | S3 object data plane | Supported | Data, metadata, manifest, and delete files are ordinary S3 objects with table-aware policy checks on warehouse paths. | | S3 table-bucket listings | Supported | `ListObjects`, `ListObjectsV2`, `ListObjectVersions`, metadata-list extensions, and multipart-upload listings omit warehouse objects unless the caller has `admin:GetTableMetadata` for that table, and always omit internal `.rustfs-table` keys. Protected cursors are encrypted with a key derived from the cluster root credentials, so rotating those credentials invalidates cursors already issued. | -| Object-backed warehouse-index recovery | Supported with bounded request fallback | Data-plane requests repair or verify a missing warehouse-prefix index against bounded pages totaling at most 4,096 catalog objects. Larger catalogs fail closed with a retryable service error and require `POST /iceberg/v1/{warehouse}/catalog/warehouse-index/backfill` with `admin:MigrateTableCatalog` instead of allowing one S3 request to trigger an unbounded scan. | +| Object-backed warehouse-index recovery | Supported with bounded request fallback | Data-plane requests repair or verify a missing warehouse-prefix index by listing the catalog prefix. Only table entries count toward the 4,096-object limit, so maintenance reports and other per-table objects cannot exhaust it; the listing itself is bounded to 64 pages. Larger catalogs fail closed with a retryable service error and require operator backfill. An optional table-entry directory (see Maintenance Matrix) answers these lookups without the prefix listing. | | Table bucket enablement | Supported | A regular bucket is enabled for catalog use and addressed as the REST catalog warehouse. | | Catalog-vended table credentials | Automated when enabled | Disabled by default. LoadTable vends credentials only when `X-Iceberg-Access-Delegation` contains the exact `vended-credentials` token; the dedicated credentials endpoint uses the same issuer path. | | AWS S3 Tables endpoint shape | Profile generator | Generates the AWS catalog URI and warehouse ARN shape for migration docs. API parity not claimed. | @@ -106,12 +106,24 @@ expiration and compaction plan/commit, remains object-backed only. A configured server has opt-in, allowlisted periodic queue admission; see the [scheduler runbook](../operations/s3-tables-maintenance-scheduler.md). -Renamed tables retain their original metadata paths. Durable-strong scheduling -and cleanup fail closed with a conflict if another table shares a scanned -metadata or warehouse directory, including a recreated source name after rename. -Such tables require disjoint cleanup roots before maintenance can run; enabling +Renamed tables retain their original metadata paths. Scheduling, planning, +and cleanup on both backings fail closed with a conflict if another table +shares a scanned metadata or warehouse directory, including a recreated source +name after rename. The object backing rechecks this ownership inventory under +the bucket publication fence before every delete pass; the inventory reads every +table entry in the bucket, so maintenance cost grows with the table count. Such +tables require disjoint cleanup roots before maintenance can run; enabling deletion does not override this ownership check. +Object-backed create, register, and commit publication share one preparation deadline, `RUSTFS_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS` (default 30, never below the object lock-acquire timeout). Migration and bucket-lock queueing, reference HEAD validation, ownership inventories, metadata parsing, and snapshot preparation consume that same interval; repeated preparation does not renew it. A create writes its initial metadata file before registration without ending the deadline, so an orphaned file (reclaimed by maintenance) is the worst outcome of an expired create. Expiry before a possible durable catalog write returns HTTP 503 and releases the preparation locks; a valid large operation that cannot finish within the interval keeps failing on retry until the interval is raised. Once a catalog write may have begun, this deadline no longer cancels that transaction. Individual PUT completion and persisted commit/rename recovery retain their existing cancellation guarantees. This is a preparation limit, not a hard bound on disk I/O or the durable tail. The durable-strong backing does not implement this deadline; it is being retired. A rename that the client abandons mid-flight completes or recovers on a detached task, so the bucket's `active_rename_id` is always cleared. + +Compaction commits hold inputs to a 512 MiB stored and decoded budget per rewrite group, validate the footer and every page header before decoding a page (a recursive or over-deep Iceberg manifest Avro schema and a Parquet page that exceeds its budget are refused with HTTP 400), and run one compaction per node at a time; a concurrent request receives HTTP 503. + +### Table-entry directory + +An optional per-bucket directory of table entries under `.rustfs.sys` answers data-plane index-miss lookups without listing the catalog prefix. It is off by default: `RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX=true` and `RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX_FLEET_CONFIRMED=true` on every node, then `POST /iceberg/v1/{warehouse}/catalog/directory` (`admin:MigrateTableCatalog`) advances a backfill of 128 catalog objects per call until the directory reports `Ready`; `GET` reports the phase and `DELETE` rolls the directory back. Entry writers add a directory candidate only while the directory exists, and the backfill covers earlier entries. The directory state occupies the backing-migration fence object: nodes without this feature answer catalog mutations on that bucket with a parse error until they are upgraded, which is what the fleet-confirmed flag attests, and a backing migration is refused with a conflict while the directory is active. + + Durable-strong rename preserves maintenance configuration, job identifiers, current/latest aliases, history, audit events, and quarantine state. A persisted rename intent protects sidecar migration across interruption: the source remains diff --git a/rustfs/src/admin/handlers/table_catalog/config.rs b/rustfs/src/admin/handlers/table_catalog/config.rs index 9f94d670e..47a668105 100644 --- a/rustfs/src/admin/handlers/table_catalog/config.rs +++ b/rustfs/src/admin/handlers/table_catalog/config.rs @@ -293,3 +293,46 @@ impl Operation for SyncExternalCatalogBridgeHandler { build_json_response(StatusCode::OK, &response) } } + +pub struct GetTableDirectoryHandler {} + +#[async_trait::async_trait] +impl Operation for GetTableDirectoryHandler { + async fn call(&self, req: S3Request, params: Params<'_, '_>) -> S3Result> { + let warehouse = warehouse_from_params(¶ms)?; + let resource = TableCatalogResource::warehouse(&warehouse); + authorize_table_catalog_resource_request(&req, &resource, AdminAction::GetTableCatalogAction).await?; + ensure_table_bucket_enabled_from_extensions(&req.extensions, &warehouse).await?; + let store = table_catalog_object_store_from_extensions(&req.extensions)?; + let response = store.table_directory_status(&warehouse).await.map_err(catalog_store_error)?; + build_json_response(StatusCode::OK, &response) + } +} + +pub struct AdvanceTableDirectoryHandler {} + +#[async_trait::async_trait] +impl Operation for AdvanceTableDirectoryHandler { + async fn call(&self, req: S3Request, params: Params<'_, '_>) -> S3Result> { + let warehouse = warehouse_from_params(¶ms)?; + authorize_table_catalog_request(&req, AdminAction::MigrateTableCatalogAction).await?; + ensure_table_bucket_enabled_from_extensions(&req.extensions, &warehouse).await?; + let store = table_catalog_object_store_from_extensions(&req.extensions)?; + let response = store.advance_table_directory(&warehouse).await.map_err(catalog_store_error)?; + build_json_response(StatusCode::OK, &response) + } +} + +pub struct DisableTableDirectoryHandler {} + +#[async_trait::async_trait] +impl Operation for DisableTableDirectoryHandler { + async fn call(&self, req: S3Request, params: Params<'_, '_>) -> S3Result> { + let warehouse = warehouse_from_params(¶ms)?; + authorize_table_catalog_request(&req, AdminAction::MigrateTableCatalogAction).await?; + ensure_table_bucket_enabled_from_extensions(&req.extensions, &warehouse).await?; + let store = table_catalog_object_store_from_extensions(&req.extensions)?; + let response = store.disable_table_directory(&warehouse).await.map_err(catalog_store_error)?; + build_json_response(StatusCode::OK, &response) + } +} diff --git a/rustfs/src/admin/handlers/table_catalog/mod.rs b/rustfs/src/admin/handlers/table_catalog/mod.rs index c429835fa..e041e93aa 100644 --- a/rustfs/src/admin/handlers/table_catalog/mod.rs +++ b/rustfs/src/admin/handlers/table_catalog/mod.rs @@ -1364,8 +1364,14 @@ enum TableCommitObjectIdentity { ContentSha256(String), } +// Legacy objects without a write-unique body identity retain SHA validation, +// with a total byte budget independent of the manifest graph's larger limit. +const TABLE_COMMIT_LEGACY_REVALIDATION_MAX_BYTES: usize = 2 * MAX_ADMIN_REQUEST_BODY_SIZE; + #[derive(Clone, PartialEq, Eq)] struct TableCommitObservedObject { + body_revision: Option, + content_size: usize, identity: TableCommitObjectIdentity, max_size: Option, } @@ -1377,6 +1383,7 @@ struct TableCommitPublicationState { table_fence: Option<(String, String, String)>, catalog_migration_read_permit: bool, observed_objects: BTreeMap<(String, String), TableCommitObservedObject>, + legacy_revalidation_bytes: usize, guards: Vec, } @@ -1387,6 +1394,7 @@ struct TableCommitObjectBackend { authorization_error: Arc>>, publication: Arc>, publication_fence_fleet_confirmed: bool, + preparation: Arc, } impl TableCommitObjectBackend @@ -1400,6 +1408,7 @@ where authorization_error: Arc::new(tokio::sync::Mutex::new(None)), publication: Arc::new(parking_lot::Mutex::new(TableCommitPublicationState::default())), publication_fence_fleet_confirmed, + preparation: Arc::new(crate::table_catalog::TablePublicationPreparation::default()), } } @@ -1445,6 +1454,62 @@ where ) } + async fn prepare_put_observation( + &self, + bucket: &str, + object: &str, + data: &[u8], + ) -> crate::table_catalog::TableCatalogStoreResult { + self.prepare_put_observation_with(bucket, object, data, true).await + } + + /// `begins_mutation` is false for a write that precedes the catalog + /// transaction (see `put_object_before_publication`): it is still fenced + /// and authorized, but it leaves the preparation deadline armed. + async fn prepare_put_observation_with( + &self, + bucket: &str, + object: &str, + data: &[u8], + begins_mutation: bool, + ) -> crate::table_catalog::TableCatalogStoreResult { + let fence = { + let publication = self.publication.lock(); + publication.bucket_fence.as_ref().map(|_| { + crate::table_catalog::TableCatalogObjectMutationFence::from_signals( + publication + .guards + .iter() + .map(crate::table_catalog::TableCatalogLockGuard::lock_lost_signal), + ) + }) + }; + let authorize = self.authorize(bucket, object, S3Action::PutObjectAction); + if fence.is_some() { + self.preparation.read(self, authorize).await?; + } else { + authorize.await?; + } + self.ensure_observation_allowed(bucket, object)?; + let observation = TableCommitObservedObject { + identity: TableCommitObjectIdentity::ContentSha256(hex_sha256(data, str::to_string)), + body_revision: None, + content_size: data.len(), + max_size: Some(data.len()), + }; + if let Some(fence) = fence { + // An unfenced candidate write precedes the publication transaction. Only a PUT + // under the bucket fence commits this preparation to finishing its durable tail. + fence.ensure_held()?; + if begins_mutation { + self.preparation.begin_mutation()?; + } else { + self.preparation.check_deadline()?; + } + } + Ok(observation) + } + async fn authorize(&self, bucket: &str, object: &str, action: S3Action) -> crate::table_catalog::TableCatalogStoreResult<()> { let result = match &self.authorization { TableCommitObjectAuthorization::Request(req) => { @@ -1502,31 +1567,62 @@ where bucket: &str, object: &str, observation: TableCommitObservedObject, + enforce_legacy_budget: bool, ) -> crate::table_catalog::TableCatalogStoreResult<()> { let key = (bucket.to_string(), object.to_string()); let mut publication = self.publication.lock(); if publication.phase == TableCommitPublicationPhase::Complete { return Ok(()); } + let previous_legacy_bytes = publication.observed_objects.get(&key).map_or(0, |observed| { + if observed.body_revision.is_none() { + observed.content_size + } else { + 0 + } + }); + let legacy_bytes = publication.legacy_revalidation_bytes - previous_legacy_bytes; + let legacy_bytes = legacy_bytes.checked_add(if observation.body_revision.is_none() { + observation.content_size + } else { + 0 + }); + let Some(legacy_bytes) = + legacy_bytes.filter(|bytes| !enforce_legacy_budget || *bytes <= TABLE_COMMIT_LEGACY_REVALIDATION_MAX_BYTES) + else { + return Err(crate::table_catalog::TableCatalogStoreError::Unavailable( + "legacy catalog objects exceed the publication validation budget; rewrite the referenced objects and retry" + .to_string(), + )); + }; let Some(existing) = publication.observed_objects.get_mut(&key) else { if publication.phase != TableCommitPublicationPhase::Discovering { return Err(crate::table_catalog::TableCatalogStoreError::Internal(format!( "table commit accessed an unvalidated object after publication preparation: {bucket}/{object}" ))); } + publication.legacy_revalidation_bytes = legacy_bytes; publication.observed_objects.insert(key, observation); return Ok(()); }; - if existing.identity != observation.identity { + if existing.identity != observation.identity + || existing + .body_revision + .zip(observation.body_revision) + .is_some_and(|(before, after)| before != after) + { return Err(crate::table_catalog::TableCatalogStoreError::Conflict(format!( "table commit object changed during validation: {bucket}/{object}" ))); } + existing.body_revision = observation.body_revision; + existing.content_size = observation.content_size; existing.max_size = match (existing.max_size, observation.max_size) { (Some(expected), Some(actual)) => Some(expected.min(actual)), (Some(expected), None) => Some(expected), (None, actual) => actual, }; + publication.legacy_revalidation_bytes = legacy_bytes; Ok(()) } @@ -1538,7 +1634,12 @@ where None => TableCommitObjectIdentity::Missing, Some(object) => TableCommitObjectIdentity::ContentSha256(hex_sha256(&object.data, str::to_string)), }; - TableCommitObservedObject { identity, max_size } + TableCommitObservedObject { + identity, + max_size, + body_revision: object.and_then(|object| object.body_revision).filter(|id| !id.is_nil()), + content_size: object.map_or(0, |object| object.data.len()), + } } fn observed_metadata( @@ -1560,10 +1661,18 @@ where Ok(TableCommitObservedObject { identity, max_size: None, + body_revision: None, + content_size: 0, }) } async fn begin_bucket_publication(&self, table_bucket: &str) -> crate::table_catalog::TableCatalogStoreResult<()> { + self.preparation + .read(self, self.begin_bucket_publication_inner(table_bucket)) + .await + } + + async fn begin_bucket_publication_inner(&self, table_bucket: &str) -> crate::table_catalog::TableCatalogStoreResult<()> { { let mut publication = self.publication.lock(); if publication.phase != TableCommitPublicationPhase::Discovering { @@ -1610,6 +1719,17 @@ where table_bucket: &str, namespace: &str, table: &str, + ) -> crate::table_catalog::TableCatalogStoreResult<()> { + self.preparation + .read(self, self.prepare_publication_inner(table_bucket, namespace, table)) + .await + } + + async fn prepare_publication_inner( + &self, + table_bucket: &str, + namespace: &str, + table: &str, ) -> crate::table_catalog::TableCatalogStoreResult<()> { let namespace = crate::table_catalog::Namespace::parse(namespace) .map_err(|err| crate::table_catalog::TableCatalogStoreError::Invalid(format!("invalid namespace: {err}")))?; @@ -1619,6 +1739,12 @@ where let publication_lock = crate::table_catalog::default_table_publication_lock_path(&namespace, &table); let expected = { let mut publication = self.publication.lock(); + if publication.legacy_revalidation_bytes > TABLE_COMMIT_LEGACY_REVALIDATION_MAX_BYTES { + return Err(crate::table_catalog::TableCatalogStoreError::Unavailable( + "legacy catalog objects exceed the publication validation budget; rewrite the referenced objects and retry" + .to_string(), + )); + } match publication.phase { TableCommitPublicationPhase::Prepared if publication.table_fence.as_ref() == Some(&table_fence) => { return Ok(()); @@ -1668,10 +1794,19 @@ where if !matches!(&expected_object.identity, TableCommitObjectIdentity::ContentSha256(_)) { continue; } - let actual_object = match expected_object.max_size { - Some(max_size) => self.backend.read_object_unlocked_limited(bucket, object, max_size).await?, - None => self.backend.read_object_unlocked(bucket, object).await?, - }; + if let Some(revision) = expected_object.body_revision { + let actual = self.backend.object_metadata_unlocked(bucket, object).await?; + if actual.and_then(|metadata| metadata.body_revision) != Some(revision) { + return Err(crate::table_catalog::TableCatalogStoreError::Conflict(format!( + "table commit object changed before catalog publication: {bucket}/{object}" + ))); + } + continue; + } + let max_size = expected_object + .max_size + .map_or(expected_object.content_size, |limit| limit.min(expected_object.content_size)); + let actual_object = self.backend.read_object_unlocked_limited(bucket, object, max_size).await?; let actual = Self::observed_content(actual_object.as_ref(), expected_object.max_size); if actual.identity != expected_object.identity { return Err(crate::table_catalog::TableCatalogStoreError::Conflict(format!( @@ -1762,7 +1897,7 @@ where self.authorize(bucket, object, S3Action::GetObjectAction).await?; self.ensure_observation_allowed(bucket, object)?; let result = self.backend.read_object(bucket, object).await?; - self.record_observation(bucket, object, Self::observed_content(result.as_ref(), None))?; + self.record_observation(bucket, object, Self::observed_content(result.as_ref(), None), true)?; Ok(result) } @@ -1775,7 +1910,7 @@ where self.authorize(bucket, object, S3Action::GetObjectAction).await?; self.ensure_observation_allowed(bucket, object)?; let result = self.backend.read_object_limited(bucket, object, max_size).await?; - self.record_observation(bucket, object, Self::observed_content(result.as_ref(), Some(max_size)))?; + self.record_observation(bucket, object, Self::observed_content(result.as_ref(), Some(max_size)), true)?; Ok(result) } @@ -1802,7 +1937,7 @@ where let metadata = self.backend.object_metadata(bucket, object).await?; let observation = Self::observed_metadata(bucket, object, metadata)?; let exists = observation.identity != TableCommitObjectIdentity::Missing; - self.record_observation(bucket, object, observation)?; + self.record_observation(bucket, object, observation, true)?; Ok(exists) } @@ -1817,14 +1952,24 @@ where data: Vec, precondition: crate::table_catalog::TableCatalogPutPrecondition, ) -> crate::table_catalog::TableCatalogStoreResult<()> { - self.authorize(bucket, object, S3Action::PutObjectAction).await?; - self.ensure_observation_allowed(bucket, object)?; - let observation = TableCommitObservedObject { - identity: TableCommitObjectIdentity::ContentSha256(hex_sha256(&data, str::to_string)), - max_size: None, - }; + let observation = self.prepare_put_observation(bucket, object, &data).await?; self.backend.put_object(bucket, object, data, precondition).await?; - self.record_observation(bucket, object, observation) + // The existing generated-metadata readback binds the written bytes to + // their revision. Until then, keep the original SHA expectation and + // enforce the fallback budget before publication preparation. + self.record_observation(bucket, object, observation, false) + } + + async fn put_object_before_publication( + &self, + bucket: &str, + object: &str, + data: Vec, + precondition: crate::table_catalog::TableCatalogPutPrecondition, + ) -> crate::table_catalog::TableCatalogStoreResult<()> { + let observation = self.prepare_put_observation_with(bucket, object, &data, false).await?; + self.backend.put_object(bucket, object, data, precondition).await?; + self.record_observation(bucket, object, observation, false) } async fn put_object_unlocked( @@ -1835,16 +1980,11 @@ where precondition: crate::table_catalog::TableCatalogPutPrecondition, write_guards: Vec, ) -> crate::table_catalog::TableCatalogStoreResult<()> { - self.authorize(bucket, object, S3Action::PutObjectAction).await?; - self.ensure_observation_allowed(bucket, object)?; - let observation = TableCommitObservedObject { - identity: TableCommitObjectIdentity::ContentSha256(hex_sha256(&data, str::to_string)), - max_size: None, - }; + let observation = self.prepare_put_observation(bucket, object, &data).await?; self.backend .put_object_unlocked(bucket, object, data, precondition, write_guards) .await?; - self.record_observation(bucket, object, observation) + self.record_observation(bucket, object, observation, false) } async fn delete_object(&self, bucket: &str, object: &str) -> crate::table_catalog::TableCatalogStoreResult<()> { @@ -1890,6 +2030,10 @@ where self.backend.acquire_write_lock(bucket, object).await } + fn table_publication_preparation_budget(&self) -> Arc { + self.preparation.clone() + } + async fn begin_table_bucket_commit_publication( &self, table_bucket: &str, @@ -2522,6 +2666,29 @@ where .map_err(catalog_store_error) } +/// Recheck the bucket registered before the publication lock. A missing entry +/// here can indicate retirement while queued; do not recreate it under the fence. +async fn require_table_bucket_for_publication( + store: &S, + bucket: &str, + publication: &(dyn crate::table_catalog::TableCommitPublication + Sync), +) -> S3Result<()> +where + S: crate::table_catalog::TableCatalogStore + ?Sized, +{ + let entry = publication + .preparation_budget() + .read(publication, store.get_table_bucket(bucket)) + .await + .map_err(catalog_store_error)?; + if entry.is_some_and(|entry| entry.state == crate::table_catalog::TableCatalogEntryState::Active) { + return Ok(()); + } + Err(catalog_store_error(crate::table_catalog::TableCatalogStoreError::NotFound(format!( + "table bucket {bucket}" + )))) +} + async fn table_bucket_response(store: &S, bucket: &str, enabled: bool) -> S3Result where S: crate::table_catalog::TableCatalogStore + ?Sized, @@ -6014,11 +6181,11 @@ where ))); } let _publication_completion = crate::table_catalog::TableCommitPublicationCompletion::new(metadata_backend); - ensure_table_bucket_entry(store, bucket, true).await?; + require_table_bucket_for_publication(store, bucket, metadata_backend).await?; let metadata_data = serde_json::to_vec(&metadata) .map_err(|err| s3_error!(InternalError, "failed to serialize initial table metadata: {}", err))?; metadata_backend - .put_object( + .put_object_before_publication( bucket, &entry.metadata_location, metadata_data, @@ -6055,7 +6222,7 @@ where ))); } let _publication_completion = crate::table_catalog::TableCommitPublicationCompletion::new(metadata_backend); - ensure_table_bucket_entry(store, bucket, true).await?; + require_table_bucket_for_publication(store, bucket, metadata_backend).await?; let metadata_data = serde_json::to_vec(&metadata) .map_err(|err| s3_error!(InternalError, "failed to serialize initial view metadata: {}", err))?; metadata_backend @@ -6478,15 +6645,18 @@ async fn publish_table_commit( where S: crate::table_catalog::TableCatalogStore + ?Sized, { - let _bucket_publication_completion = if table_bucket_fence_required { - metadata_backend - .begin_table_bucket_commit_publication(&request.table_bucket) - .await - .map_err(catalog_store_error)?; - Some(crate::table_catalog::TableCommitPublicationCompletion::new(metadata_backend)) - } else { - None - }; + // Real request backends acquire the migration permit here, before the store chooses its + // inline/child route. Nested publication reuses that same permit and preparation budget. + let _bucket_publication_completion = + if table_bucket_fence_required || metadata_backend.acquires_catalog_migration_read_permit() { + metadata_backend + .begin_table_bucket_commit_publication(&request.table_bucket) + .await + .map_err(catalog_store_error)?; + Some(crate::table_catalog::TableCommitPublicationCompletion::new(metadata_backend)) + } else { + None + }; store .commit_table_with_publication(request, metadata_backend) .await @@ -6843,6 +7013,7 @@ where .map_err(catalog_store_error)?; validate_table_metadata_snapshot_graph(metadata_backend, bucket, &entry, None, &metadata).await?; + ensure_table_bucket_entry(store, bucket, true).await?; crate::table_catalog::TableCommitPublication::begin_table_bucket(metadata_backend, bucket) .await .map_err(catalog_store_error)?; @@ -6852,12 +7023,12 @@ where ))); } let _publication_completion = crate::table_catalog::TableCommitPublicationCompletion::new(metadata_backend); - ensure_table_bucket_entry(store, bucket, true).await?; + require_table_bucket_for_publication(store, bucket, metadata_backend).await?; let metadata_data = serde_json::to_vec(&metadata).map_err(|err| { S3Error::with_message(S3ErrorCode::InternalError, format!("failed to serialize initial table metadata: {err}")) })?; metadata_backend - .put_object( + .put_object_before_publication( bucket, &entry.metadata_location, metadata_data, diff --git a/rustfs/src/admin/handlers/table_catalog/routes.rs b/rustfs/src/admin/handlers/table_catalog/routes.rs index 5ba191123..802bb75c6 100644 --- a/rustfs/src/admin/handlers/table_catalog/routes.rs +++ b/rustfs/src/admin/handlers/table_catalog/routes.rs @@ -39,6 +39,21 @@ fn register_table_catalog_prefix_routes(r: &mut S3Router, prefix format!("{prefix}/buckets/{{warehouse}}").as_str(), AdminOperation(&GET_TABLE_BUCKET_HANDLER), )?; + r.insert( + Method::GET, + format!("{prefix}/{{warehouse}}/catalog/directory").as_str(), + AdminOperation(&GetTableDirectoryHandler {}), + )?; + r.insert( + Method::POST, + format!("{prefix}/{{warehouse}}/catalog/directory").as_str(), + AdminOperation(&AdvanceTableDirectoryHandler {}), + )?; + r.insert( + Method::DELETE, + format!("{prefix}/{{warehouse}}/catalog/directory").as_str(), + AdminOperation(&DisableTableDirectoryHandler {}), + )?; r.insert( Method::GET, format!("{prefix}/{{warehouse}}/catalog/migration").as_str(), diff --git a/rustfs/src/admin/handlers/table_catalog/table.rs b/rustfs/src/admin/handlers/table_catalog/table.rs index 66ea2f404..9f572d103 100644 --- a/rustfs/src/admin/handlers/table_catalog/table.rs +++ b/rustfs/src/admin/handlers/table_catalog/table.rs @@ -56,6 +56,25 @@ impl Operation for RestRenameTableHandler { ensure_table_bucket_enabled_from_extensions(&req.extensions, &warehouse).await?; let store = table_catalog_store_from_extensions(&req.extensions)?; + rename_table_response(store, warehouse, source_namespace, source_table, destination_namespace, destination_table).await?; + Ok(empty_response(StatusCode::NO_CONTENT)) + } +} + +// Authorization has completed before ownership moves to the mutation task. A +// disconnected request must not strand the persisted bucket-wide rename fence. +pub(super) async fn rename_table_response( + store: S, + warehouse: String, + source_namespace: crate::table_catalog::Namespace, + source_table: crate::table_catalog::IdentifierSegment, + destination_namespace: crate::table_catalog::Namespace, + destination_table: crate::table_catalog::IdentifierSegment, +) -> S3Result<()> +where + S: crate::table_catalog::TableCatalogStore + Send + 'static, +{ + crate::admin::handlers::supervise_admin_mutation("table rename", async move { store .rename_table( &warehouse, @@ -65,9 +84,9 @@ impl Operation for RestRenameTableHandler { destination_table.as_str(), ) .await - .map_err(catalog_store_error)?; - Ok(empty_response(StatusCode::NO_CONTENT)) - } + .map_err(catalog_store_error) + }) + .await } pub struct RestCreateTableHandler {} diff --git a/rustfs/src/admin/handlers/table_catalog/tests.rs b/rustfs/src/admin/handlers/table_catalog/tests.rs index f43154dfa..49af69533 100644 --- a/rustfs/src/admin/handlers/table_catalog/tests.rs +++ b/rustfs/src/admin/handlers/table_catalog/tests.rs @@ -277,6 +277,41 @@ async fn table_catalog_authentication_and_credentials_use_the_request_context() assert!(Arc::ptr_eq(&resolved_store, &store)); let mut request = request; + iam.set_policy(policy_name, Policy::parse_config(br#"{ + "Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":["admin:GetTable","admin:CreatePluginDelegation"]}, + {"Effect":"Deny","Action":["admin:GetTable","admin:CreatePluginDelegation"],"Condition":{"NotIpAddress":{"aws:SourceIp":"10.0.0.0/8"}}} + ] + }"#).unwrap()).await.unwrap(); + request + .extensions + .insert(Some(crate::server::RemoteAddr("10.1.2.3:4567".parse().unwrap()))); + request.headers.insert("x-forwarded-for", "10.1.2.3".parse().unwrap()); + for (client, allowed) in [ + (None, true), + (Some("203.0.113.17:7890"), false), + (Some("10.8.7.6:7890"), true), + ] { + request.extensions.remove::(); + if let Some(client) = client { + request + .extensions + .insert(rustfs_trusted_proxies::ClientInfo::direct(client.parse().unwrap())); + } + let unscoped = authorize_table_catalog_request(&request, AdminAction::GetTableAction).await; + let scoped = authorize_table_catalog_resource_request(&request, &resource, AdminAction::GetTableAction).await; + let delegated = + crate::admin::handlers::plugin_delegation::owner(&request, AdminAction::CreatePluginDelegationAction).await; + if allowed { + unscoped.expect("verified private address authorizes table catalog access"); + scoped.expect("verified private address authorizes scoped table access"); + delegated.expect("verified private address authorizes delegation"); + } else { + assert_eq!(unscoped.expect_err("external client is denied").code(), &S3ErrorCode::AccessDenied); + assert_eq!(scoped.err().expect("external client is denied").code(), &S3ErrorCode::AccessDenied); + assert_eq!(delegated.err().expect("external client is denied").code(), &S3ErrorCode::AccessDenied); + } + } request.method = Method::POST; request.uri = "/iceberg/v1/request-warehouse/namespaces/analytics/tables/events/maintenance/worker/run" .parse() @@ -1473,6 +1508,7 @@ fn table_catalog_handlers_require_table_admin_actions() { 2 ); assert_eq!(rename_block.matches("AdminAction::SetTableAction").count(), 2); + assert!(rename_block.contains("rename_table_response(")); let migration_block = operation_block(&src, "GetTableCatalogMigrationHandler"); assert!( @@ -1731,7 +1767,7 @@ fn table_bucket_toggle_handlers_reload_peer_metadata_after_publication() { #[test] fn table_commit_publication_acquires_migration_permits_before_publication_lock() { let src = table_catalog_handler_source(); - let block = function_block(&src, "async fn begin_bucket_publication"); + let block = function_block(&src, "async fn begin_bucket_publication_inner"); let migration_permits = block .find("acquire_catalog_migration_read_guards(table_bucket)") .expect("publication should acquire catalog migration permits"); @@ -7128,6 +7164,371 @@ async fn commit_publication_acquires_discovered_object_locks_in_key_order() { crate::table_catalog::TableCatalogObjectBackend::complete_table_commit_publication(&commit_backend); } +#[tokio::test] +async fn catalog_publication_reads_graph_once_for_registration_and_relocation() { + for relocate in [false, true] { + let backend = TestTableCatalogObjectBackend::content_addressed(); + let store = crate::table_catalog::ObjectTableCatalogStore::new(backend.clone()); + let namespace = crate::table_catalog::Namespace::parse("analytics").expect("namespace"); + let (metadata_location, mut metadata, version_token) = if relocate { + let created = create_standard_events_table(&store, &backend, &namespace).await; + let current = store + .load_table("warehouse", "analytics", "events") + .await + .expect("load table") + .expect("table exists"); + let table = crate::table_catalog::IdentifierSegment::parse("events").expect("table"); + ( + crate::table_catalog::default_table_metadata_file_path(&namespace, &table, "00002-relocation.metadata.json"), + created.metadata, + Some(current.version_token), + ) + } else { + let (_, location) = seed_events_registration_target(&store, &backend).await; + ( + location, + test_table_metadata_json("metadata-table-uuid", "s3://warehouse/tables/table-id"), + None, + ) + }; + let warehouse = "s3://warehouse/tables/revision-graph"; + let manifest_list = format!("{warehouse}/metadata/snap-10.avro"); + let manifest = format!("{warehouse}/metadata/manifest-snap-10.avro"); + let data_file = format!("{warehouse}/data/part-10.parquet"); + metadata["location"] = serde_json::json!(warehouse); + metadata["last-sequence-number"] = serde_json::json!(1); + metadata["snapshots"] = serde_json::json!([{ + "snapshot-id": 10, "sequence-number": 1, "timestamp-ms": 10, + "manifest-list": manifest_list, "summary": {"operation": "append"} + }]); + metadata["current-snapshot-id"] = serde_json::json!(10); + metadata["refs"] = serde_json::json!({"main": {"type": "branch", "snapshot-id": 10}}); + backend.put_json("warehouse", &metadata_location, metadata).await; + seed_test_snapshot_manifest(&backend, "warehouse", &manifest_list, 10, 1, &[(&data_file, 0, 1, 10, 1)]).await; + let commit = TableCommitObjectBackend::trusted(backend.clone()); + if let Some(version_token) = version_token { + update_table_metadata_location_response( + &store, + &commit, + "warehouse", + &namespace, + "events", + UpdateTableMetadataLocationRequest { + metadata_location: table_metadata_location_for_client("warehouse", &metadata_location), + version_token, + commit_id: Some("revision-relocation".to_string()), + idempotency_key: None, + }, + ) + .await + .expect("relocation publishes the validated graph"); + } else { + register_table_response( + &store, + &commit, + "warehouse", + &namespace, + RegisterTableRequest { + name: "events".to_string(), + metadata_location: metadata_location.clone(), + overwrite: false, + }, + true, + ) + .await + .expect("registration publishes the validated graph"); + } + let published = store + .load_table("warehouse", "analytics", "events") + .await + .expect("load published table") + .expect("table exists"); + assert_eq!(published.warehouse_location, warehouse); + assert_eq!(published.metadata_location, metadata_location); + let state = backend.state.lock().await; + for location in [&manifest_list, &manifest] { + let key = ("warehouse".to_string(), test_snapshot_object_key("warehouse", location)); + assert_eq!( + state.read_attempts.get(&key), + Some(&1), + "manifest bodies must be read once, before the bucket fence: {location}" + ); + assert_eq!( + state.read_lock_acquisitions.get(&key), + Some(&1), + "publication must retain the manifest object read lock" + ); + } + } +} + +#[tokio::test] +async fn catalog_create_binds_generated_metadata_before_publication() { + let backend = TestTableCatalogObjectBackend::content_addressed(); + let store = crate::table_catalog::ObjectTableCatalogStore::new(backend.clone()); + let namespace = crate::table_catalog::Namespace::parse("analytics").expect("namespace"); + let created = create_standard_events_table(&store, &backend, &namespace).await; + let object = table_metadata_location_for_catalog("warehouse", &created.metadata_location).expect("metadata key"); + let state = backend.state.lock().await; + let key = ("warehouse".to_string(), object); + assert_eq!( + state.read_attempts.get(&key), + Some(&1), + "initial create keeps its generated metadata SHA read bounded" + ); + assert!( + state + .objects + .get(&key) + .expect("created metadata exists") + .body_revision + .is_some() + ); + assert_eq!( + state.read_lock_acquisitions.get(&key), + Some(&1), + "create keeps its metadata body stable until publication" + ); +} + +#[tokio::test] +async fn commit_publication_revalidates_legacy_bytes_with_a_total_budget() { + use crate::table_catalog::{TableCatalogObjectBackend as _, TableCatalogStoreError}; + + let backend = TestTableCatalogObjectBackend::content_addressed(); + let first = "metadata/legacy-first.avro"; + let second = "metadata/legacy-second.avro"; + let payload = vec![b'x'; TABLE_COMMIT_LEGACY_REVALIDATION_MAX_BYTES / 2 + 1]; + for object in [first, second] { + backend.put_bytes("warehouse", object, payload.clone()).await; + backend + .state + .lock() + .await + .objects + .get_mut(&("warehouse".to_string(), object.to_string())) + .expect("legacy fixture exists") + .body_revision = None; + } + let commit = TableCommitObjectBackend::trusted(backend.clone()); + commit + .read_object("warehouse", first) + .await + .expect("first legacy object fits"); + commit + .read_object("warehouse", first) + .await + .expect("repeated observations do not consume the budget twice"); + let error = commit + .read_object("warehouse", second) + .await + .expect_err("combined legacy payload exceeds the budget"); + assert!(matches!(error, TableCatalogStoreError::Unavailable(_))); + assert!( + backend.lock_attempts.lock().await.is_empty(), + "oversized legacy validation fails before publication locks" + ); + + // A supported small legacy object still uses the original body fingerprint, + // even when metadata and the absent revision cannot distinguish an overwrite. + let legacy = "metadata/legacy-small.json"; + backend.put_bytes("warehouse", legacy, b"before".to_vec()).await; + backend + .state + .lock() + .await + .objects + .get_mut(&("warehouse".to_string(), legacy.to_string())) + .expect("small legacy fixture exists") + .body_revision = None; + let commit = TableCommitObjectBackend::trusted(backend.clone()); + commit.read_object("warehouse", legacy).await.expect("read legacy object"); + backend + .state + .lock() + .await + .objects + .get_mut(&("warehouse".to_string(), legacy.to_string())) + .expect("legacy object remains") + .data = b"after!".to_vec(); + let error = commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect_err("legacy SHA validation detects an equal-length overwrite with unchanged metadata"); + assert!(matches!(error, TableCatalogStoreError::Conflict(_))); + + // Rewriting old objects supplies fresh server revisions and removes the + // legacy-only size restriction without removing publication validation. + for object in [first, second] { + backend.put_bytes("warehouse", object, payload.clone()).await; + } + let commit = TableCommitObjectBackend::trusted(backend.clone()); + for object in [first, second] { + commit + .read_object("warehouse", object) + .await + .expect("rewritten object can be validated"); + } + let reads = backend.read_call_count().await; + commit + .begin_table_bucket_commit_publication("warehouse") + .await + .expect("take bucket fence"); + commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect("publish rewritten objects"); + assert_eq!( + backend.read_call_count().await, + reads, + "revision validation must not reread bodies under the bucket fence" + ); + commit.complete_table_commit_publication(); +} + +#[tokio::test] +async fn commit_publication_bounds_unread_generated_metadata() { + use crate::table_catalog::{TableCatalogObjectBackend as _, TableCatalogPutPrecondition, TableCatalogStoreError}; + let backend = TestTableCatalogObjectBackend::content_addressed(); + let payload = vec![b'x'; TABLE_COMMIT_LEGACY_REVALIDATION_MAX_BYTES + 1]; + let commit = TableCommitObjectBackend::trusted(backend.clone()); + commit + .put_object("warehouse", "metadata/generated.json", payload, TableCatalogPutPrecondition::IfAbsent) + .await + .expect("write generated metadata before its normal readback"); + let error = commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect_err("publication cannot read oversized unbound generated metadata under its locks"); + assert!(matches!(error, TableCatalogStoreError::Unavailable(_))); + assert!(backend.lock_attempts.lock().await.is_empty()); + assert_eq!(backend.read_call_count().await, 0); + commit + .read_object("warehouse", "metadata/generated.json") + .await + .expect("the existing generated metadata readback binds the revision"); + let reads = backend.read_call_count().await; + commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect("bound generated metadata can publish"); + assert_eq!(backend.read_call_count().await, reads); + commit.complete_table_commit_publication(); +} + +#[tokio::test] +async fn commit_publication_accepts_metadata_only_changes_with_same_body_revision() { + use crate::table_catalog::TableCatalogObjectBackend as _; + let backend = TestTableCatalogObjectBackend::content_addressed(); + backend + .put_bytes("warehouse", "metadata/current.json", b"unchanged body".to_vec()) + .await; + let commit = TableCommitObjectBackend::trusted(backend.clone()); + commit + .read_object("warehouse", "metadata/current.json") + .await + .expect("observe body"); + { + let mut state = backend.state.lock().await; + let object = state + .objects + .get_mut(&("warehouse".to_string(), "metadata/current.json".to_string())) + .expect("object exists"); + object.etag = "metadata-only-update".to_string(); + object.mod_time = Some(time::OffsetDateTime::now_utc()); + } + commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect("unchanged body remains valid after a metadata-only update"); + assert_eq!(backend.read_call_count().await, 1); + commit.complete_table_commit_publication(); +} + +#[tokio::test] +async fn commit_publication_accepts_unchanged_legacy_bytes() { + use crate::table_catalog::TableCatalogObjectBackend as _; + let backend = TestTableCatalogObjectBackend::content_addressed(); + backend + .put_bytes("warehouse", "metadata/legacy.json", b"unchanged".to_vec()) + .await; + backend + .state + .lock() + .await + .objects + .get_mut(&("warehouse".to_string(), "metadata/legacy.json".to_string())) + .expect("legacy fixture exists") + .body_revision = None; + let commit = TableCommitObjectBackend::trusted(backend.clone()); + commit + .read_object("warehouse", "metadata/legacy.json") + .await + .expect("observe legacy body"); + commit + .prepare_table_commit_publication("warehouse", "analytics", "events") + .await + .expect("small legacy object remains supported"); + assert_eq!( + backend.read_call_count().await, + 2, + "legacy body must still be verified under its object lock" + ); + commit.complete_table_commit_publication(); +} + +#[tokio::test] +async fn catalog_body_revision_changes_on_real_identical_body_overwrite() { + use crate::table_catalog::{ + EcStoreTableCatalogObjectBackend, StrongTableCatalogRuntime, TableCatalogObjectBackend as _, TableCatalogPutPrecondition, + }; + let (_temp_dir, _disk_paths, store) = crate::app::gating_test_env::isolated_multi_pool_ecstore().await; + let bucket = "catalog-body-revision"; + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("create bucket"); + let backend = EcStoreTableCatalogObjectBackend::new_with_strong_runtime(store, StrongTableCatalogRuntime::default()); + backend + .put_object(bucket, "manifest.avro", b"same bytes".to_vec(), TableCatalogPutPrecondition::IfAbsent) + .await + .expect("write object"); + let before = backend + .read_object(bucket, "manifest.avro") + .await + .expect("read body") + .expect("object exists"); + let before_stat = backend + .object_metadata(bucket, "manifest.avro") + .await + .expect("stat body") + .expect("object exists"); + assert!(before.body_revision.is_some()); + assert_eq!(before.body_revision, before_stat.body_revision); + backend + .put_object( + bucket, + "manifest.avro", + b"same bytes".to_vec(), + TableCatalogPutPrecondition::IfMatch(before.etag.clone().expect("etag exists")), + ) + .await + .expect("overwrite same bytes"); + let after = backend + .read_object(bucket, "manifest.avro") + .await + .expect("read overwritten body") + .expect("object exists"); + let after_stat = backend + .object_metadata(bucket, "manifest.avro") + .await + .expect("stat overwritten body") + .expect("object exists"); + assert_eq!(before.etag, after.etag); + assert_ne!(before.body_revision, after.body_revision); + assert_eq!(after.body_revision, after_stat.body_revision); +} + #[tokio::test] async fn commit_publication_revalidates_objects_after_ordered_lock_acquisition() { let metadata_backend = TestTableCatalogObjectBackend::content_addressed(); @@ -7160,6 +7561,7 @@ async fn commit_publication_binds_fingerprint_to_returned_bytes() { metadata_backend.state.lock().await.objects.insert( ("warehouse".to_string(), object.to_string()), TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), data: replacement, etag: original_etag.clone(), mod_time: None, @@ -7173,6 +7575,7 @@ async fn commit_publication_binds_fingerprint_to_returned_bytes() { metadata_backend.state.lock().await.objects.insert( ("warehouse".to_string(), object.to_string()), TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), data: original, etag: original_etag, mod_time: None, @@ -7226,6 +7629,7 @@ async fn standard_commit_publishes_more_than_ten_thousand_live_files() { state.objects.insert( ("warehouse".to_string(), test_snapshot_object_key("warehouse", file)), TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), data: data.clone(), etag: etag.clone(), mod_time: None, @@ -7256,7 +7660,7 @@ async fn standard_commit_publishes_more_than_ten_thousand_live_files() { ] })) .expect("boundary commit request should parse"); - let commit_backend = TableCommitObjectBackend::trusted(metadata_backend); + let commit_backend = TableCommitObjectBackend::trusted(metadata_backend.clone()); let result = standard_commit_table_response(&store, &commit_backend, "warehouse", &namespace, "events", request).await; let response = commit_backend @@ -7265,6 +7669,14 @@ async fn standard_commit_publishes_more_than_ten_thousand_live_files() { .expect("a valid snapshot with more than ten thousand live files should publish"); assert_eq!(response.metadata["current-snapshot-id"], 20); + for location in [&manifest_list, &manifest] { + let key = ("warehouse".to_string(), test_snapshot_object_key("warehouse", location)); + assert_eq!( + metadata_backend.state.lock().await.read_attempts.get(&key), + Some(&2), + "graph and conflict validation each read manifests once; publication must not read them again" + ); + } assert_eq!( store .load_table("warehouse", "analytics", "events") @@ -7565,6 +7977,78 @@ async fn metadata_location_api_rejects_unbound_legacy_catalog_identity() { assert_events_table_entry_unchanged(&store, &legacy_entry).await; } +#[tokio::test] +async fn rename_request_cancellation_finishes_the_persisted_bucket_fence() { + let backend = TestTableCatalogObjectBackend::content_addressed(); + let store = crate::table_catalog::ObjectTableCatalogStore::new(backend.clone()); + let namespace = crate::table_catalog::Namespace::parse("analytics").unwrap(); + let source = crate::table_catalog::IdentifierSegment::parse("events").unwrap(); + let destination = crate::table_catalog::IdentifierSegment::parse("renamed").unwrap(); + create_standard_events_table(&store, &backend, &namespace).await; + let original = store.load_table("warehouse", "analytics", "events").await.unwrap().unwrap(); + let paths = crate::table_catalog::TableCatalogObjectPaths::default(); + let source_path = paths.table_entry_path("warehouse", &namespace, &source); + let attempt = backend + .put_attempt_count(crate::admin::storage_api::RUSTFS_META_BUCKET, &source_path) + .await + + 2; + let pause = backend + .pause_put_attempt(crate::admin::storage_api::RUSTFS_META_BUCKET, &source_path, attempt) + .await; + let request = tokio::spawn(table::rename_table_response( + store.clone(), + "warehouse".to_string(), + namespace.clone(), + source.clone(), + namespace.clone(), + destination.clone(), + )); + tokio::time::timeout(std::time::Duration::from_secs(10), pause.wait_started()) + .await + .expect("rename reaches persisted fence"); + assert!( + store + .get_table_bucket("warehouse") + .await + .unwrap() + .unwrap() + .active_rename_id + .is_some() + ); + request.abort(); + assert!(request.await.unwrap_err().is_cancelled()); + pause.release(); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while store + .get_table_bucket("warehouse") + .await + .unwrap() + .unwrap() + .active_rename_id + .is_some() + { + tokio::task::yield_now().await; + } + }) + .await + .expect("detached mutation clears the durable fence"); + let renamed = store.load_table("warehouse", "analytics", "renamed").await.unwrap().unwrap(); + assert_eq!(renamed.table_id, original.table_id); + assert_eq!(renamed.metadata_location, original.metadata_location); + assert!(store.load_table("warehouse", "analytics", "events").await.unwrap().is_none()); + assert!( + store + .resolve_table_data_plane_resource("warehouse", "ordinary.txt") + .await + .unwrap() + .is_none() + ); + table::rename_table_response(store.clone(), "warehouse".to_string(), namespace.clone(), destination, namespace, source) + .await + .expect("subsequent ordinary rename succeeds"); + assert!(store.load_table("warehouse", "analytics", "events").await.unwrap().is_some()); +} + #[tokio::test] async fn table_metadata_maintenance_helper_runs_dry_run_and_delete() { let backend = TestTableCatalogObjectBackend::content_addressed(); @@ -15963,6 +16447,7 @@ struct ColdStrongRegistrationBackend { armed: Arc, reader_ready: Arc, resume: Arc, + backup_writer_waiting: Arc, } impl ColdStrongRegistrationBackend { @@ -16012,7 +16497,16 @@ impl crate::table_catalog::T bucket: &str, object: &str, ) -> crate::table_catalog::TableCatalogStoreResult { - self.inner.acquire_write_lock(bucket, object).await + let mut acquire = self.inner.acquire_write_lock(bucket, object); + if bucket == crate::admin::storage_api::RUSTFS_META_BUCKET + && object == crate::table_catalog::TableCatalogObjectPaths::default().backing_migration_global_fence_lock_path() + { + match futures::poll!(acquire.as_mut()) { + std::task::Poll::Ready(result) => return result, + std::task::Poll::Pending => self.backup_writer_waiting.notify_one(), + } + } + acquire.await } async fn read_object( @@ -16116,6 +16610,7 @@ async fn cold_strong_registration_completes_while_other_bucket_backup_waits() { armed: Arc::new(std::sync::atomic::AtomicBool::new(false)), reader_ready: Arc::new(tokio::sync::Notify::new()), resume: Arc::new(tokio::sync::Notify::new()), + backup_writer_waiting: Arc::new(tokio::sync::Notify::new()), }; let catalog = StrongTableCatalogStore::new(backend.clone()); for bucket in [&bucket, &backup_bucket] { @@ -16176,17 +16671,18 @@ async fn cold_strong_registration_completes_while_other_bucket_backup_waits() { .await .expect("registration holds first real global reader"); // The backup targets another bucket, so its documented writer-drain precondition is met. - let backup_catalog = StrongTableCatalogStore::new(inner); - let mut backup = Box::pin(backup_catalog.create_durable_catalog_backup(&backup_bucket, None)); - assert!( - futures::poll!(backup.as_mut()).is_pending(), - "actual backup writer must queue behind held registration reader" - ); + let backup_catalog = StrongTableCatalogStore::new(backend.clone()); + // Keep driving route discovery and snapshot I/O until the global write lock itself is pending. + let backup = tokio::spawn(async move { backup_catalog.create_durable_catalog_backup(&backup_bucket, None).await }); + tokio::time::timeout(StdDuration::from_secs(5), backend.backup_writer_waiting.notified()) + .await + .expect("actual backup writer must queue behind held registration reader"); backend.resume.notify_one(); let result = tokio::time::timeout(StdDuration::from_secs(2), &mut registration).await; if result.is_err() { // Prove that the queued backup, rather than ECStore I/O latency, caused the stall. - drop(backup); + backup.abort(); + assert!(backup.await.unwrap_err().is_cancelled()); tokio::time::timeout(StdDuration::from_secs(5), &mut registration) .await .expect("registration must resume once only the queued backup is cancelled") @@ -16201,6 +16697,7 @@ async fn cold_strong_registration_completes_while_other_bucket_backup_waits() { tokio::time::timeout(StdDuration::from_secs(5), backup) .await .expect("the other bucket backup should resume when registration releases its reader") + .expect("backup task joins") .expect("the other bucket backup succeeds"); } @@ -16528,6 +17025,66 @@ async fn cleanup_empty_table_bucket_reclaims_twenty_dropped_tables_and_allows_bu object_store.delete_bucket(&bucket, &Default::default()).await.unwrap(); } +#[tokio::test] +#[serial_test::serial] +async fn cleanup_empty_table_bucket_validates_directory_candidate_path_and_body_before_deletion() { + use crate::admin::storage_api::RUSTFS_META_BUCKET; + use crate::table_catalog::{TableCatalogObjectPaths, TableCatalogPutPrecondition}; + + let (_temp, _disks, storage) = crate::app::gating_test_env::isolated_multi_pool_ecstore().await; + let bucket = format!("cleanup-directory-{}", Uuid::new_v4().simple()); + let backend = crate::table_catalog::EcStoreTableCatalogObjectBackend::new_with_strong_runtime( + storage.clone(), + crate::table_catalog::StrongTableCatalogRuntime::default(), + ); + let catalog = crate::table_catalog::ConfiguredTableCatalogStore::new_for_test( + backend.clone(), + crate::table_catalog::TableCatalogBackingMode::ObjectBacked, + ); + let metadata = create_dropped_empty_tables_for_cleanup(&catalog, &backend, &storage, &bucket, 1).await; + let paths = TableCatalogObjectPaths::default(); + let bucket_entry = paths.table_bucket_entry_path(&bucket); + let directory = format!("{}/table-entry-directory/v1/", bucket_entry.rsplit_once('/').unwrap().0); + let real = format!("{directory}analytics/tables/events_00/table-entry.json"); + // Entry writers add candidates only while the directory is active; seed the + // one a backfill would have written so cleanup has a valid candidate to keep. + let real_target = format!("{}analytics/tables/events_00/table-entry.json", paths.namespace_entries_prefix(&bucket)); + backend + .put_object(RUSTFS_META_BUCKET, &real, real_target.into_bytes(), TableCatalogPutPrecondition::IfAbsent) + .await + .unwrap(); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &real).await.unwrap()); + for (suffix, target_suffix) in [ + ("bad segment/tables/x/table-entry.json", "bad segment/tables/x/table-entry.json"), + ("analytics/tables/forged/table-entry.json", "analytics/tables/events_00/table-entry.json"), + ] { + let unknown = format!("{directory}{suffix}"); + let target = format!("{}{target_suffix}", paths.namespace_entries_prefix(&bucket)); + backend + .put_object(RUSTFS_META_BUCKET, &unknown, target.into_bytes(), TableCatalogPutPrecondition::IfAbsent) + .await + .unwrap(); + let publication = TableCommitObjectBackend::trusted(backend.clone()); + assert!( + cleanup_empty_table_bucket_response(&catalog, &publication, &storage, &bucket) + .await + .is_err() + ); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &unknown).await.unwrap()); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &real).await.unwrap()); + assert!(backend.object_exists(&bucket, &metadata[0]).await.unwrap()); + backend.delete_object(RUSTFS_META_BUCKET, &unknown).await.unwrap(); + } + let publication = TableCommitObjectBackend::trusted(backend.clone()); + assert!( + !cleanup_empty_table_bucket_response(&catalog, &publication, &storage, &bucket) + .await + .unwrap() + .enabled + ); + assert!(!backend.object_exists(RUSTFS_META_BUCKET, &real).await.unwrap()); +} + #[tokio::test] #[serial_test::serial] async fn cleanup_empty_table_bucket_validates_authorization_and_metadata_before_deleting() { @@ -16705,7 +17262,7 @@ async fn cleanup_empty_table_bucket_handler_enforces_request_object_permissions( assert!(!backend.object_exists(&bucket, object).await.unwrap()); } } else { - assert_eq!(result.unwrap_err().code(), &S3ErrorCode::AccessDenied); + assert_eq!(result.expect_err("request must be denied").code(), &S3ErrorCode::AccessDenied); for object in &metadata_objects { assert!(backend.object_exists(&bucket, object).await.unwrap()); } @@ -17099,3 +17656,202 @@ async fn cleanup_empty_table_bucket_stops_before_delete_when_publication_lease_i ); object_store.delete_bucket(&bucket, &Default::default()).await.unwrap(); } + +#[tokio::test(start_paused = true)] +async fn publication_deadline_first_fenced_metadata_put_finishes_after_budget() { + use crate::table_catalog::{TableCatalogObjectBackend, TableCatalogPutPrecondition, TableCommitPublication}; + let backend = TestTableCatalogObjectBackend::content_addressed(); + let publication = TableCommitObjectBackend::trusted(backend.clone()); + publication.begin_table_bucket("warehouse").await.unwrap(); + let key = "generated.metadata.json"; + let pause = backend.pause_next_put("warehouse", key).await; + let worker = publication.clone(); + let put = tokio::spawn(async move { + worker + .put_object("warehouse", key, b"{}".to_vec(), TableCatalogPutPrecondition::IfAbsent) + .await + }); + pause.wait_started().await; + tokio::time::advance(crate::storage_api::table::get_lock_acquire_timeout() + StdDuration::from_millis(1)).await; + assert!(publication.holds_table_bucket("warehouse")); + assert!(!put.is_finished()); + pause.release(); + put.await.unwrap().unwrap(); + publication + .preparation + .read(&publication, backend.object_metadata("warehouse", key)) + .await + .unwrap() + .unwrap(); + assert!(publication.holds_table_bucket("warehouse")); + publication.complete(); +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_slow_head_clears_partial_http_preparation() { + use crate::table_catalog::test_support::TestCatalogObjectPause; + use crate::table_catalog::{TableCatalogObjectBackend, TableCommitPublication}; + let backend = TestTableCatalogObjectBackend::content_addressed(); + backend.seed_object("warehouse", "data/file.parquet", b"data".to_vec()).await; + let publication = TableCommitObjectBackend::trusted(backend.clone()); + publication.object_exists("warehouse", "data/file.parquet").await.unwrap(); + publication.begin_table_bucket("warehouse").await.unwrap(); + let pause = TestCatalogObjectPause::default(); + backend + .state + .lock() + .await + .pause_metadata + .insert(("warehouse".to_string(), "data/file.parquet".to_string()), pause.clone()); + let preparing = { + let publication = publication.clone(); + tokio::spawn(async move { publication.prepare("warehouse", "analytics", "events").await }) + }; + pause.wait_started().await; + tokio::time::advance(crate::storage_api::table::get_lock_acquire_timeout() + StdDuration::from_millis(1)).await; + assert!( + matches!(preparing.await.unwrap(), Err(crate::table_catalog::TableCatalogStoreError::Unavailable(message)) if message.contains("preparation deadline")) + ); + assert!(!publication.holds_table_bucket("warehouse")); + assert!(!publication.holds_table("warehouse", "analytics", "events")); + assert!(publication.publication.lock().observed_objects.is_empty()); + assert!(backend.state.lock().await.put_attempts.is_empty()); + pause.release(); + let _writer = backend + .acquire_read_lock("warehouse", &crate::table_catalog::default_table_bucket_publication_lock_path()) + .await + .unwrap(); +} + +#[tokio::test] +#[serial_test::serial] +async fn directory_management_handlers_enforce_migration_permission_before_mutation() { + temp_env::async_with_vars([ + (crate::table_catalog::ENV_TABLE_CATALOG_BACKING, Some("object")), + ("RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX", Some("true")), + ("RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX_FLEET_CONFIRMED", Some("true")), + ], async { + + let (_temp_dir, _disks, object_store) = crate::app::gating_test_env::isolated_multi_pool_ecstore().await; + object_store + .make_bucket("warehouse", &MakeBucketOptions::default()) + .await + .unwrap(); + rustfs_iam::store::object::ObjectStore::new(object_store.clone()) + .save_iam_config( + serde_json::json!({"version": 1}), + format!("{}/format.json", *rustfs_iam::store::object::IAM_CONFIG_PREFIX), + ) + .await + .unwrap(); + let iam = rustfs_iam::build_iam_sys(object_store.clone()).await.unwrap(); + let access_key = "directory-upgrade-user"; + let secret_key = "directory-upgrade-secret"; + iam.create_user( + access_key, + &AddOrUpdateUserReq { + secret_key: secret_key.to_string(), + policy: None, + status: AccountStatus::Enabled, + }, + ) + .await + .unwrap(); + let context = Arc::new(AppContext::new( + object_store.clone(), + Arc::new(RequestIam { handle: iam.clone() }), + Arc::new(RequestKms), + )); + assert!(context.publish_action_credentials(rustfs_credentials::Credentials { + access_key: "directory-root".to_string(), + secret_key: "directory-root-secret".to_string(), + status: "on".to_string(), + ..Default::default() + })); + let setup = table_catalog_handler_request(context.clone(), access_key, secret_key); + let backend = table_catalog_backend_from_extensions(&setup.extensions).unwrap(); + let configured = table_catalog_store_from_backend(backend.clone()).unwrap(); + enable_table_bucket_marker(object_store.as_ref(), "warehouse").await.unwrap(); + ensure_table_bucket_entry(&configured, "warehouse", true).await.unwrap(); + + for (name, policy, allowed) in [ + ( + "directory-maintenance", + serde_json::json!({"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["admin:RunTableMaintenance"]}]}), + false, + ), + ( + "directory-denied", + serde_json::json!({"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["admin:*"]},{"Effect":"Deny","Action":["admin:MigrateTableCatalog"]}]}), + false, + ), + ( + "directory-migrator", + serde_json::json!({"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["admin:MigrateTableCatalog"]}]}), + true, + ), + ] { + iam.set_policy(name, Policy::parse_config(&serde_json::to_vec(&policy).unwrap()).unwrap()) + .await + .unwrap(); + iam.policy_db_set(access_key, UserType::Reg, false, name).await.unwrap(); + for prefix in ["/iceberg/v1", "/_iceberg/v1"] { + let mut router = matchit::Router::new(); + router + .insert(format!("{prefix}/{{warehouse}}/catalog/directory"), ()) + .unwrap(); + let uri = format!("{prefix}/warehouse/catalog/directory"); + let mut request = table_catalog_handler_request(context.clone(), access_key, secret_key); + request.method = Method::DELETE; + request.uri = uri.parse().unwrap(); + let matched = router.at(&uri).unwrap(); + let result = DisableTableDirectoryHandler {}.call(request, matched.params).await; + if allowed { + assert_eq!(result.unwrap().output.0, StatusCode::OK); + } else { + assert_eq!(result.unwrap_err().code(), &S3ErrorCode::AccessDenied); + let mut request = table_catalog_handler_request(context.clone(), access_key, secret_key); + request.method = Method::POST; + request.uri = uri.parse().unwrap(); + let matched = router.at(&uri).unwrap(); + assert_eq!( + AdvanceTableDirectoryHandler {} + .call(request, matched.params) + .await + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + } + if allowed { + for expected_phase in ["building", "ready"] { + let mut request = table_catalog_handler_request(context.clone(), access_key, secret_key); + request.method = Method::POST; + request.uri = uri.parse().unwrap(); + let matched = router.at(&uri).unwrap(); + let response = AdvanceTableDirectoryHandler {}.call(request, matched.params).await.unwrap(); + assert_eq!(response.output.0, StatusCode::OK); + let bytes = http_body_util::BodyExt::collect(response.output.1).await.unwrap().to_bytes(); + let value: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(value["phase"], expected_phase); + } + let mut request = table_catalog_handler_request(context.clone(), access_key, secret_key); + request.method = Method::DELETE; + request.uri = uri.parse().unwrap(); + let matched = router.at(&uri).unwrap(); + assert_eq!(DisableTableDirectoryHandler {}.call(request, matched.params).await.unwrap().output.0, StatusCode::OK); + } + let object_catalog = crate::table_catalog::ObjectTableCatalogStore::new(backend.clone()); + assert!( + object_catalog + .table_directory_status("warehouse") + .await + .unwrap() + .phase + .is_none(), + "denied requests and idempotent rollback must not activate a directory" + ); + } + } + }).await; +} diff --git a/rustfs/src/admin/route_policy.rs b/rustfs/src/admin/route_policy.rs index 13a555a1c..76a639c4d 100644 --- a/rustfs/src/admin/route_policy.rs +++ b/rustfs/src/admin/route_policy.rs @@ -1132,6 +1132,24 @@ pub const ADMIN_ROUTE_POLICY_SPECS: &[AdminRouteSpec] = &[ GET_TABLE_BUCKET, RouteRiskLevel::Sensitive, ), + admin( + HttpMethod::Get, + "/iceberg/v1/{warehouse}/catalog/directory", + GET_TABLE_CATALOG, + RouteRiskLevel::Sensitive, + ), + admin( + HttpMethod::Post, + "/iceberg/v1/{warehouse}/catalog/directory", + MIGRATE_TABLE_CATALOG, + RouteRiskLevel::High, + ), + admin( + HttpMethod::Delete, + "/iceberg/v1/{warehouse}/catalog/directory", + MIGRATE_TABLE_CATALOG, + RouteRiskLevel::High, + ), admin( HttpMethod::Get, "/iceberg/v1/{warehouse}/catalog/migration", @@ -1440,6 +1458,24 @@ pub const ADMIN_ROUTE_POLICY_SPECS: &[AdminRouteSpec] = &[ GET_TABLE_BUCKET, RouteRiskLevel::Sensitive, ), + admin( + HttpMethod::Get, + "/_iceberg/v1/{warehouse}/catalog/directory", + GET_TABLE_CATALOG, + RouteRiskLevel::Sensitive, + ), + admin( + HttpMethod::Post, + "/_iceberg/v1/{warehouse}/catalog/directory", + MIGRATE_TABLE_CATALOG, + RouteRiskLevel::High, + ), + admin( + HttpMethod::Delete, + "/_iceberg/v1/{warehouse}/catalog/directory", + MIGRATE_TABLE_CATALOG, + RouteRiskLevel::High, + ), admin( HttpMethod::Get, "/_iceberg/v1/{warehouse}/catalog/migration", @@ -2125,7 +2161,7 @@ mod tests { let table_specs = ADMIN_ROUTE_POLICY_SPECS .iter() .filter(|spec| spec.path().starts_with("/iceberg/v1") || spec.path().starts_with("/_iceberg/v1")); - assert_eq!(table_specs.count(), 106); + assert_eq!(table_specs.count(), 112); assert_action(HttpMethod::Put, "/iceberg/v1/buckets/{warehouse}", SET_TABLE_BUCKET); assert_action(HttpMethod::Delete, "/iceberg/v1/buckets/{warehouse}", SET_TABLE_BUCKET); assert_action(HttpMethod::Delete, "/_iceberg/v1/buckets/{warehouse}", SET_TABLE_BUCKET); @@ -2304,6 +2340,12 @@ mod tests { "/_iceberg/v1/{warehouse}/namespaces/{namespace}/tables/{table}/maintenance/jobs/{job}/quarantine", RUN_TABLE_MAINTENANCE, ); + assert_action(HttpMethod::Get, "/iceberg/v1/{warehouse}/catalog/directory", GET_TABLE_CATALOG); + assert_action(HttpMethod::Post, "/iceberg/v1/{warehouse}/catalog/directory", MIGRATE_TABLE_CATALOG); + assert_action(HttpMethod::Delete, "/iceberg/v1/{warehouse}/catalog/directory", MIGRATE_TABLE_CATALOG); + assert_action(HttpMethod::Get, "/_iceberg/v1/{warehouse}/catalog/directory", GET_TABLE_CATALOG); + assert_action(HttpMethod::Post, "/_iceberg/v1/{warehouse}/catalog/directory", MIGRATE_TABLE_CATALOG); + assert_action(HttpMethod::Delete, "/_iceberg/v1/{warehouse}/catalog/directory", MIGRATE_TABLE_CATALOG); assert_action(HttpMethod::Get, "/iceberg/v1/{warehouse}/catalog/migration", GET_TABLE_CATALOG); assert_action(HttpMethod::Get, "/_iceberg/v1/{warehouse}/catalog/migration", GET_TABLE_CATALOG); assert_action(HttpMethod::Post, "/iceberg/v1/{warehouse}/catalog/migration", MIGRATE_TABLE_CATALOG); diff --git a/rustfs/src/storage_api.rs b/rustfs/src/storage_api.rs index bb77f0c1a..ebb098a56 100644 --- a/rustfs/src/storage_api.rs +++ b/rustfs/src/storage_api.rs @@ -483,6 +483,7 @@ pub(crate) mod workload { pub(crate) mod table { pub(crate) use crate::storage::storage_api::ecstore_bucket::utils::check_valid_bucket_name_strict; pub(crate) use crate::storage::storage_api::ecstore_object::{WriteCommitGuard, WriteCompletion}; + pub(crate) use crate::storage::storage_api::request_context_consumer::spawn_traced_join; pub(crate) mod contract { pub(crate) mod http { pub(crate) use super::super::super::storage_contracts::HTTPPreconditions; @@ -518,6 +519,13 @@ pub(crate) mod table { StorageObjectInfo, StorageObjectOptions, StorageObjectToDelete, StoragePutObjReader, get_bucket_metadata, get_lock_acquire_timeout, table_catalog_path_hash, }; + + #[cfg(test)] + pub(crate) mod test_support { + pub(crate) use crate::storage::storage_api::contract::bucket::{BucketOperations, MakeBucketOptions}; + pub(crate) use crate::storage::storage_api::ecstore_bucket::metadata_sys::init_bucket_metadata_sys; + pub(crate) use crate::storage::storage_api::ecstore_bucket::metadata_sys::test_support::isolated_store_over_temp_disks; + } } pub(crate) mod on_demand_migration { diff --git a/rustfs/src/table_catalog/iceberg/manifest.rs b/rustfs/src/table_catalog/iceberg/manifest.rs index a88eebfb8..0754e45ae 100644 --- a/rustfs/src/table_catalog/iceberg/manifest.rs +++ b/rustfs/src/table_catalog/iceberg/manifest.rs @@ -17,6 +17,7 @@ use std::io::Read; use super::super::*; const AVRO_ZSTANDARD_MAX_WINDOW_LOG: u32 = 27; +const AVRO_MANIFEST_MAX_SCHEMA_DEPTH: usize = 64; #[derive(Debug, Clone, PartialEq)] pub(crate) struct ManifestDataFileReference { @@ -182,6 +183,7 @@ pub(crate) fn decode_manifest_list_avro(data: &[u8]) -> TableCatalogStoreResult< let decoded_size = validate_avro_container(data)?; let reader = apache_avro::Reader::new(data) .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to read manifest list Avro: {err}")))?; + validate_manifest_writer_schema(reader.writer_schema())?; let format_version = avro_record_format_version(reader.writer_schema(), &["sequence_number", "min_sequence_number"], "manifest list")?; if format_version == 2 { @@ -276,6 +278,7 @@ pub(crate) fn decode_manifest_avro(data: &[u8]) -> TableCatalogStoreResult TableCatalogStoreResult<()> { + use apache_avro::schema::{Name, NamesRef, ResolvedSchema}; + use std::collections::{HashMap, HashSet}; + + fn depth( + schema: &apache_avro::Schema, + names: &NamesRef<'_>, + active: &mut HashSet, + heights: &mut HashMap, + nesting: usize, + ) -> TableCatalogStoreResult { + use apache_avro::Schema; + if nesting > AVRO_MANIFEST_MAX_SCHEMA_DEPTH { + return Err(TableCatalogStoreError::Invalid( + "manifest Avro schema exceeds the maximum nesting depth".to_string(), + )); + } + if let Schema::Ref { name } = schema { + let target = names.get(name).ok_or_else(|| { + TableCatalogStoreError::Invalid("manifest Avro schema has an unresolved type reference".to_string()) + })?; + return depth(target, names, active, heights, nesting); + } + if let Some(name) = schema.name() { + if let Some(height) = heights.get(name) { + if nesting + height - 1 > AVRO_MANIFEST_MAX_SCHEMA_DEPTH { + return Err(TableCatalogStoreError::Invalid( + "manifest Avro schema exceeds the maximum nesting depth".to_string(), + )); + } + return Ok(*height); + } + if !active.insert(name.clone()) { + return Err(TableCatalogStoreError::Invalid( + "manifest Avro schema contains a recursive type".to_string(), + )); + } + } + let mut height = 1; + let mut visit = |child| -> TableCatalogStoreResult<()> { + height = height.max(1 + depth(child, names, active, heights, nesting + 1)?); + Ok(()) + }; + match schema { + Schema::Record(record) => { + for field in &record.fields { + visit(&field.schema)?; + } + } + Schema::Array(array) => visit(&array.items)?, + Schema::Map(map) => visit(&map.types)?, + Schema::Union(union) => { + for variant in union.variants() { + visit(variant)?; + } + } + _ => {} + } + if let Some(name) = schema.name() { + active.remove(name); + heights.insert(name.clone(), height); + } + Ok(height) + } + + let resolved = ResolvedSchema::try_from(schema) + .map_err(|err| TableCatalogStoreError::Invalid(format!("invalid manifest Avro schema: {err}")))?; + depth(schema, resolved.get_names(), &mut HashSet::new(), &mut HashMap::new(), 1)?; + Ok(()) +} + fn validate_avro_container(data: &[u8]) -> TableCatalogStoreResult { if !data.starts_with(b"Obj\x01") { return Err(TableCatalogStoreError::Invalid("Avro container has invalid header magic".to_string())); @@ -665,6 +741,96 @@ mod tests { use super::*; use std::assert_matches; + fn schema_container(schema: &serde_json::Value, record: &[u8]) -> Vec { + fn long(bytes: &mut Vec, value: usize) { + let mut encoded = u64::try_from(value).expect("fixture length") << 1; + while encoded >= 128 { + bytes.push((encoded as u8 & 0x7f) | 0x80); + encoded >>= 7; + } + bytes.push(encoded as u8); + } + let schema = serde_json::to_vec(schema).expect("schema JSON"); + let mut bytes = b"Obj\x01".to_vec(); + long(&mut bytes, 1); + long(&mut bytes, b"avro.schema".len()); + bytes.extend_from_slice(b"avro.schema"); + long(&mut bytes, schema.len()); + bytes.extend_from_slice(&schema); + long(&mut bytes, 0); + bytes.extend_from_slice(&[0; 16]); + long(&mut bytes, 1); + long(&mut bytes, record.len()); + bytes.extend_from_slice(record); + bytes.extend_from_slice(&[0; 16]); + bytes + } + + #[test] + fn manifest_decoders_reject_recursive_writer_schemas_before_decoding_values() { + for child in [ + serde_json::json!(["null", "example.manifest_file"]), + serde_json::json!({"type":"array", "items":"example.manifest_file"}), + serde_json::json!({"type":"map", "values":"example.manifest_file"}), + serde_json::json!({"type":"record", "name":"child", "fields":[{"name":"parent", "type":"example.manifest_file"}]}), + ] { + let schema = serde_json::json!({ + "type":"record", "name":"manifest_file", "namespace":"example", + "fields":[{"name":"manifest_path","type":"string"}, {"name":"next", "type":child}] + }); + // A valid self-referential union datum can drive thousands of decoder frames. + let mut record = [0, 2].repeat(10_000); + record.extend_from_slice(&[0, 0]); + let data = schema_container(&schema, &record); + assert!( + matches!(decode_manifest_list_avro(&data), Err(TableCatalogStoreError::Invalid(message)) if message.contains("recursive type")) + ); + assert!( + matches!(decode_manifest_avro(&data), Err(TableCatalogStoreError::Invalid(message)) if message.contains("recursive type")) + ); + } + } + + #[test] + fn manifest_decoders_bound_resolved_schema_depth() { + let mut fields = vec![serde_json::json!({"name":"manifest_path", "type":"string"})]; + for index in 0..AVRO_MANIFEST_MAX_SCHEMA_DEPTH { + let children = if index == 0 { + vec![] + } else { + vec![serde_json::json!({"name":"previous", "type":format!("node{}", index - 1)})] + }; + fields.push(serde_json::json!({"name":format!("field{index}"), "type":{"type":"record", "name":format!("node{index}"), "fields":children}})); + } + let data = schema_container(&serde_json::json!({"type":"record", "name":"manifest_file", "fields":fields}), &[0]); + assert!( + matches!(decode_manifest_list_avro(&data), Err(TableCatalogStoreError::Invalid(message)) if message.contains("maximum nesting depth")) + ); + assert!( + matches!(decode_manifest_avro(&data), Err(TableCatalogStoreError::Invalid(message)) if message.contains("maximum nesting depth")) + ); + } + + #[test] + fn manifest_decoder_accepts_reused_nonrecursive_named_types() { + let schema = serde_json::json!({ + "type":"record", "name":"manifest_file", "namespace":"example", + "fields":[ + {"name":"manifest_path", "type":"string"}, + {"name":"first", "type":{"type":"record", "name":"detail", "fields":[{"name":"value", "type":"string"}]}}, + {"name":"second", "type":"detail"} + ] + }); + let path = "manifest.avro"; + let mut record = vec![u8::try_from(path.len() * 2).expect("short path")]; + record.extend_from_slice(path.as_bytes()); + record.extend_from_slice(&[0, 0]); + let decoded = + decode_manifest_list_avro(&schema_container(&schema, &record)).expect("reused named types are not recursive"); + assert_eq!(decoded.references.len(), 1); + assert_eq!(decoded.references[0].manifest_path, path); + } + #[test] fn rejects_v2_manifest_lists_without_required_count_fields() { let schema = apache_avro::Schema::parse_str( diff --git a/rustfs/src/table_catalog/maintenance/compaction.rs b/rustfs/src/table_catalog/maintenance/compaction.rs new file mode 100644 index 000000000..23079908b --- /dev/null +++ b/rustfs/src/table_catalog/maintenance/compaction.rs @@ -0,0 +1,924 @@ +// 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::super::*; +use datafusion::arrow::array::{ + Array, BinaryArray, BinaryViewArray, FixedSizeListArray, LargeBinaryArray, LargeListArray, LargeListViewArray, + LargeStringArray, ListArray, ListViewArray, MapArray, StringArray, StringViewArray, StructArray, downcast_dictionary_array, +}; +use datafusion::arrow::datatypes::{DataType, Field, Schema}; +use datafusion::parquet::arrow::{ + add_encoded_arrow_schema_to_metadata, arrow_reader::ArrowReaderOptions, arrow_writer::ArrowWriterOptions, +}; +use datafusion::parquet::file::properties::{EnabledStatistics, WriterProperties}; +use std::io::Write; +use std::sync::OnceLock; +use tokio::sync::{OwnedSemaphorePermit, Semaphore}; + +const COMPACTION_WRITE_BYTES: usize = 8 * 1024 * 1024; +const COMPACTION_BATCH_ROWS: usize = 256; +const COMPACTION_ROW_GROUP_ROWS: usize = 65_536; +const COMPACTION_MAX_ROW_GROUPS: usize = 256; +const COMPACTION_MAX_COLUMN_CHUNKS: usize = 16_384; +const COMPACTION_SCHEMA_DEPTH: usize = 64; +const COMPACTION_SCHEMA_FIELDS: usize = 1024; + +pub(crate) fn try_admit_compaction() -> TableCatalogStoreResult> { + static ADMISSION: OnceLock> = OnceLock::new(); + ADMISSION + .get_or_init(|| Arc::new(Semaphore::new(1))) + .clone() + .try_acquire_owned() + .map(Arc::new) + .map_err(|_| TableCatalogStoreError::Unavailable("table compaction capacity is full".to_owned())) +} + +fn budget_error() -> TableCatalogStoreError { + TableCatalogStoreError::Invalid("table compaction exceeds the server memory or expanded-value budget".to_owned()) +} + +fn add_budget(total: &mut usize, amount: usize, limit: usize) -> TableCatalogStoreResult<()> { + *total = total + .checked_add(amount) + .filter(|value| *value <= limit) + .ok_or_else(budget_error)?; + Ok(()) +} + +/// Sizes read from one Parquet page header (Thrift compact encoding). +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +struct PageHeaderSizes { + uncompressed: usize, + compressed: usize, +} + +const THRIFT_COMPACT_STOP: u8 = 0; +const THRIFT_COMPACT_BOOL_TRUE: u8 = 1; +const THRIFT_COMPACT_BOOL_FALSE: u8 = 2; +const THRIFT_COMPACT_BYTE: u8 = 3; +const THRIFT_COMPACT_I16: u8 = 4; +const THRIFT_COMPACT_I32: u8 = 5; +const THRIFT_COMPACT_I64: u8 = 6; +const THRIFT_COMPACT_DOUBLE: u8 = 7; +const THRIFT_COMPACT_BINARY: u8 = 8; +const THRIFT_COMPACT_LIST: u8 = 9; +const THRIFT_COMPACT_SET: u8 = 10; +const THRIFT_COMPACT_MAP: u8 = 11; +const THRIFT_COMPACT_STRUCT: u8 = 12; +const THRIFT_MAX_NESTING: usize = 16; +const PAGE_HEADER_FIELD_UNCOMPRESSED_SIZE: i16 = 2; +const PAGE_HEADER_FIELD_COMPRESSED_SIZE: i16 = 3; + +/// Minimal Thrift compact-protocol cursor: enough to read a `PageHeader`'s +/// size fields and skip everything else so the next header can be located. +struct ThriftCompactCursor<'a> { + bytes: &'a [u8], + pos: usize, +} + +impl ThriftCompactCursor<'_> { + fn error() -> TableCatalogStoreError { + TableCatalogStoreError::Invalid("compaction parquet page header is malformed".to_owned()) + } + + fn byte(&mut self) -> TableCatalogStoreResult { + let byte = *self.bytes.get(self.pos).ok_or_else(Self::error)?; + self.pos += 1; + Ok(byte) + } + + fn skip_bytes(&mut self, count: usize) -> TableCatalogStoreResult<()> { + self.pos = self + .pos + .checked_add(count) + .filter(|end| *end <= self.bytes.len()) + .ok_or_else(Self::error)?; + Ok(()) + } + + fn varint(&mut self) -> TableCatalogStoreResult { + let mut value = 0_u64; + for shift in (0..64).step_by(7) { + let byte = self.byte()?; + value |= u64::from(byte & 0x7f) << shift; + if byte & 0x80 == 0 { + return Ok(value); + } + } + Err(Self::error()) + } + + fn zigzag(&mut self) -> TableCatalogStoreResult { + let raw = self.varint()?; + Ok(((raw >> 1) as i64) ^ -((raw & 1) as i64)) + } + + fn skip_value(&mut self, kind: u8, depth: usize) -> TableCatalogStoreResult<()> { + if depth > THRIFT_MAX_NESTING { + return Err(Self::error()); + } + match kind { + THRIFT_COMPACT_BOOL_TRUE | THRIFT_COMPACT_BOOL_FALSE => Ok(()), + THRIFT_COMPACT_BYTE => self.skip_bytes(1), + THRIFT_COMPACT_I16 | THRIFT_COMPACT_I32 | THRIFT_COMPACT_I64 => self.varint().map(drop), + THRIFT_COMPACT_DOUBLE => self.skip_bytes(8), + THRIFT_COMPACT_BINARY => { + let len = usize::try_from(self.varint()?).map_err(|_| Self::error())?; + self.skip_bytes(len) + } + THRIFT_COMPACT_LIST | THRIFT_COMPACT_SET => { + let header = self.byte()?; + let element = header & 0x0f; + let count = match header >> 4 { + 15 => usize::try_from(self.varint()?).map_err(|_| Self::error())?, + short => usize::from(short), + }; + for _ in 0..count { + self.skip_value(element, depth + 1)?; + } + Ok(()) + } + THRIFT_COMPACT_MAP => { + let count = usize::try_from(self.varint()?).map_err(|_| Self::error())?; + if count == 0 { + return Ok(()); + } + let kinds = self.byte()?; + for _ in 0..count { + self.skip_value(kinds >> 4, depth + 1)?; + self.skip_value(kinds & 0x0f, depth + 1)?; + } + Ok(()) + } + THRIFT_COMPACT_STRUCT => self.skip_struct(depth + 1), + _ => Err(Self::error()), + } + } + + fn skip_struct(&mut self, depth: usize) -> TableCatalogStoreResult<()> { + let mut field_id = 0_i16; + loop { + let header = self.byte()?; + if header == THRIFT_COMPACT_STOP { + return Ok(()); + } + let kind = header & 0x0f; + field_id = match header >> 4 { + 0 => i16::try_from(self.zigzag()?).map_err(|_| Self::error())?, + delta => field_id.checked_add(i16::from(delta)).ok_or_else(Self::error)?, + }; + self.skip_value(kind, depth)?; + } + } + + /// Read a `PageHeader` struct, returning its sizes and leaving the cursor at + /// the first page byte. + fn page_header(&mut self) -> TableCatalogStoreResult { + let mut sizes = PageHeaderSizes::default(); + let mut field_id = 0_i16; + loop { + let header = self.byte()?; + if header == THRIFT_COMPACT_STOP { + return Ok(sizes); + } + let kind = header & 0x0f; + field_id = match header >> 4 { + 0 => i16::try_from(self.zigzag()?).map_err(|_| Self::error())?, + delta => field_id.checked_add(i16::from(delta)).ok_or_else(Self::error)?, + }; + match (field_id, kind) { + (PAGE_HEADER_FIELD_UNCOMPRESSED_SIZE, THRIFT_COMPACT_I32) => { + sizes.uncompressed = usize::try_from(self.zigzag()?).map_err(|_| Self::error())?; + } + (PAGE_HEADER_FIELD_COMPRESSED_SIZE, THRIFT_COMPACT_I32) => { + sizes.compressed = usize::try_from(self.zigzag()?).map_err(|_| Self::error())?; + } + _ => self.skip_value(kind, 1)?, + } + } + } +} + +/// Sum the page headers of one column chunk (`[start, end)` in the file) and +/// hold every page and the chunk total to `limit`. Pages are never decompressed. +fn validate_page_headers_in_range(chunk: &[u8], chunk_declared: usize, limit: usize) -> TableCatalogStoreResult { + let mut cursor = ThriftCompactCursor { bytes: chunk, pos: 0 }; + let mut total = 0_usize; + while cursor.pos < chunk.len() { + let sizes = cursor.page_header()?; + add_budget(&mut total, sizes.uncompressed, limit)?; + if total > chunk_declared.max(limit.min(chunk_declared.saturating_mul(2))) { + return Err(TableCatalogStoreError::Invalid( + "compaction parquet page headers exceed the footer's declared chunk size".to_owned(), + )); + } + cursor.skip_bytes(sizes.compressed)?; + } + Ok(total) +} + +fn validate_column_chunk_page_headers( + data: &Bytes, + column: &datafusion::parquet::file::metadata::ColumnChunkMetaData, + chunk_declared: usize, + limit: usize, + location: &str, +) -> TableCatalogStoreResult<()> { + let start = column + .dictionary_page_offset() + .map_or(column.data_page_offset(), |dictionary| dictionary.min(column.data_page_offset())); + let start = usize::try_from(start).map_err(|_| budget_error())?; + let length = usize::try_from(column.compressed_size()).map_err(|_| budget_error())?; + let end = start.checked_add(length).filter(|end| *end <= data.len()).ok_or_else(|| { + TableCatalogStoreError::Invalid(format!("compaction parquet {location} column chunk lies outside the file")) + })?; + validate_page_headers_in_range(&data[start..end], chunk_declared, limit).map_err(|err| match err { + TableCatalogStoreError::Invalid(message) => TableCatalogStoreError::Invalid(format!("{location}: {message}")), + other => other, + })?; + Ok(()) +} + +fn view_field(field: &Field, depth: usize, fields: &mut usize) -> TableCatalogStoreResult> { + add_budget(fields, 1, COMPACTION_SCHEMA_FIELDS)?; + Ok(Arc::new(field.clone().with_data_type(view_type(field.data_type(), depth, fields)?))) +} + +fn view_type(data_type: &DataType, depth: usize, fields: &mut usize) -> TableCatalogStoreResult { + if depth > COMPACTION_SCHEMA_DEPTH { + return Err(budget_error()); + } + Ok(match data_type { + DataType::Utf8 | DataType::LargeUtf8 => DataType::Utf8View, + DataType::Binary | DataType::LargeBinary => DataType::BinaryView, + DataType::List(field) => DataType::List(view_field(field, depth + 1, fields)?), + DataType::LargeList(field) => DataType::LargeList(view_field(field, depth + 1, fields)?), + DataType::ListView(field) => DataType::ListView(view_field(field, depth + 1, fields)?), + DataType::LargeListView(field) => DataType::LargeListView(view_field(field, depth + 1, fields)?), + DataType::FixedSizeList(field, size) => DataType::FixedSizeList(view_field(field, depth + 1, fields)?, *size), + DataType::Map(field, sorted) => DataType::Map(view_field(field, depth + 1, fields)?, *sorted), + DataType::Struct(children) => DataType::Struct( + children + .iter() + .map(|field| view_field(field, depth + 1, fields)) + .collect::>>()? + .into(), + ), + DataType::Dictionary(_, value) => { + // The pinned reader does not support Dictionary. Keep + // its already shared values, while still checking nested schema limits. + let _ = view_type(value, depth + 1, fields)?; + data_type.clone() + } + other => other.clone(), + }) +} + +fn view_schema(schema: &Schema) -> TableCatalogStoreResult { + let mut fields = 0; + let mapped = schema + .fields() + .iter() + .map(|field| view_field(field, 0, &mut fields)) + .collect::>>()?; + Ok(Arc::new(Schema::new_with_metadata(mapped, schema.metadata().clone()))) +} + +/// Charge the values the writer may expand, not the small view/dictionary +/// buffers. Even a null or empty nested cell consumes structural capacity. +fn charge_value(array: &dyn Array, row: usize, depth: usize, total: &mut usize, limit: usize) -> TableCatalogStoreResult<()> { + if depth > COMPACTION_SCHEMA_DEPTH { + return Err(budget_error()); + } + add_budget(total, 8, limit)?; + if array.is_null(row) { + return Ok(()); + } + macro_rules! scalar_bytes { + ($ty:ty) => { + add_budget( + total, + array + .as_any() + .downcast_ref::<$ty>() + .ok_or_else(budget_error)? + .value(row) + .len(), + limit, + ) + }; + } + macro_rules! nested_values { + ($ty:ty) => {{ + let value = array.as_any().downcast_ref::<$ty>().ok_or_else(budget_error)?.value(row); + for index in 0..value.len() { + charge_value(value.as_ref(), index, depth + 1, total, limit)?; + } + Ok(()) + }}; + } + match array.data_type() { + DataType::Utf8 => scalar_bytes!(StringArray), + DataType::LargeUtf8 => scalar_bytes!(LargeStringArray), + DataType::Utf8View => scalar_bytes!(StringViewArray), + DataType::Binary => scalar_bytes!(BinaryArray), + DataType::LargeBinary => scalar_bytes!(LargeBinaryArray), + DataType::BinaryView => scalar_bytes!(BinaryViewArray), + DataType::Boolean | DataType::Null => Ok(()), + DataType::FixedSizeBinary(size) => add_budget(total, usize::try_from(*size).map_err(|_| budget_error())?, limit), + DataType::List(_) => nested_values!(ListArray), + DataType::LargeList(_) => nested_values!(LargeListArray), + DataType::ListView(_) => nested_values!(ListViewArray), + DataType::LargeListView(_) => nested_values!(LargeListViewArray), + DataType::FixedSizeList(_, _) => nested_values!(FixedSizeListArray), + DataType::Map(_, _) => { + let value = array.as_any().downcast_ref::().ok_or_else(budget_error)?.value(row); + for index in 0..value.len() { + charge_value(&value, index, depth + 1, total, limit)?; + } + Ok(()) + } + DataType::Struct(_) => { + for column in array + .as_any() + .downcast_ref::() + .ok_or_else(budget_error)? + .columns() + { + charge_value(column.as_ref(), row, depth + 1, total, limit)?; + } + Ok(()) + } + DataType::Dictionary(_, _) => downcast_dictionary_array!(array => { + if let Some(key) = array.key(row) { charge_value(array.values().as_ref(), key, depth + 1, total, limit)?; } + Ok(()) + }, _ => Err(budget_error())), + data_type => add_budget(total, data_type.primitive_width().ok_or_else(budget_error)?, limit), + } +} + +struct BoundedOutput { + bytes: Vec, + limit: usize, +} +impl Write for BoundedOutput { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + let Some(required) = self.bytes.len().checked_add(bytes.len()).filter(|size| *size <= self.limit) else { + return Err(std::io::Error::other("compaction output exceeds the server size limit")); + }; + if required > self.bytes.capacity() { + // Parquet writes successive pages and buffered footer fragments. + // Amortize their copies without allocating past the output budget. + let capacity = required.max(self.bytes.capacity().saturating_mul(2)).min(self.limit); + self.bytes + .try_reserve_exact(capacity - self.bytes.len()) + .map_err(std::io::Error::other)?; + } + self.bytes.extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +pub(crate) fn compact_parquet_data_files(input_files: Vec<(String, Vec)>) -> TableCatalogStoreResult { + compact_parquet_with_limits(input_files, TABLE_COMPACTION_MAX_DECODED_BYTES, COMPACTION_WRITE_BYTES) +} + +fn compact_parquet_with_limits( + input_files: Vec<(String, Vec)>, + limit: usize, + write_limit: usize, +) -> TableCatalogStoreResult { + compact_parquet_with_budgets(input_files, limit, write_limit, COMPACTION_MAX_ROW_GROUPS, COMPACTION_ROW_GROUP_ROWS) +} + +fn compact_parquet_with_budgets( + input_files: Vec<(String, Vec)>, + limit: usize, + write_limit: usize, + group_limit: usize, + row_group_rows: usize, +) -> TableCatalogStoreResult { + let mut schema: Option = None; + let mut declared = 0; + let mut stored = 0; + let mut files = Vec::new(); + let mut leaf_columns = 0; + // Validate every footer before decoding any page or creating output. + for (location, data) in input_files { + add_budget(&mut stored, data.len(), TABLE_COMPACTION_MAX_STORED_BYTES)?; + let data = Bytes::from(data); + let builder = ParquetRecordBatchReaderBuilder::try_new(data.clone()) + .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to read compaction parquet {location}: {err}")))?; + let file_schema = builder.schema().clone(); + leaf_columns = leaf_columns.max(builder.metadata().file_metadata().schema_descr().num_columns()); + match &schema { + Some(expected) if expected.as_ref() != file_schema.as_ref() => { + return Err(TableCatalogStoreError::Invalid("compaction input parquet schemas must match".to_owned())); + } + None => schema = Some(file_schema), + _ => {} + } + for group in builder.metadata().row_groups() { + let mut columns = 0; + for column in group.columns() { + let chunk_declared = usize::try_from(column.uncompressed_size()).map_err(|_| budget_error())?; + add_budget(&mut columns, chunk_declared, limit)?; + // The footer is one claim; the page headers the decoder actually + // sizes its buffers from are another. Walk them without + // decompressing and hold both to the budget. + validate_column_chunk_page_headers(&data, column, chunk_declared, limit, &location)?; + } + let group_bytes = usize::try_from(group.total_byte_size()).map_err(|_| budget_error())?; + add_budget(&mut declared, group_bytes.max(columns), limit)?; + } + files.push((location, data)); + } + let schema = schema + .ok_or_else(|| TableCatalogStoreError::Invalid("compaction requires at least one parquet input file".to_owned()))?; + let read_schema = view_schema(&schema)?; + let group_limit = group_limit.min(COMPACTION_MAX_COLUMN_CHUNKS / leaf_columns.max(1)); + // These bounds include metadata retained until the footer is serialized. + // Page/offset indexes would add a separate, page-count-dependent inventory. + let mut properties = WriterProperties::builder() + .set_max_row_group_row_count(Some(row_group_rows)) + .set_statistics_enabled(EnabledStatistics::Chunk) + .set_statistics_truncate_length(Some(64)) + .set_offset_index_disabled(true) + .build(); + // The writer requires nested array types to match its input schema exactly. + // Views share the same Parquet physical/logical types; retain the original + // Arrow schema in the footer so readers recover its types and field metadata. + // Do not cast sliced lists/maps: Arrow casts their entire child buffers. + add_encoded_arrow_schema_to_metadata(&schema, &mut properties); + let options = ArrowWriterOptions::new() + .with_properties(properties) + .with_skip_arrow_metadata(true); + let mut writer = ArrowWriter::try_new_with_options( + BoundedOutput { + bytes: Vec::new(), + limit, + }, + read_schema.clone(), + options, + ) + .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to build compaction writer: {err}")))?; + let mut record_count = 0_u64; + let mut decoded = 0; + for (location, data) in files { + let options = ArrowReaderOptions::new().with_schema(read_schema.clone()); + let reader = ParquetRecordBatchReaderBuilder::try_new_with_options(data, options) + .and_then(|builder| builder.with_batch_size(COMPACTION_BATCH_ROWS).build()) + .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to build compaction reader {location}: {err}")))?; + for batch in reader { + let batch = batch + .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to decode compaction batch {location}: {err}")))?; + add_budget(&mut decoded, batch.get_array_memory_size(), limit)?; + let mut start = 0; + while start < batch.num_rows() { + let mut end = start; + let mut expanded = 0_usize; + let remaining_rows = row_group_rows.saturating_sub(writer.in_progress_rows()); + while end < batch.num_rows() && end - start < remaining_rows { + let mut row_bytes = 0; + for column in batch.columns() { + charge_value(column.as_ref(), end, 0, &mut row_bytes, write_limit)?; + } + if expanded.checked_add(row_bytes).is_none_or(|size| size > write_limit) { + break; + } + expanded += row_bytes; + end += 1; + } + if end == start { + return Err(budget_error()); + } + if writer + .memory_size() + .checked_add(expanded) + .is_none_or(|size| size > write_limit) + { + writer.flush().map_err(|err| { + TableCatalogStoreError::Invalid(format!("failed to flush bounded compaction output: {err}")) + })?; + } + if writer.flushed_row_groups().len() >= group_limit { + return Err(TableCatalogStoreError::Invalid( + "compaction output row-group metadata exceeds the server limit".to_owned(), + )); + } + writer.write(&batch.slice(start, end - start)).map_err(|err| { + TableCatalogStoreError::Invalid(format!("failed to write bounded compaction output: {err}")) + })?; + record_count = record_count.checked_add((end - start) as u64).ok_or_else(budget_error)?; + start = end; + } + } + } + let output = writer + .into_inner() + .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to finalize bounded compaction output: {err}")))?; + Ok(CompactedParquetFile { + data: output.bytes, + record_count, + }) +} + +pub(crate) async fn run_compaction_worker( + input_files: Vec<(String, Vec)>, + admission: Arc, +) -> TableCatalogStoreResult { + #[cfg(test)] + let barrier = COMPACTION_WORKER_BARRIER.try_with(Arc::clone).ok(); + tokio::task::spawn_blocking(move || { + let _admission = admission; + #[cfg(test)] + if let Some(barrier) = barrier { + barrier.entered.notify_one(); + let mut released = barrier.released.lock().unwrap(); + while !*released { + released = barrier.release.wait(released).unwrap(); + } + } + compact_parquet_data_files(input_files) + }) + .await + .map_err(|_| TableCatalogStoreError::Internal("compaction decoder worker failed".to_owned()))? +} + +pub(crate) async fn run_compaction_output(operation: F, admission: Arc) -> TableCatalogStoreResult<()> +where + F: std::future::Future> + Send + 'static, +{ + let credentials = rustfs_keystone::KEYSTONE_CREDENTIALS.try_with(Clone::clone).ok().flatten(); + let operation = rustfs_keystone::KEYSTONE_CREDENTIALS.scope(credentials, async move { + let _admission = admission; + operation.await + }); + let operation = rustfs_io_metrics::s3_http_metrics::with_s3_http_operation_context( + rustfs_io_metrics::s3_http_metrics::current_s3_http_operation_context(), + operation, + ); + crate::storage_api::table::spawn_traced_join(operation) + .await + .map_err(|_| TableCatalogStoreError::Internal("compaction output worker failed".to_owned()))? +} + +#[cfg(test)] +struct WorkerBarrier { + entered: tokio::sync::Notify, + released: std::sync::Mutex, + release: std::sync::Condvar, +} +#[cfg(test)] +tokio::task_local! { static COMPACTION_WORKER_BARRIER: Arc; } + +#[cfg(test)] +mod tests { + use super::*; + use datafusion::arrow::record_batch::RecordBatch; + use datafusion::parquet::basic::{Compression, ZstdLevel}; + + fn parquet_strings(value: &str, rows: usize, dictionary: bool) -> Vec { + let schema = Arc::new(Schema::new_with_metadata( + vec![Field::new("value", DataType::Utf8, false)], + [("custom-key".to_owned(), "preserved".to_owned())].into_iter().collect(), + )); + let values = StringArray::from(vec![value; rows]); + let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(values)]).unwrap(); + let properties = WriterProperties::builder() + .set_dictionary_enabled(dictionary) + .set_compression(Compression::ZSTD(ZstdLevel::default())) + .build(); + let mut writer = ArrowWriter::try_new(Vec::new(), schema, Some(properties)).unwrap(); + writer.write(&batch).unwrap(); + writer.into_inner().unwrap() + } + + fn compact_i32_field(field_delta: u8, value: i64) -> Vec { + let mut out = vec![(field_delta << 4) | THRIFT_COMPACT_I32]; + let mut zigzag = ((value << 1) ^ (value >> 63)) as u64; + loop { + let byte = (zigzag & 0x7f) as u8; + zigzag >>= 7; + if zigzag == 0 { + out.push(byte); + break; + } + out.push(byte | 0x80); + } + out + } + + #[test] + fn compaction_page_header_walker_matches_the_footer_of_an_honest_file() { + let file = Bytes::from(parquet_strings("value", 1024, true)); + let builder = ParquetRecordBatchReaderBuilder::try_new(file.clone()).unwrap(); + for group in builder.metadata().row_groups() { + for column in group.columns() { + let declared = usize::try_from(column.uncompressed_size()).unwrap(); + validate_column_chunk_page_headers(&file, column, declared, usize::MAX, "honest.parquet") + .expect("an honest file's page headers fit its footer"); + let start = usize::try_from(column.dictionary_page_offset().unwrap_or(column.data_page_offset())).unwrap(); + let end = start + usize::try_from(column.compressed_size()).unwrap(); + let total = validate_page_headers_in_range(&file[start..end], declared, usize::MAX).unwrap(); + // The footer's uncompressed size also counts the page headers themselves. + assert!(total > 0 && total <= declared, "page payload sizes ({total}) fit the footer's {declared}"); + } + } + } + + #[test] + fn compaction_page_header_walker_rejects_a_page_that_lies_about_its_size() { + // PageHeader { type: 0, uncompressed_page_size: 2 GiB - 1, compressed_page_size: 4 } + 4 bytes of "page". + let mut chunk = compact_i32_field(1, 0); + chunk.extend(compact_i32_field(1, i64::from(i32::MAX))); + chunk.extend(compact_i32_field(1, 4)); + // data_page_header struct (field 5, delta 2): one BOOL field, then STOP. + chunk.extend([ + (2 << 4) | THRIFT_COMPACT_STRUCT, + (1 << 4) | THRIFT_COMPACT_BOOL_TRUE, + THRIFT_COMPACT_STOP, + ]); + chunk.push(THRIFT_COMPACT_STOP); + chunk.extend([0_u8; 4]); + let error = validate_page_headers_in_range(&chunk, 4096, 64 * 1024 * 1024) + .expect_err("a page header above the budget must be rejected before any allocation"); + assert!(matches!(error, TableCatalogStoreError::Invalid(_)), "{error:?}"); + // The same header within budget but far above the footer's chunk size is also a lie. + let mut chunk = compact_i32_field(1, 0); + chunk.extend(compact_i32_field(1, 1024 * 1024)); + chunk.extend(compact_i32_field(1, 4)); + chunk.push(THRIFT_COMPACT_STOP); + chunk.extend([0_u8; 4]); + let error = validate_page_headers_in_range(&chunk, 4096, 64 * 1024 * 1024) + .expect_err("page headers far above the footer's chunk size are rejected"); + assert!(matches!(error, TableCatalogStoreError::Invalid(message) if message.contains("footer"))); + // Truncated header: the cursor must fail closed, never read past the chunk. + assert!(validate_page_headers_in_range(&compact_i32_field(1, 0), 4096, 4096).is_err()); + } + + #[test] + fn compaction_rejects_compressed_expansion_before_decoding_pages() { + let file = parquet_strings(&"x".repeat(64 * 1024), 32, false); + assert!(file.len() < 16 * 1024, "fixture must really be highly compressed"); + let error = compact_parquet_with_limits(vec![("bomb.parquet".to_owned(), file)], 128 * 1024, 64 * 1024) + .err() + .expect("footer must reject decoded expansion before allocation"); + assert!(matches!(error, TableCatalogStoreError::Invalid(_))); + } + + #[test] + fn compaction_dictionary_views_bound_each_writer_input_and_preserve_schema() { + let value = "x".repeat(16 * 1024); + let file = parquet_strings(&value, 32, true); + let original_schema = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(file.clone())) + .unwrap() + .schema() + .clone(); + let output = + compact_parquet_with_limits(vec![("dictionary.parquet".to_owned(), file)], 2 * 1024 * 1024, 32 * 1024).unwrap(); + assert_eq!(output.record_count, 32); + let builder = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(output.data)).unwrap(); + assert_eq!(builder.schema(), &original_schema); + assert!(builder.metadata().num_row_groups() <= 32); + for batch in builder.build().unwrap() { + let batch = batch.unwrap(); + let values = batch.column(0).as_any().downcast_ref::().unwrap(); + assert!(values.iter().all(|item| item == Some(value.as_str()))); + } + } + + #[test] + fn compaction_rejects_a_single_expanded_value_before_writer_allocation() { + let file = parquet_strings(&"x".repeat(4096), 16, true); + let error = compact_parquet_with_limits(vec![("single-row.parquet".to_owned(), file)], 1024 * 1024, 1024) + .err() + .expect("one expanded row cannot exceed the writer input budget"); + assert!(matches!(error, TableCatalogStoreError::Invalid(_))); + } + + #[test] + fn compaction_nested_views_keep_field_and_schema_metadata() { + use datafusion::arrow::array::{ListBuilder, StringBuilder}; + let child = Arc::new( + Field::new("text", DataType::Utf8, false) + .with_metadata([("PARQUET:field_id".to_owned(), "2".to_owned())].into_iter().collect()), + ); + let array = StructArray::from(vec![(child.clone(), Arc::new(StringArray::from(vec!["nested"; 4])) as Arc)]); + let value = "nested".repeat(32); + let mut lists = ListBuilder::new(StringBuilder::new()); + for _ in 0..4 { + lists.values().append_value(&value); + lists.append(true); + } + let lists = lists.finish(); + let schema = Arc::new(Schema::new_with_metadata( + vec![ + Field::new("outer", DataType::Struct(vec![child].into()), false), + Field::new("list", lists.data_type().clone(), false), + ], + [("source".to_owned(), "nested-compaction".to_owned())].into_iter().collect(), + )); + let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(array), Arc::new(lists)]).unwrap(); + let mut writer = ArrowWriter::try_new(Vec::new(), schema.clone(), None).unwrap(); + writer.write(&batch).unwrap(); + let file = writer.into_inner().unwrap(); + let rejected = compact_parquet_with_limits(vec![("nested.parquet".to_owned(), file.clone())], 1024 * 1024, 200) + .err() + .expect("a nested row must fit before reaching the writer"); + assert!(matches!(rejected, TableCatalogStoreError::Invalid(message) if message.contains("expanded-value budget"))); + // Force slices within the decoded nested batch. They must not require + // expanding all values in the retained child buffer before writing. + let output = compact_parquet_with_limits(vec![("nested.parquet".to_owned(), file)], 1024 * 1024, 300).unwrap(); + let builder = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(output.data)).unwrap(); + assert_eq!(builder.schema(), &schema); + let batches = builder.build().unwrap().collect::, _>>().unwrap(); + assert_eq!(batches.iter().map(|batch| batch.num_rows()).sum::(), 4); + for batch in batches { + let outer = batch.column(0).as_any().downcast_ref::().unwrap(); + let texts = outer.column(0).as_any().downcast_ref::().unwrap(); + assert!(texts.iter().all(|text| text == Some("nested"))); + let lists = batch.column(1).as_any().downcast_ref::().unwrap(); + for row in 0..batch.num_rows() { + let values = lists.value(row); + let values = values.as_any().downcast_ref::().unwrap(); + assert_eq!(values.len(), 1); + assert_eq!(values.value(0), value); + } + } + } + + #[test] + fn compaction_arrow_dictionary_schema_round_trips_through_view_reader() { + use datafusion::arrow::array::{DictionaryArray, Int8Array}; + use datafusion::arrow::datatypes::Int8Type; + let values = Arc::new(StringArray::from(vec!["dictionary-value".repeat(1024)])); + let array = DictionaryArray::::try_new(Int8Array::from(vec![0; 16]), values).unwrap(); + let field = Arc::new(Field::new("dictionary", array.data_type().clone(), false)); + let array = Arc::new(array); + let nested = StructArray::from(vec![(field.clone(), array.clone() as Arc)]); + let schema = Arc::new(Schema::new(vec![ + field.as_ref().clone(), + Field::new("nested", nested.data_type().clone(), false), + ])); + let batch = RecordBatch::try_new(schema.clone(), vec![array, Arc::new(nested)]).unwrap(); + let mut writer = ArrowWriter::try_new(Vec::new(), schema, None).unwrap(); + writer.write(&batch).unwrap(); + let file = writer.into_inner().unwrap(); + let expected = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(file.clone())) + .unwrap() + .schema() + .clone(); + let output = + compact_parquet_with_limits(vec![("arrow-dictionary.parquet".to_owned(), file)], 1024 * 1024, 40 * 1024).unwrap(); + assert_eq!(output.record_count, 16); + let reader = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(output.data)).unwrap(); + assert_eq!(reader.schema(), &expected); + let batches = reader.build().unwrap().collect::, _>>().unwrap(); + assert_eq!(batches.iter().map(RecordBatch::num_rows).sum::(), 16); + for batch in batches { + let nested = batch.column(1).as_any().downcast_ref::().unwrap(); + for column in [batch.column(0), nested.column(0)] { + let dictionary = column.as_any().downcast_ref::>().unwrap(); + let values = dictionary.values().as_any().downcast_ref::().unwrap(); + for row in 0..dictionary.len() { + assert_eq!(values.value(dictionary.key(row).unwrap()), "dictionary-value".repeat(1024)); + } + } + } + } + + #[test] + fn compaction_retained_row_group_metadata_is_bounded_before_another_group() { + let file = parquet_strings("", 1024, true); + let output = + compact_parquet_with_budgets(vec![("rle.parquet".to_owned(), file)], 1024 * 1024, 64 * 1024, 2, 512).unwrap(); + let reader = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(output.data)).unwrap(); + assert_eq!( + reader.metadata().num_row_groups(), + 2, + "256-row input batches must not force separate row groups" + ); + assert_eq!(output.record_count, 1024); + let file = parquet_strings("", 1025, true); + let error = compact_parquet_with_budgets(vec![("rle.parquet".to_owned(), file)], 1024 * 1024, 64 * 1024, 2, 512) + .err() + .expect("a third row group must not allocate retained metadata"); + assert!(matches!(error, TableCatalogStoreError::Invalid(message) if message.contains("row-group metadata"))); + } + + #[test] + fn compaction_output_sink_rejects_before_growing() { + let mut output = BoundedOutput { + bytes: Vec::new(), + limit: 4, + }; + output.write_all(b"1234").unwrap(); + let capacity = output.bytes.capacity(); + assert!(output.write_all(b"5").is_err()); + assert_eq!(&output.bytes, b"1234"); + assert_eq!(output.bytes.capacity(), capacity, "rejected writes must not allocate"); + } + + #[test] + fn compaction_output_sink_amortizes_small_writes_within_the_capacity_limit() { + // Include a non-power-of-two cap so the last growth must stop at the + // limit instead of using Vec's unconstrained doubling policy. + for chunk_size in [1, 7, 64] { + let mut output = BoundedOutput { + bytes: Vec::new(), + limit: 4099, + }; + let mut growths = 0; + let mut capacity = 0; + while output.bytes.len() < output.limit { + let count = chunk_size.min(output.limit - output.bytes.len()); + output.write_all(&[42; 64][..count]).expect("bounded fragment"); + if output.bytes.capacity() != capacity { + growths += 1; + capacity = output.bytes.capacity(); + } + assert!(capacity >= output.bytes.len() && capacity <= output.limit); + } + assert!(growths <= 14, "small writes must require logarithmic capacity growth, got {growths}"); + assert_eq!(output.bytes, vec![42; output.limit]); + assert!(output.write_all(b"x").is_err()); + assert_eq!(output.bytes.capacity(), capacity); + assert_eq!(output.bytes.len(), output.limit); + } + } + + #[tokio::test] + #[serial_test::serial(table_compaction)] + async fn compaction_output_worker_preserves_keystone_authorization_context() { + let credentials = rustfs_credentials::Credentials { + access_key: "keystone:compaction".to_owned(), + ..Default::default() + }; + let admission = try_admit_compaction().unwrap(); + rustfs_keystone::KEYSTONE_CREDENTIALS + .scope(Some(credentials), async { + run_compaction_output( + async { + assert_eq!( + rustfs_keystone::KEYSTONE_CREDENTIALS + .with(|credentials| credentials.as_ref().map(|value| value.access_key.clone())), + Some("keystone:compaction".to_owned()) + ); + Ok(()) + }, + admission, + ) + .await + .unwrap(); + }) + .await; + assert!(rustfs_keystone::KEYSTONE_CREDENTIALS.try_with(Clone::clone).is_err()); + } + + #[tokio::test] + #[serial_test::serial(table_compaction)] + async fn compaction_cancelled_caller_keeps_actual_decoder_worker_admitted() { + let barrier = Arc::new(WorkerBarrier { + entered: tokio::sync::Notify::new(), + released: std::sync::Mutex::new(false), + release: std::sync::Condvar::new(), + }); + let admission = try_admit_compaction().unwrap(); + let file = parquet_strings("valid", 4, true); + let scope_barrier = barrier.clone(); + let caller = tokio::spawn(COMPACTION_WORKER_BARRIER.scope(scope_barrier, async move { + run_compaction_worker(vec![("valid.parquet".to_owned(), file)], admission).await + })); + barrier.entered.notified().await; + caller.abort(); + let _ = caller.await; + assert!(matches!(try_admit_compaction(), Err(TableCatalogStoreError::Unavailable(_)))); + *barrier.released.lock().unwrap() = true; + barrier.release.notify_one(); + tokio::time::timeout(std::time::Duration::from_secs(30), async { + loop { + if try_admit_compaction().is_ok() { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("worker completion must return the permit"); + } +} diff --git a/rustfs/src/table_catalog/maintenance/mod.rs b/rustfs/src/table_catalog/maintenance/mod.rs index 723db7d8d..a0411dfb1 100644 --- a/rustfs/src/table_catalog/maintenance/mod.rs +++ b/rustfs/src/table_catalog/maintenance/mod.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +mod compaction; mod config; mod periodic; mod periodic_runtime; @@ -19,6 +20,7 @@ mod planner; mod recovery; mod worker; +pub(crate) use compaction::{run_compaction_output, run_compaction_worker, try_admit_compaction}; pub(crate) use config::*; pub(crate) use periodic_runtime::{PeriodicMaintenanceConfig, PeriodicMaintenanceRuntime}; pub(crate) use planner::*; diff --git a/rustfs/src/table_catalog/maintenance/planner.rs b/rustfs/src/table_catalog/maintenance/planner.rs index fdc6afaf0..ce6369c3c 100644 --- a/rustfs/src/table_catalog/maintenance/planner.rs +++ b/rustfs/src/table_catalog/maintenance/planner.rs @@ -402,10 +402,10 @@ where "compaction data file must be inside the table data directory".to_string(), )); } - let Some(data_object) = backend.read_object(table_bucket, &data_key).await? else { + let Some(data_metadata) = backend.object_metadata(table_bucket, &data_key).await? else { return Err(TableCatalogStoreError::NotFound(format!("compaction data file {data_key}"))); }; - let size_bytes = u64::try_from(data_object.data.len()).unwrap_or(u64::MAX); + let size_bytes = data_metadata.size; if size_bytes <= config.small_file_threshold_bytes { planning.candidates.push(TableCompactionDataFileCandidate { rewrite_prefix: compaction_data_file_rewrite_prefix_for_entry(entry, warehouse_object_prefix, &data_key) @@ -447,7 +447,7 @@ where )); } - let object_exists = backend.read_object(table_bucket, &delete_key).await?.is_some(); + let object_exists = backend.object_metadata(table_bucket, &delete_key).await?.is_some(); planning.status = TableRowLevelMaintenancePlanningStatus::ManualReviewRequired; planning.delete_file_count = planning.delete_file_count.saturating_add(1); planning.manual_review_count = planning.manual_review_count.saturating_add(1); @@ -519,6 +519,7 @@ pub(crate) async fn compaction_current_data_files( table: &IdentifierSegment, entry: &TableEntry, current_metadata: &serde_json::Value, + admission: &Arc, ) -> TableCatalogStoreResult> where B: TableCatalogObjectBackend, @@ -603,7 +604,7 @@ where "compaction data file must be inside the table data directory".to_string(), )); } - let Some(data_object) = backend.read_object(table_bucket, &data_key).await? else { + let Some(data_metadata) = backend.object_metadata(table_bucket, &data_key).await? else { return Err(TableCatalogStoreError::NotFound(format!("compaction data file {data_key}"))); }; let snapshot_id = reference @@ -625,14 +626,24 @@ where TableCatalogStoreError::Invalid("compaction manifest data file missing file sequence number".to_string()) })?; data_files.push(CompactedDataFile { - object_key: data_key, + object_key: data_key.clone(), file_path: reference.location, - file_size_bytes: reference - .file_size_bytes - .unwrap_or_else(|| u64::try_from(data_object.data.len()).unwrap_or(u64::MAX)), + file_size_bytes: reference.file_size_bytes.unwrap_or(data_metadata.size), record_count: match reference.record_count { Some(record_count) => record_count, - None => parquet_record_count(&data_object.data)?, + None => { + let object = backend + .read_object_limited(table_bucket, &data_key, TABLE_COMPACTION_MAX_STORED_BYTES) + .await? + .ok_or_else(|| TableCatalogStoreError::NotFound(format!("compaction data file {data_key}")))?; + let admission = Arc::clone(admission); + tokio::task::spawn_blocking(move || { + let _admission = admission; + parquet_record_count(object.data) + }) + .await + .map_err(|_| TableCatalogStoreError::Internal("compaction footer worker failed".to_owned()))?? + } }, partition_spec_id: manifest_reference.partition_spec_id.unwrap_or(0), partition: reference.partition, @@ -836,58 +847,6 @@ pub(crate) fn compaction_manifest_partition_spec_id(data_files: &[CompactedDataF Ok(partition_spec_id) } -pub(crate) fn compact_parquet_data_files(input_files: &[(String, Vec)]) -> TableCatalogStoreResult { - let mut schema: Option = None; - let mut batches = Vec::new(); - let mut record_count = 0_u64; - - for (location, data) in input_files { - let builder = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(data.clone())).map_err(|err| { - TableCatalogStoreError::Invalid(format!("failed to read compaction input parquet {location}: {err}")) - })?; - let file_schema = builder.schema().clone(); - match schema.as_ref() { - Some(expected_schema) if expected_schema.as_ref() != file_schema.as_ref() => { - return Err(TableCatalogStoreError::Invalid("compaction input parquet schemas must match".to_string())); - } - Some(_) => {} - None => schema = Some(file_schema), - } - - let reader = builder.build().map_err(|err| { - TableCatalogStoreError::Invalid(format!("failed to build compaction parquet reader {location}: {err}")) - })?; - for batch in reader { - let batch = batch.map_err(|err| { - TableCatalogStoreError::Invalid(format!("failed to read compaction parquet batch {location}: {err}")) - })?; - record_count = record_count.saturating_add(u64::try_from(batch.num_rows()).unwrap_or(u64::MAX)); - batches.push(batch); - } - } - - let Some(schema) = schema else { - return Err(TableCatalogStoreError::Invalid( - "compaction requires at least one parquet input file".to_string(), - )); - }; - let mut data = Vec::new(); - { - let mut writer = ArrowWriter::try_new(&mut data, schema, None) - .map_err(|err| TableCatalogStoreError::Internal(format!("failed to build compaction parquet writer: {err}")))?; - for batch in batches { - writer - .write(&batch) - .map_err(|err| TableCatalogStoreError::Internal(format!("failed to write compaction parquet batch: {err}")))?; - } - writer - .close() - .map_err(|err| TableCatalogStoreError::Internal(format!("failed to close compaction parquet writer: {err}")))?; - } - - Ok(CompactedParquetFile { data, record_count }) -} - fn validate_compaction_manifest_entry_status(entry_status: Option) -> TableCatalogStoreResult<()> { match entry_status { Some(0 | 1) => Ok(()), @@ -901,8 +860,8 @@ fn validate_compaction_manifest_entry_status(entry_status: Option) -> Table } } -fn parquet_record_count(data: &[u8]) -> TableCatalogStoreResult { - let builder = ParquetRecordBatchReaderBuilder::try_new(Bytes::copy_from_slice(data)) +fn parquet_record_count(data: Vec) -> TableCatalogStoreResult { + let builder = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(data)) .map_err(|err| TableCatalogStoreError::Invalid(format!("failed to read compaction parquet metadata: {err}")))?; u64::try_from(builder.metadata().file_metadata().num_rows()) .map_err(|_| TableCatalogStoreError::Invalid("compaction parquet record count must not be negative".to_string())) diff --git a/rustfs/src/table_catalog/maintenance/recovery.rs b/rustfs/src/table_catalog/maintenance/recovery.rs index a60b9600c..23af4616a 100644 --- a/rustfs/src/table_catalog/maintenance/recovery.rs +++ b/rustfs/src/table_catalog/maintenance/recovery.rs @@ -72,6 +72,22 @@ fn insert_referenced_object_report( report.reasons.insert(reason); } +pub(crate) fn maintenance_cleanup_roots(entry: &TableEntry) -> TableCatalogStoreResult> { + let metadata_dir = table_metadata_dir_path_for_entry(entry)?; + let table_root = metadata_dir + .strip_suffix(&format!("/{METADATA_DIR}")) + .ok_or_else(|| TableCatalogStoreError::Invalid("maintenance metadata directory has no table root".to_string()))?; + let warehouse = table_warehouse_object_prefix(entry)?; + Ok(vec![ + format!("{metadata_dir}/"), + format!("{table_root}/{DATA_DIR}/"), + format!("{table_root}/{DELETE_DIR}/"), + format!("{warehouse}{METADATA_DIR}/"), + format!("{warehouse}{DATA_DIR}/"), + format!("{warehouse}{DELETE_DIR}/"), + ]) +} + #[derive(Clone, Copy)] enum TableMaintenanceObjectScope<'a> { Logical { @@ -795,7 +811,7 @@ where table_metadata_maintenance_reason_for_object_kind(&object_kind), TableMetadataMaintenanceReason::NoCurrentReachability, ]); - let state = match backend.read_object(table_bucket, &object_location).await? { + let state = match backend.object_metadata(table_bucket, &object_location).await? { Some(object) if metadata_candidate_is_past_safety_window(object.mod_time, now) => { reasons.insert(TableMetadataMaintenanceReason::SafetyWindowSatisfied); cleanup_candidate_locations.push(object_location.clone()); diff --git a/rustfs/src/table_catalog/mod.rs b/rustfs/src/table_catalog/mod.rs index b105bb305..d50c6628a 100644 --- a/rustfs/src/table_catalog/mod.rs +++ b/rustfs/src/table_catalog/mod.rs @@ -110,6 +110,13 @@ pub(crate) const TABLE_MAINTENANCE_CONFIG_VERSION: u16 = 1; pub(crate) const TABLE_EXTERNAL_CATALOG_BRIDGE_VERSION: u16 = 1; pub(crate) const TABLE_CATALOG_BACKING_MANIFEST_VERSION: u16 = 1; pub(crate) const ENV_TABLE_CATALOG_BACKING: &str = "RUSTFS_TABLE_CATALOG_BACKING"; +/// Seconds a create, register, or commit may spend preparing (queueing for the +/// bucket publication fence, validating references, inventories) before it is +/// refused with 503 so object writes in the bucket are admitted. Never below +/// the object lock-acquire timeout. +pub(crate) const ENV_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS: &str = + "RUSTFS_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS"; +pub(crate) const DEFAULT_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS: u64 = 30; pub(crate) const ENV_TABLE_CATALOG_PUBLICATION_FENCE_FLEET_CONFIRMED: &str = "RUSTFS_TABLE_CATALOG_PUBLICATION_FENCE_FLEET_CONFIRMED"; pub(crate) const ENV_TABLE_CATALOG_STRONG_SNAPSHOT_V2: &str = "RUSTFS_TABLE_CATALOG_STRONG_SNAPSHOT_V2"; @@ -130,6 +137,8 @@ const TABLE_COMMIT_MAX_AVRO_BYTES: usize = 512 * 1024 * 1024; const TABLE_COMMIT_MAX_FILE_REFERENCES: usize = 1_000_000; const TABLE_COMMIT_MAX_STATISTICS_OBJECTS: usize = 1_024; const TABLE_COMMIT_MAX_STATISTICS_BYTES: usize = 512 * 1024 * 1024; +const TABLE_COMPACTION_MAX_STORED_BYTES: usize = 512 * 1024 * 1024; +const TABLE_COMPACTION_MAX_DECODED_BYTES: usize = 512 * 1024 * 1024; const TABLE_STATISTICS_FILE_MAX_SIZE: usize = 128 * 1024 * 1024; pub(crate) const TABLE_COMMIT_OBJECT_VALIDATION_CONCURRENCY: usize = 16; pub const TABLE_RESERVED_PREFIX: &str = BUCKET_TABLE_RESERVED_PREFIX; @@ -173,6 +182,10 @@ const WAREHOUSE_INDEX_STATE_FILE: &str = "state.json"; const TABLE_RENAME_ROOT: &str = "renames"; const WAREHOUSE_INDEX_MAX_PREFIX_DEPTH: usize = 64; const TABLE_DATA_PLANE_INDEX_MISS_SCAN_MAX_CATALOG_OBJECTS: usize = 4096; +/// Listing pages an index-miss scan may consume before failing closed. Only +/// table entries count toward the object limit, so maintenance reports and +/// other catalog objects cannot trip it; this bounds the listing work itself. +const TABLE_DATA_PLANE_INDEX_MISS_SCAN_MAX_PAGES: usize = 64; const EXTERNAL_CATALOG_ROOT: &str = "external-catalog"; const EXTERNAL_CATALOG_BRIDGE_FILE: &str = "bridge.json"; const MAINTENANCE_ROOT: &str = "maintenance"; diff --git a/rustfs/src/table_catalog/store/cleanup.rs b/rustfs/src/table_catalog/store/cleanup.rs index 086a38930..378ebc60f 100644 --- a/rustfs/src/table_catalog/store/cleanup.rs +++ b/rustfs/src/table_catalog/store/cleanup.rs @@ -131,6 +131,7 @@ impl ObjectTableCatalogStore { let bucket_path = self.paths.table_bucket_entry_path(bucket); let index_state_path = self.paths.warehouse_index_state_path(bucket); let index_prefix = self.paths.warehouse_index_entries_prefix(bucket); + let directory_prefix = self.paths.table_directory_prefix(bucket); let mut indexes = Vec::new(); for object in catalog_objects { if object.delete_marker @@ -173,6 +174,14 @@ impl ObjectTableCatalogStore { return Err(cleanup_conflict("cleanup requires the default table warehouse location")); } etag + } else if object.key.starts_with(&directory_prefix) { + // Registration/rename share the held bucket publication fence. + // Remove only the exact immutable candidate of a vanished entry. + let (target, etag) = self.directory_candidate_target(bucket, &object.key).await?; + if self.backend.object_exists(self.catalog_bucket(), &target).await? { + return Err(cleanup_conflict("table directory candidate references a retained table entry")); + } + etag } else { return Err(cleanup_conflict("table bucket contains namespaces or retained catalog resources")); }; diff --git a/rustfs/src/table_catalog/store/directory_index.rs b/rustfs/src/table_catalog/store/directory_index.rs new file mode 100644 index 000000000..fb957fe52 --- /dev/null +++ b/rustfs/src/table_catalog/store/directory_index.rs @@ -0,0 +1,1334 @@ +// 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::object::validate_table_entry_object; +use super::*; + +const DIRECTORY_KIND: &str = "table-entry-directory"; +const DIRECTORY_VERSION: u16 = 1; +const DIRECTORY_PAGE_SIZE: usize = 128; +const DIRECTORY_STATE_MAX_SIZE: usize = 16 * 1024; +const ENV_DIRECTORY_ENABLE: &str = "RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX"; +const ENV_DIRECTORY_FLEET: &str = "RUSTFS_TABLE_CATALOG_DIRECTORY_INDEX_FLEET_CONFIRMED"; + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum TableDirectoryPhase { + Building, + Ready, +} + +// This occupies the existing migration fence path. Old binaries reject the +// unknown fields and cannot publish an entry without its directory candidate. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub(super) struct TableDirectoryState { + kind: String, + version: u16, + table_bucket: String, + activation_id: Uuid, + phase: TableDirectoryPhase, + cursor: Option, + prune_cursor: Option, + #[serde(skip)] + etag: Option, +} + +#[derive(Debug, Serialize)] +pub(crate) struct TableDirectoryStatus { + pub(crate) phase: Option, + pub(crate) scanned_objects: usize, + pub(crate) removed_candidates: usize, +} + +// A directory read does not acquire publication or migration locks. Callers +// retain their existing synchronization; the activation epoch detects rollback +// and reactivation without recursively taking an already-held fair lock. +pub(super) struct TableDirectoryScan { + pub(super) objects: Vec, + pub(super) activation_id: Uuid, +} + +impl TableCatalogObjectPaths { + pub(super) fn table_directory_prefix(&self, table_bucket: &str) -> String { + format!("{}table-entry-directory/v1/", self.table_bucket_root_prefix(table_bucket)) + } + + fn table_directory_candidate(&self, entry: &TableEntry) -> TableCatalogStoreResult { + let namespace = parse_namespace_for_store(&entry.namespace)?; + let table = parse_table_for_store(&entry.table)?; + Ok(format!( + "{}{}/tables/{}/{}", + self.table_directory_prefix(&entry.table_bucket), + namespace.storage_id(), + table.as_str(), + TABLE_ENTRY_FILE + )) + } + + fn table_directory_progress_lock(&self, table_bucket: &str) -> String { + format!("{}table-entry-directory/progress.lock", self.table_bucket_root_prefix(table_bucket)) + } +} + +async fn run_table_directory_worker(worker: F) -> TableCatalogStoreResult +where + F: std::future::Future> + Send + 'static, + T: Send + 'static, +{ + let credentials = rustfs_keystone::KEYSTONE_CREDENTIALS.try_with(Clone::clone).ok().flatten(); + let worker = rustfs_keystone::KEYSTONE_CREDENTIALS.scope(credentials, worker); + let worker = rustfs_io_metrics::s3_http_metrics::with_s3_http_operation_context( + rustfs_io_metrics::s3_http_metrics::current_s3_http_operation_context(), + worker, + ); + crate::storage_api::table::spawn_traced_join(worker) + .await + .map_err(|_| TableCatalogStoreError::Internal("table directory worker failed".to_string()))? +} + +impl ObjectTableCatalogStore { + pub(super) async fn read_table_directory_state( + &self, + table_bucket: &str, + ) -> TableCatalogStoreResult> { + let path = self.paths.backing_migration_fence_path(table_bucket); + let Some(object) = self + .backend + .read_object_limited(self.catalog_bucket(), &path, DIRECTORY_STATE_MAX_SIZE) + .await? + else { + return Ok(None); + }; + let value: serde_json::Value = serde_json::from_slice(&object.data) + .map_err(|err| TableCatalogStoreError::Invalid(format!("invalid catalog migration fence: {err}")))?; + if value.get("kind").and_then(serde_json::Value::as_str) != Some(DIRECTORY_KIND) { + return Ok(None); + } + let mut state: TableDirectoryState = serde_json::from_value(value) + .map_err(|err| TableCatalogStoreError::Invalid(format!("invalid table directory state: {err}")))?; + if state.version != DIRECTORY_VERSION + || state.activation_id.is_nil() + || state.table_bucket != table_bucket + || state + .cursor + .as_ref() + .is_some_and(|cursor| !cursor.starts_with(&self.paths.namespace_entries_prefix(table_bucket))) + || state + .prune_cursor + .as_ref() + .is_some_and(|cursor| !cursor.starts_with(&self.paths.table_directory_prefix(table_bucket))) + || (state.phase == TableDirectoryPhase::Ready && state.cursor.is_some()) + { + return Err(TableCatalogStoreError::Invalid( + "table directory state identity or version is invalid".to_string(), + )); + } + state.etag = + Some(object.etag.ok_or_else(|| { + TableCatalogStoreError::Unavailable("directory state has no conditional-write ETag".to_string()) + })?); + Ok(Some(state)) + } + + pub(crate) async fn table_directory_status(&self, table_bucket: &str) -> TableCatalogStoreResult { + self.require_table_bucket(table_bucket).await?; + Ok(TableDirectoryStatus { + phase: self.read_table_directory_state(table_bucket).await?.map(|state| state.phase), + scanned_objects: 0, + removed_candidates: 0, + }) + } + + // The locked entry writer calls this before the authoritative put. The + // original entry remains authoritative in every phase, including rollback. + pub(in crate::table_catalog) async fn prepare_table_directory_candidate( + &self, + bucket: &str, + object: &str, + data: &[u8], + ) -> TableCatalogStoreResult<()> { + if bucket != self.catalog_bucket() || !object.ends_with(&format!("/{TABLE_ENTRY_FILE}")) { + return Ok(()); + } + let entry: TableEntry = serde_json::from_slice(data) + .map_err(|err| TableCatalogStoreError::Invalid(format!("invalid table entry for directory publication: {err}")))?; + validate_table_entry_object(&self.paths, object, &entry)?; + // Without an activated directory there is nothing to keep consistent, and + // activation backfills every existing entry; writing candidates regardless + // would grow an unreclaimed set on every cluster. + if self.read_table_directory_state(&entry.table_bucket).await?.is_none() { + return Ok(()); + } + let candidate = self.paths.table_directory_candidate(&entry)?; + let expected = object.as_bytes(); + if let Some(current) = self + .backend + .read_object_limited(bucket, &candidate, DIRECTORY_STATE_MAX_SIZE) + .await? + { + if current.data.as_slice() != expected { + return Err(TableCatalogStoreError::Invalid( + "table directory candidate has an invalid target".to_string(), + )); + } + return Ok(()); + } + match self + .backend + .put_object(bucket, &candidate, expected.to_vec(), TableCatalogPutPrecondition::IfAbsent) + .await + { + Ok(()) => Ok(()), + Err(error) => { + // Concurrent backfill and an entry writer may add the same + // immutable candidate. Only the exact persisted target proves it. + if self + .backend + .read_object_limited(bucket, &candidate, DIRECTORY_STATE_MAX_SIZE) + .await? + .is_some_and(|current| current.data.as_slice() == expected) + { + Ok(()) + } else { + Err(error) + } + } + } + } + + pub(crate) async fn advance_table_directory(&self, table_bucket: &str) -> TableCatalogStoreResult { + self.advance_table_directory_with_confirmation( + table_bucket, + rustfs_utils::get_env_bool(ENV_DIRECTORY_ENABLE, false), + rustfs_utils::get_env_bool(ENV_DIRECTORY_FLEET, false), + ) + .await + } + + async fn advance_table_directory_with_confirmation( + &self, + table_bucket: &str, + enabled: bool, + fleet_confirmed: bool, + ) -> TableCatalogStoreResult { + let owner = self.clone(); + let table_bucket = table_bucket.to_string(); + // Once a page starts, keep its migration/progress guards through the + // durable tail even when the HTTP caller disconnects. In particular an + // initial IfAbsent must not arrive after rollback has observed absence. + run_table_directory_worker(async move { + owner + .advance_table_directory_owned(&table_bucket, enabled, fleet_confirmed) + .await + }) + .await + } + + async fn advance_table_directory_owned( + &self, + table_bucket: &str, + enabled: bool, + fleet_confirmed: bool, + ) -> TableCatalogStoreResult { + self.require_table_bucket(table_bucket).await?; + let fence_path = self.paths.backing_migration_fence_path(table_bucket); + if self.read_table_directory_state(table_bucket).await?.is_none() { + if !enabled || !fleet_confirmed { + return Err(TableCatalogStoreError::Conflict( + "directory activation requires both directory-index and fleet-confirmed settings after the compatibility rollout".to_string(), + )); + } + // Drain in-flight legacy writers before publishing the durable + // capability fence. Readers still use the original catalog. + let guard = self + .backend + .acquire_write_lock(self.catalog_bucket(), &self.paths.backing_migration_fence_lock_path(table_bucket)) + .await?; + if self + .backend + .object_metadata(self.catalog_bucket(), &fence_path) + .await? + .is_none() + { + let state = TableDirectoryState { + kind: DIRECTORY_KIND.to_string(), + version: DIRECTORY_VERSION, + table_bucket: table_bucket.to_string(), + activation_id: Uuid::new_v4(), + phase: TableDirectoryPhase::Building, + cursor: None, + prune_cursor: None, + etag: None, + }; + self.write_table_directory_state( + &state, + TableCatalogPutPrecondition::IfAbsent, + TableCatalogObjectMutationFence::from_signals([guard.lock_lost_signal()]), + ) + .await?; + return Ok(TableDirectoryStatus { + phase: Some(state.phase), + scanned_objects: 0, + removed_candidates: 0, + }); + } + if self.read_table_directory_state(table_bucket).await?.is_none() { + return Err(TableCatalogStoreError::Conflict( + "table bucket has an active backing migration".to_string(), + )); + } + } + + let permit = self.acquire_object_backed_catalog_write_permit(table_bucket).await?; + let progress = self + .backend + .acquire_write_lock(self.catalog_bucket(), &self.paths.table_directory_progress_lock(table_bucket)) + .await?; + let mut state = self + .read_table_directory_state(table_bucket) + .await? + .ok_or_else(|| TableCatalogStoreError::Conflict("directory activation was cancelled".to_string()))?; + if state.phase == TableDirectoryPhase::Ready { + return self.prune_table_directory_page(state, permit, progress).await; + } + let prefix = self.paths.namespace_entries_prefix(table_bucket); + let page = self + .backend + .list_catalog_objects_page( + self.catalog_bucket(), + &prefix, + state.cursor.as_deref(), + NonZeroUsize::new(DIRECTORY_PAGE_SIZE) + .ok_or_else(|| TableCatalogStoreError::Internal("directory page size must be positive".to_string()))?, + ) + .await?; + let page = catalog_page_names(page)?; + validate_directory_page(&page, &prefix, state.cursor.as_deref(), DIRECTORY_PAGE_SIZE)?; + for object in &page.objects { + if object.ends_with(&format!("/{TABLE_ENTRY_FILE}")) + && let Some((entry, _)) = self.read_entry::(self.catalog_bucket(), object).await? + { + validate_table_entry_object(&self.paths, object, &entry)?; + let data = serde_json::to_vec(&entry).map_err(|err| TableCatalogStoreError::Internal(err.to_string()))?; + self.prepare_table_directory_candidate(self.catalog_bucket(), object, &data) + .await?; + } + } + state.cursor = page.objects.last().cloned(); + if !page.is_truncated { + state.phase = TableDirectoryPhase::Ready; + state.cursor = None; + } + // A cancelled page can leave candidates, but never advances the cursor + // before they are durable. Writers behind the cursor publish candidates + // first, so reaching EOF does not require a stop-the-world rescan. + self.write_table_directory_state( + &state, + directory_state_precondition(&state)?, + TableCatalogObjectMutationFence::from_signals([permit.lock_lost_signal(), progress.lock_lost_signal()]), + ) + .await?; + Ok(TableDirectoryStatus { + phase: Some(state.phase), + scanned_objects: page.objects.len(), + removed_candidates: 0, + }) + } + + pub(crate) async fn disable_table_directory(&self, table_bucket: &str) -> TableCatalogStoreResult { + let owner = self.clone(); + let table_bucket = table_bucket.to_string(); + run_table_directory_worker(async move { owner.disable_table_directory_owned(&table_bucket).await }).await + } + + async fn disable_table_directory_owned(&self, table_bucket: &str) -> TableCatalogStoreResult { + self.require_table_bucket(table_bucket).await?; + let guard = self + .backend + .acquire_write_lock(self.catalog_bucket(), &self.paths.backing_migration_fence_lock_path(table_bucket)) + .await?; + if self.read_table_directory_state(table_bucket).await?.is_some() { + // Entry publication has always retained the legacy objects. Exclude + // backfill and all writers until the old authority is restored. + let path = self.paths.backing_migration_fence_path(table_bucket); + let etag = self + .backend + .object_metadata(self.catalog_bucket(), &path) + .await? + .and_then(|metadata| metadata.etag) + .ok_or_else(|| TableCatalogStoreError::Unavailable("directory state has no deletion ETag".to_string()))?; + let fence = TableCatalogObjectMutationFence::from_signals([guard.lock_lost_signal()]); + self.backend + .delete_object_if_match_fenced(self.catalog_bucket(), &path, &etag, &fence) + .await?; + } else if self + .backend + .object_metadata(self.catalog_bucket(), &self.paths.backing_migration_fence_path(table_bucket)) + .await? + .is_some() + { + return Err(TableCatalogStoreError::Conflict( + "cannot cancel a backing migration through directory rollback".to_string(), + )); + } + Ok(TableDirectoryStatus { + phase: None, + scanned_objects: 0, + removed_candidates: 0, + }) + } + + async fn write_table_directory_state( + &self, + state: &TableDirectoryState, + precondition: TableCatalogPutPrecondition, + fence: TableCatalogObjectMutationFence, + ) -> TableCatalogStoreResult<()> { + let data = serde_json::to_vec(state).map_err(|err| TableCatalogStoreError::Internal(err.to_string()))?; + self.backend + .put_object_fenced( + self.catalog_bucket(), + &self.paths.backing_migration_fence_path(&state.table_bucket), + data, + precondition, + &fence, + ) + .await + } + + async fn prune_table_directory_page( + &self, + mut state: TableDirectoryState, + permit: TableCatalogLockGuard, + progress: TableCatalogLockGuard, + ) -> TableCatalogStoreResult { + let prefix = self.paths.table_directory_prefix(&state.table_bucket); + let page = self + .backend + .list_catalog_objects_page( + self.catalog_bucket(), + &prefix, + state.prune_cursor.as_deref(), + NonZeroUsize::new(DIRECTORY_PAGE_SIZE) + .ok_or_else(|| TableCatalogStoreError::Internal("directory page size must be positive".to_string()))?, + ) + .await?; + let page = catalog_page_names(page)?; + validate_directory_page(&page, &prefix, state.prune_cursor.as_deref(), DIRECTORY_PAGE_SIZE)?; + let mut removed = 0; + for candidate in &page.objects { + let (target, candidate_etag) = self.directory_candidate_target(&state.table_bucket, candidate).await?; + // A creator holds the authoritative entry's write lock before + // publishing its candidate. Retain this read lock through removal, + // so pruning never erases a concurrent creator's only candidate. + let entry_guard = self.backend.acquire_read_lock(self.catalog_bucket(), &target).await?; + let current = self.read_entry_unlocked::(self.catalog_bucket(), &target).await?; + if let Some((entry, _)) = current.as_ref() { + validate_table_entry_object(&self.paths, &target, entry)?; + } + if current + .as_ref() + .is_none_or(|(entry, _)| entry.state == TableCatalogEntryState::Deleted) + { + let fence = TableCatalogObjectMutationFence::from_signals([ + permit.lock_lost_signal(), + progress.lock_lost_signal(), + entry_guard.lock_lost_signal(), + ]); + let etag = candidate_etag + .ok_or_else(|| TableCatalogStoreError::Unavailable("directory candidate has no deletion ETag".to_string()))?; + self.backend + .delete_object_if_match_fenced(self.catalog_bucket(), candidate, &etag, &fence) + .await?; + removed += 1; + } + } + state.prune_cursor = if page.is_truncated { + page.objects.last().cloned() + } else { + None + }; + self.write_table_directory_state( + &state, + directory_state_precondition(&state)?, + TableCatalogObjectMutationFence::from_signals([permit.lock_lost_signal(), progress.lock_lost_signal()]), + ) + .await?; + Ok(TableDirectoryStatus { + phase: Some(state.phase), + scanned_objects: page.objects.len(), + removed_candidates: removed, + }) + } + + pub(super) async fn directory_candidate_target( + &self, + table_bucket: &str, + candidate: &str, + ) -> TableCatalogStoreResult<(String, Option)> { + let prefix = self.paths.table_directory_prefix(table_bucket); + let suffix = candidate + .strip_prefix(&prefix) + .ok_or_else(|| TableCatalogStoreError::Invalid("table directory candidate is outside its prefix".to_string()))?; + // Namespace segments may themselves be named `tables`; parse the final + // table boundary and validate every identifier before trusting this key. + let invalid = || TableCatalogStoreError::Invalid("unexpected object in table-entry directory".to_string()); + let (namespace, name) = suffix + .strip_suffix(&format!("/{TABLE_ENTRY_FILE}")) + .and_then(|path| path.rsplit_once(&format!("/{TABLE_ROOT}/"))) + .ok_or_else(invalid)?; + let namespace = Namespace::from_segments(namespace.split('/').map(str::to_owned).collect()).map_err(|_| invalid())?; + let table = IdentifierSegment::parse(name.to_owned()).map_err(|_| invalid())?; + let target = self.paths.table_entry_path(table_bucket, &namespace, &table); + let current = self + .backend + .read_object_limited(self.catalog_bucket(), candidate, DIRECTORY_STATE_MAX_SIZE) + .await? + .ok_or_else(|| { + TableCatalogStoreError::Unavailable("table directory candidate disappeared during scan".to_string()) + })?; + if current.data.as_slice() != target.as_bytes() { + return Err(TableCatalogStoreError::Invalid( + "table directory target does not match its key".to_string(), + )); + } + Ok((target, current.etag)) + } + + pub(super) async fn table_directory_objects( + &self, + table_bucket: &str, + max_objects: Option, + ) -> TableCatalogStoreResult> { + let Some(state) = self.read_table_directory_state(table_bucket).await? else { + return Ok(None); + }; + if state.phase != TableDirectoryPhase::Ready { + return Ok(None); + } + let prefix = self.paths.table_directory_prefix(table_bucket); + let mut cursor = None; + let mut objects = Vec::new(); + loop { + let remaining = max_objects.map_or(TABLE_CATALOG_LIST_MAX_KEYS, |limit| limit.saturating_sub(objects.len())); + // A full authoritative page stays truncated until another read + // proves EOF. At the inclusive cap, probe one key without admitting + // it into the inventory; an empty complete page is still valid. + let limit = NonZeroUsize::new(remaining.min(TABLE_CATALOG_LIST_MAX_KEYS)).unwrap_or(NonZeroUsize::MIN); + let page = self + .backend + .list_catalog_objects_page(self.catalog_bucket(), &prefix, cursor.as_deref(), limit) + .await?; + let page = catalog_page_names(page)?; + validate_directory_page(&page, &prefix, cursor.as_deref(), limit.get())?; + if remaining == 0 && !page.objects.is_empty() { + return Err(TableCatalogStoreError::Unavailable( + "table-entry directory exceeds the data-plane scan safety limit".to_string(), + )); + } + for candidate in &page.objects { + let (target, _) = self.directory_candidate_target(table_bucket, candidate).await?; + objects.push(target); + } + if !page.is_truncated { + self.ensure_table_directory_epoch(table_bucket, state.activation_id).await?; + return Ok(Some(TableDirectoryScan { + objects, + activation_id: state.activation_id, + })); + } + cursor = page.objects.last().cloned(); + } + } + + pub(super) async fn ensure_table_directory_epoch( + &self, + table_bucket: &str, + activation_id: Uuid, + ) -> TableCatalogStoreResult<()> { + if self + .read_table_directory_state(table_bucket) + .await? + .is_none_or(|state| state.phase != TableDirectoryPhase::Ready || state.activation_id != activation_id) + { + return Err(TableCatalogStoreError::Unavailable( + "table directory activation changed during catalog inventory; retry the request".to_string(), + )); + } + Ok(()) + } +} + +fn catalog_page_names(page: TableCatalogCleanupPage) -> TableCatalogStoreResult { + if page + .objects + .iter() + .any(|object| object.delete_marker || object.version_id.as_deref().is_some_and(|id| !id.is_empty() && id != "null")) + { + return Err(TableCatalogStoreError::Invalid( + "table directory requires an unversioned authoritative catalog page".to_string(), + )); + } + Ok(TableCatalogObjectListPage { + objects: page.objects.into_iter().map(|object| object.key).collect(), + is_truncated: page.is_truncated, + }) +} + +fn directory_state_precondition(state: &TableDirectoryState) -> TableCatalogStoreResult { + state + .etag + .clone() + .map(TableCatalogPutPrecondition::IfMatch) + .ok_or_else(|| TableCatalogStoreError::Unavailable("directory state has no update ETag".to_string())) +} + +fn validate_directory_page( + page: &TableCatalogObjectListPage, + prefix: &str, + after: Option<&str>, + limit: usize, +) -> TableCatalogStoreResult<()> { + let mut previous = after; + if page.objects.len() > limit || (page.is_truncated && page.objects.is_empty()) { + return Err(TableCatalogStoreError::Unavailable( + "table directory listing returned an invalid page".to_string(), + )); + } + for object in &page.objects { + if !object.starts_with(prefix) || previous.is_some_and(|previous| object.as_str() <= previous) { + return Err(TableCatalogStoreError::Invalid( + "table directory listing did not advance within its prefix".to_string(), + )); + } + previous = Some(object); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::table_catalog::test_support::TestCatalogObjectBackend; + use crate::table_catalog::tests::{test_bucket_entry, test_namespace_entry, test_table_entry}; + + fn entry(bucket: &str, namespace: &Namespace, name: &str) -> TableEntry { + let mut entry = test_table_entry( + bucket, + namespace, + &IdentifierSegment::parse(name).unwrap(), + format!("tables/{name}/metadata/v1.json"), + ); + entry.table_id = name.to_string(); + entry.table_uuid = name.to_string(); + entry.warehouse_location = format!("s3://{bucket}/tables/{name}"); + entry + } + + async fn fixture() -> (TestCatalogObjectBackend, ObjectTableCatalogStore) { + let backend = TestCatalogObjectBackend::default(); + let store = ObjectTableCatalogStore::new(backend.clone()); + store.put_table_bucket(test_bucket_entry("warehouse")).await.unwrap(); + (backend, store) + } + + async fn finish(store: &ObjectTableCatalogStore) { + for _ in 0..100 { + let status = store + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + assert!(status.scanned_objects <= DIRECTORY_PAGE_SIZE); + if status.phase == Some(TableDirectoryPhase::Ready) { + return; + } + } + panic!("bounded backfill did not finish"); + } + + #[tokio::test] + async fn directory_index_keeps_legacy_authority_until_bounded_backfill_reaches_eof() { + let (backend, store) = fixture().await; + let namespace = Namespace::parse("sales").unwrap(); + let legacy = entry("warehouse", &namespace, "orders"); + let path = store + .paths + .table_entry_path("warehouse", &namespace, &IdentifierSegment::parse("orders").unwrap()); + // Simulate an old writer that never creates a directory candidate. + backend + .seed_object(RUSTFS_META_BUCKET, &path, serde_json::to_vec(&legacy).unwrap()) + .await; + assert_eq!( + store + .resolve_table_data_plane_resource("warehouse", "tables/orders/data/a") + .await + .unwrap() + .unwrap() + .table, + "orders" + ); + for (enabled, confirmed) in [(false, false), (true, false), (false, true)] { + assert!( + store + .advance_table_directory_with_confirmation("warehouse", enabled, confirmed) + .await + .is_err() + ); + assert!(store.read_table_directory_state("warehouse").await.unwrap().is_none()); + } + let started = store + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + assert_eq!(started.phase, Some(TableDirectoryPhase::Building)); + assert!( + store + .table_directory_objects("warehouse", Some(4096)) + .await + .unwrap() + .is_none() + ); + let fence = backend + .read_object(RUSTFS_META_BUCKET, &store.paths.backing_migration_fence_path("warehouse")) + .await + .unwrap() + .unwrap(); + assert!( + serde_json::from_slice::(&fence.data).is_err(), + "legacy writer parser must reject the directory capability fence" + ); + finish(&store).await; + assert!( + backend + .object_exists(RUSTFS_META_BUCKET, &store.paths.table_directory_candidate(&legacy).unwrap()) + .await + .unwrap() + ); + assert_eq!( + store + .resolve_table_data_plane_resource("warehouse", "tables/orders/data/a") + .await + .unwrap() + .unwrap() + .table, + "orders" + ); + // Restart with local activation settings off still honors persisted mode. + let restarted = ObjectTableCatalogStore::new(backend.clone()); + assert_eq!( + restarted + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .unwrap() + .phase, + Some(TableDirectoryPhase::Ready) + ); + restarted.disable_table_directory("warehouse").await.unwrap(); + restarted.disable_table_directory("warehouse").await.unwrap(); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &path).await.unwrap()); + assert!( + restarted + .table_directory_objects("warehouse", Some(4096)) + .await + .unwrap() + .is_none() + ); + assert_eq!( + restarted + .resolve_table_data_plane_resource("warehouse", "tables/orders/data/a") + .await + .unwrap() + .unwrap() + .table, + "orders" + ); + } + + #[tokio::test] + async fn directory_index_excludes_reports_but_preserves_colliding_nested_namespaces() { + let (backend, store) = fixture().await; + let parent = Namespace::parse("sales").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + for number in 0..4100 { + let report = store + .paths + .table_maintenance_job_path("warehouse", &parent, &table, "orders", &format!("job-{number}")); + backend.seed_object(RUSTFS_META_BUCKET, &report, b"{}".to_vec()).await; + } + let nested = Namespace::parse(&format!( + "sales.tables.orders.maintenance.{}.jobs.{}", + table_catalog_path_hash("orders"), + table_catalog_path_hash("job-0") + )) + .unwrap(); + let protected = entry("warehouse", &nested, "nested"); + let object = store + .paths + .table_entry_path("warehouse", &nested, &IdentifierSegment::parse("nested").unwrap()); + backend + .seed_object(RUSTFS_META_BUCKET, &object, serde_json::to_vec(&protected).unwrap()) + .await; + finish(&store).await; + backend.reset_call_counts().await; + let resource = store + .resolve_table_data_plane_resource("warehouse", "tables/nested/data/file") + .await + .unwrap() + .unwrap(); + assert_eq!(resource.namespace, nested.public_name()); + assert!( + store + .resolve_table_data_plane_resource("warehouse", "ordinary/file") + .await + .unwrap() + .is_none() + ); + assert!( + backend.list_call_count().await <= 3, + "report count must not add foreground directory pages" + ); + assert!( + backend.read_call_count().await < 100, + "report bodies must not be read by index-miss resolution" + ); + let candidates = backend + .list_objects(RUSTFS_META_BUCKET, &store.paths.table_directory_prefix("warehouse")) + .await + .unwrap(); + assert_eq!(candidates, vec![store.paths.table_directory_candidate(&protected).unwrap()]); + // Ready registration must not return to the mixed namespace inventory, + // even though its unlimited ownership check still validates every table. + backend + .state + .lock() + .await + .failed_inventory_prefixes + .insert(store.paths.namespace_entries_prefix("warehouse")); + let fresh = Namespace::parse("fresh").unwrap(); + store + .create_namespace(test_namespace_entry("warehouse", &fresh)) + .await + .unwrap(); + store.create_table(entry("warehouse", &fresh, "independent")).await.unwrap(); + let mut overlapping = entry("warehouse", &fresh, "collision"); + overlapping.warehouse_location = "s3://warehouse/tables/nested/child".to_string(); + assert!( + matches!(store.create_table(overlapping).await, Err(TableCatalogStoreError::Conflict(_))), + "directory inventory must retain real ownership conflicts" + ); + } + + #[tokio::test] + async fn directory_index_cancelled_page_retries_and_dual_write_precedes_entry_publication() { + let (backend, store) = fixture().await; + let namespace = Namespace::parse("sales").unwrap(); + store + .create_namespace(test_namespace_entry("warehouse", &namespace)) + .await + .unwrap(); + store + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + let state_path = store.paths.backing_migration_fence_path("warehouse"); + let pause = backend.pause_next_put(RUSTFS_META_BUCKET, &state_path).await; + let owner = store.clone(); + let task = tokio::spawn(async move { owner.advance_table_directory_with_confirmation("warehouse", true, true).await }); + tokio::time::timeout(StdDuration::from_secs(5), pause.wait_started()) + .await + .unwrap(); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!( + store.read_table_directory_state("warehouse").await.unwrap().unwrap().phase, + TableDirectoryPhase::Building + ); + pause.release(); + let table = entry("warehouse", &namespace, "new"); + let candidate = store.paths.table_directory_candidate(&table).unwrap(); + let path = store + .paths + .table_entry_path("warehouse", &namespace, &IdentifierSegment::parse("new").unwrap()); + backend.fail_next_put(RUSTFS_META_BUCKET, &candidate).await; + assert!(store.create_table(table.clone()).await.is_err()); + assert!( + !backend.object_exists(RUSTFS_META_BUCKET, &path).await.unwrap(), + "candidate write failure must prevent authoritative publication" + ); + store.create_table(table.clone()).await.unwrap(); + finish(&store).await; + assert_eq!( + store + .resolve_table_data_plane_resource("warehouse", "tables/new/data/a") + .await + .unwrap() + .unwrap() + .table, + "new" + ); + } + #[tokio::test] + async fn directory_index_pruning_waits_for_create_and_retains_rename_and_recreation() { + let (backend, store) = fixture().await; + let namespace = Namespace::parse("sales").unwrap(); + store + .create_namespace(test_namespace_entry("warehouse", &namespace)) + .await + .unwrap(); + finish(&store).await; + let table = entry("warehouse", &namespace, "orders"); + let path = store + .paths + .table_entry_path("warehouse", &namespace, &IdentifierSegment::parse("orders").unwrap()); + let candidate = store.paths.table_directory_candidate(&table).unwrap(); + let pause = backend.pause_next_put(RUSTFS_META_BUCKET, &path).await; + let owner = store.clone(); + let create = tokio::spawn(async move { owner.create_table(table).await }); + tokio::time::timeout(StdDuration::from_secs(5), pause.wait_started()) + .await + .unwrap(); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &candidate).await.unwrap()); + assert!(!backend.object_exists(RUSTFS_META_BUCKET, &path).await.unwrap()); + let reads = backend.read_lock_acquisition_count(RUSTFS_META_BUCKET, &path).await; + let owner = store.clone(); + let prune = tokio::spawn(async move { + owner + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + }); + tokio::time::timeout(StdDuration::from_secs(5), async { + while backend.read_lock_acquisition_count(RUSTFS_META_BUCKET, &path).await == reads { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + assert!(!prune.is_finished(), "GC must wait on the creator's authoritative entry lock"); + assert!(backend.object_exists(RUSTFS_META_BUCKET, &candidate).await.unwrap()); + pause.release(); + create.await.unwrap().unwrap(); + assert_eq!(prune.await.unwrap().unwrap().removed_candidates, 0); + store + .rename_table("warehouse", "sales", "orders", "sales", "renamed") + .await + .unwrap(); + let candidates = backend + .list_objects(RUSTFS_META_BUCKET, &store.paths.table_directory_prefix("warehouse")) + .await + .unwrap(); + assert_eq!(candidates.len(), 2, "rename must publish destination before retiring source"); + assert_eq!( + store + .resolve_table_data_plane_resource("warehouse", "tables/orders/data/a") + .await + .unwrap() + .unwrap() + .table, + "renamed" + ); + assert_eq!( + store + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .unwrap() + .removed_candidates, + 1 + ); + store.drop_table("warehouse", "sales", "renamed").await.unwrap(); + store + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .unwrap(); + let replacement = entry("warehouse", &namespace, "renamed"); + store.create_table(replacement.clone()).await.unwrap(); + assert!( + backend + .object_exists(RUSTFS_META_BUCKET, &store.paths.table_directory_candidate(&replacement).unwrap()) + .await + .unwrap() + ); + assert_eq!( + store + .resolve_table_data_plane_resource("warehouse", "tables/renamed/data/a") + .await + .unwrap() + .unwrap() + .table, + "renamed" + ); + } + #[tokio::test] + async fn directory_index_cancelled_initial_write_retains_guard_until_rollback_can_finish() { + let (backend, store) = fixture().await; + let fence_path = store.paths.backing_migration_fence_path("warehouse"); + let lock_path = store.paths.backing_migration_fence_lock_path("warehouse"); + let pause = backend.pause_next_put(RUSTFS_META_BUCKET, &fence_path).await; + let owner = store.clone(); + let request = tokio::spawn(async move { owner.advance_table_directory_with_confirmation("warehouse", true, true).await }); + tokio::time::timeout(StdDuration::from_secs(5), pause.wait_started()) + .await + .unwrap(); + request.abort(); + assert!(request.await.unwrap_err().is_cancelled()); + let locks = backend.write_lock_acquisition_count(RUSTFS_META_BUCKET, &lock_path).await; + let owner = store.clone(); + let rollback = tokio::spawn(async move { owner.disable_table_directory("warehouse").await }); + tokio::time::timeout(StdDuration::from_secs(5), async { + while backend.write_lock_acquisition_count(RUSTFS_META_BUCKET, &lock_path).await == locks { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + assert!( + !rollback.is_finished(), + "rollback must not observe Absent while an owned initial put can still arrive" + ); + pause.release(); + rollback.await.unwrap().unwrap(); + assert!(!backend.object_exists(RUSTFS_META_BUCKET, &fence_path).await.unwrap()); + assert!(store.read_table_directory_state("warehouse").await.unwrap().is_none()); + } + #[tokio::test] + async fn directory_index_lookup_reuses_existing_migration_and_publication_protection() { + let (backend, store) = fixture().await; + finish(&store).await; + let publication_path = default_table_bucket_publication_lock_path(); + let publication_write = backend.acquire_write_lock("warehouse", &publication_path).await.unwrap(); + assert!( + tokio::time::timeout(StdDuration::from_secs(5), store.resolve_table_data_plane_resource("warehouse", "outside")) + .await + .unwrap() + .unwrap() + .is_none(), + "an enclosing catalog publication must not self-deadlock" + ); + drop(publication_write); + let publication_read = backend.acquire_read_lock("warehouse", &publication_path).await.unwrap(); + let writes = backend.write_lock_acquisition_count("warehouse", &publication_path).await; + let writer_backend = backend.clone(); + let writer_path = publication_path.clone(); + let writer = tokio::spawn(async move { + drop(writer_backend.acquire_write_lock("warehouse", &writer_path).await.unwrap()); + }); + tokio::time::timeout(StdDuration::from_secs(5), async { + while backend.write_lock_acquisition_count("warehouse", &publication_path).await == writes { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + assert!( + tokio::time::timeout(StdDuration::from_secs(5), store.resolve_table_data_plane_resource("warehouse", "outside")) + .await + .unwrap() + .unwrap() + .is_none(), + "a queued writer must not turn an already-protected lookup into a recursive read wait" + ); + drop(publication_read); + writer.await.unwrap(); + + let state_path = store.paths.warehouse_index_state_path("warehouse"); + backend.delete_object(RUSTFS_META_BUCKET, &state_path).await.unwrap(); + let pause = crate::table_catalog::test_support::TestCatalogObjectPause::default(); + { + let mut state = backend.state.lock().await; + let key = (RUSTFS_META_BUCKET.to_string(), state_path); + // First lookup checks readiness; second read belongs to backfill, + // after it has taken its migration permit and warehouse-state lock. + let attempt = state.read_attempts.get(&key).copied().unwrap_or_default() + 2; + state + .pause_before_read_attempts + .entry(key) + .or_default() + .insert(attempt, pause.clone()); + } + let owner = store.clone(); + let mut lookup = tokio::spawn(async move { owner.resolve_table_data_plane_resource("warehouse", "outside").await }); + tokio::time::timeout(StdDuration::from_secs(5), pause.wait_started()) + .await + .unwrap(); + let lock_path = store.paths.backing_migration_fence_lock_path("warehouse"); + let writes = backend.write_lock_acquisition_count(RUSTFS_META_BUCKET, &lock_path).await; + let writer_backend = backend.clone(); + let writer_path = lock_path.clone(); + let writer = tokio::spawn(async move { + drop( + writer_backend + .acquire_write_lock(RUSTFS_META_BUCKET, &writer_path) + .await + .unwrap(), + ); + }); + tokio::time::timeout(StdDuration::from_secs(5), async { + while backend.write_lock_acquisition_count(RUSTFS_META_BUCKET, &lock_path).await == writes { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + pause.release(); + let result = tokio::time::timeout(StdDuration::from_secs(5), &mut lookup).await; + if result.is_err() { + lookup.abort(); + } + assert!( + result.unwrap().unwrap().unwrap().is_none(), + "backfill must reuse its existing migration permit" + ); + writer.await.unwrap(); + } + #[tokio::test(flavor = "current_thread")] + async fn directory_index_owned_advance_and_rollback_preserve_request_context() { + use crate::table_catalog::test_support::TestCatalogWorkerContext; + use rustfs_io_metrics::s3_http_metrics::{ + S3HttpRequestGuard, current_s3_http_operation_context, observe_s3_http_operation, with_s3_http_operation_context, + }; + use tracing::Instrument; + + let _subscriber = tracing::subscriber::set_default(tracing_subscriber::registry()); + let (backend, store) = fixture().await; + backend.state.lock().await.observed_worker_contexts = Some(Vec::new()); + let mut request = S3HttpRequestGuard::new("POST"); + let operation = request.in_scope(|| { + observe_s3_http_operation(rustfs_s3_ops::S3Operation::PutObject); + current_s3_http_operation_context().unwrap() + }); + let credentials = rustfs_credentials::Credentials { + access_key: "directory-request-user".to_string(), + ..Default::default() + }; + rustfs_keystone::KEYSTONE_CREDENTIALS + .scope( + Some(credentials), + with_s3_http_operation_context(Some(operation), async { + for step in 0..3 { + backend.state.lock().await.observed_worker_contexts.as_mut().unwrap().clear(); + let status = if step == 2 { + store.disable_table_directory("warehouse").await.unwrap() + } else { + store + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap() + }; + assert_eq!( + status.phase, + match step { + 0 => Some(TableDirectoryPhase::Building), + 1 => Some(TableDirectoryPhase::Ready), + _ => None, + } + ); + let state = backend.state.lock().await; + let observations = state.observed_worker_contexts.as_ref().unwrap(); + assert!(!observations.is_empty(), "each owned operation must reach actual backend reads"); + let expected = TestCatalogWorkerContext { + access_key: Some("directory-request-user".to_string()), + operation: Some("s3:PutObject"), + span_name: Some("table_directory_request"), + }; + assert!(observations.iter().all(|observation| observation == &expected), "{observations:?}"); + } + }) + .instrument(tracing::info_span!("table_directory_request")), + ) + .await; + backend.state.lock().await.observed_worker_contexts.as_mut().unwrap().clear(); + store + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + let state = backend.state.lock().await; + let observations = state.observed_worker_contexts.as_ref().unwrap(); + assert!(!observations.is_empty()); + let expected = TestCatalogWorkerContext { + access_key: None, + operation: None, + span_name: None, + }; + assert!(observations.iter().all(|observation| observation == &expected), "{observations:?}"); + } + + #[tokio::test] + async fn directory_index_accepts_inclusive_cap_only_after_authoritative_eof_probe() { + let (backend, store) = fixture().await; + let namespace = Namespace::parse("sales").unwrap(); + store + .create_namespace(test_namespace_entry("warehouse", &namespace)) + .await + .unwrap(); + for name in ["one", "two", "three", "four"] { + store.create_table(entry("warehouse", &namespace, name)).await.unwrap(); + } + finish(&store).await; + let prefix = store.paths.table_directory_prefix("warehouse"); + let full = backend + .list_catalog_objects_page(RUSTFS_META_BUCKET, &prefix, None, NonZeroUsize::new(4).unwrap()) + .await + .unwrap(); + assert_eq!(full.objects.len(), 4); + assert!(full.is_truncated, "the fixture must match production's full-page EOF semantics"); + let last = full.objects.last().unwrap().key.clone(); + let eof = backend + .list_catalog_objects_page(RUSTFS_META_BUCKET, &prefix, Some(&last), NonZeroUsize::MIN) + .await + .unwrap(); + assert!(eof.objects.is_empty()); + assert!(!eof.is_truncated); + let scan = store.table_directory_objects("warehouse", Some(4)).await.unwrap().unwrap(); + assert_eq!(scan.objects.len(), 4, "the configured candidate cap is inclusive"); + store.create_table(entry("warehouse", &namespace, "five")).await.unwrap(); + assert!( + matches!( + store.table_directory_objects("warehouse", Some(4)).await, + Err(TableCatalogStoreError::Unavailable(_)) + ), + "the EOF probe must reject an additional candidate, not expand the inventory cap" + ); + } + + #[tokio::test] + async fn directory_index_rejects_rollback_reactivation_during_inventory() { + let (backend, store) = fixture().await; + let namespace = Namespace::parse("sales").unwrap(); + store + .create_namespace(test_namespace_entry("warehouse", &namespace)) + .await + .unwrap(); + let table = entry("warehouse", &namespace, "orders"); + store.create_table(table.clone()).await.unwrap(); + finish(&store).await; + // A cold warehouse-index backfill owns the migration read lock. The + // race under test is an ordinary lookup observing the directory epoch. + store.backfill_table_warehouse_index("warehouse").await.unwrap(); + assert!(store.warehouse_index_ready("warehouse").await.unwrap()); + let old_epoch = store + .read_table_directory_state("warehouse") + .await + .unwrap() + .unwrap() + .activation_id; + let candidate = store.paths.table_directory_candidate(&table).unwrap(); + let pause = backend.pause_next_read(RUSTFS_META_BUCKET, &candidate).await; + let owner = store.clone(); + let lookup = tokio::spawn(async move { owner.resolve_table_data_plane_resource("warehouse", "ordinary/file").await }); + tokio::time::timeout(StdDuration::from_secs(5), pause.wait_started()) + .await + .unwrap(); + tokio::time::timeout(StdDuration::from_secs(5), store.disable_table_directory("warehouse")) + .await + .expect("an ordinary lookup must not prevent rollback") + .unwrap(); + tokio::time::timeout(StdDuration::from_secs(5), finish(&store)) + .await + .expect("reactivation must reach Ready while the old lookup is paused"); + let new_epoch = store + .read_table_directory_state("warehouse") + .await + .unwrap() + .unwrap() + .activation_id; + assert_ne!(old_epoch, new_epoch); + pause.release(); + assert!( + matches!( + tokio::time::timeout(StdDuration::from_secs(5), lookup) + .await + .unwrap() + .unwrap(), + Err(TableCatalogStoreError::Unavailable(_)) + ), + "a stale Ready observation cannot survive rollback and reactivation" + ); + assert!( + store + .resolve_table_data_plane_resource("warehouse", "tables/orders/file") + .await + .unwrap() + .is_some() + ); + } + + #[tokio::test] + async fn directory_index_real_backend_requires_authoritative_eof_before_ready() { + use crate::storage_api::table::test_support::{BucketOperations as _, MakeBucketOptions}; + let (_temp, paths, storage) = crate::app::gating_test_env::isolated_multi_pool_ecstore().await; + storage.make_bucket("warehouse", &MakeBucketOptions::default()).await.unwrap(); + let backend = EcStoreTableCatalogObjectBackend::new_with_strong_runtime(storage, StrongTableCatalogRuntime::default()); + let catalog = ObjectTableCatalogStore::new(backend.clone()); + catalog.put_table_bucket(test_bucket_entry("warehouse")).await.unwrap(); + let prefix = catalog.paths.namespace_entries_prefix("warehouse"); + for index in 0..130 { + backend + .put_object( + RUSTFS_META_BUCKET, + &format!("{prefix}sales/tables/orders/maintenance/jobs/{index:04}.json"), + b"{}".to_vec(), + TableCatalogPutPrecondition::IfAbsent, + ) + .await + .unwrap(); + } + catalog + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + let first = catalog + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .unwrap(); + assert_eq!(first.scanned_objects, DIRECTORY_PAGE_SIZE); + assert_eq!( + first.phase, + Some(TableDirectoryPhase::Building), + "a successful bounded cut does not establish EOF" + ); + let last = catalog + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .unwrap(); + assert_eq!(last.scanned_objects, 2); + assert_eq!(last.phase, Some(TableDirectoryPhase::Ready)); + catalog.disable_table_directory("warehouse").await.unwrap(); + let bad_key = format!("{prefix}000bad.json"); + backend + .put_object(RUSTFS_META_BUCKET, &bad_key, b"{}".to_vec(), TableCatalogPutPrecondition::IfAbsent) + .await + .unwrap(); + // Poison metadata on both pools so another healthy producer cannot + // turn an unresolved early key into a successful partial inventory. + for disk in paths.iter().flatten() { + let object = disk.join(RUSTFS_META_BUCKET).join(&bad_key); + tokio::fs::create_dir_all(&object).await.unwrap(); + tokio::fs::write(object.join("xl.meta"), b"invalid xl.meta").await.unwrap(); + } + catalog + .advance_table_directory_with_confirmation("warehouse", true, true) + .await + .unwrap(); + assert!( + catalog + .advance_table_directory_with_confirmation("warehouse", false, false) + .await + .is_err() + ); + assert_eq!( + catalog.read_table_directory_state("warehouse").await.unwrap().unwrap().phase, + TableDirectoryPhase::Building + ); + } +} diff --git a/rustfs/src/table_catalog/store/migration.rs b/rustfs/src/table_catalog/store/migration.rs index e61f15a9a..b0925ec74 100644 --- a/rustfs/src/table_catalog/store/migration.rs +++ b/rustfs/src/table_catalog/store/migration.rs @@ -23,6 +23,20 @@ use super::strong::{ }; use super::*; +async fn run_owned_backing_migration( + operation: impl std::future::Future> + Send + 'static, +) -> TableCatalogStoreResult { + let credentials = rustfs_keystone::KEYSTONE_CREDENTIALS.try_with(Clone::clone).ok().flatten(); + let operation = rustfs_keystone::KEYSTONE_CREDENTIALS.scope(credentials, operation); + let operation = rustfs_io_metrics::s3_http_metrics::with_s3_http_operation_context( + rustfs_io_metrics::s3_http_metrics::current_s3_http_operation_context(), + operation, + ); + crate::storage_api::table::spawn_traced_join(operation) + .await + .map_err(|_| TableCatalogStoreError::Internal("catalog migration worker failed".to_owned()))? +} + pub(super) fn table_catalog_backing_manifest( paths: &TableCatalogObjectPaths, namespace: &Namespace, @@ -267,6 +281,13 @@ where &self, table_bucket: &str, ) -> TableCatalogStoreResult)>> { + // The table-entry directory keeps its state on this path; its presence + // excludes a backing migration rather than being a malformed fence. + if self.read_table_directory_state(table_bucket).await?.is_some() { + return Err(TableCatalogStoreError::Conflict( + "table bucket has an active table-entry directory; roll it back before a backing migration".to_string(), + )); + } let fence = self .read_entry(self.catalog_bucket(), &self.paths.backing_migration_fence_path(table_bucket)) .await?; @@ -307,6 +328,11 @@ where &self, table_bucket: &str, ) -> TableCatalogStoreResult<()> { + // The directory upgrade keeps legacy entries authoritative and requires + // every compatible writer to publish its candidate before the entry. + if self.read_table_directory_state(table_bucket).await?.is_some() { + return Ok(()); + } if self.read_backing_migration_fence(table_bucket).await?.is_some() { return Err(TableCatalogStoreError::Conflict(format!( "object-backed catalog writes are fenced while table bucket {table_bucket} is prepared for durable strong cutover" @@ -318,6 +344,7 @@ where async fn ensure_global_backing_migration_fence( &self, fence_path: &str, + mutation_fence: &TableCatalogObjectMutationFence, ) -> TableCatalogStoreResult { if let Some((fence, _)) = self .read_entry::(self.catalog_bucket(), fence_path) @@ -330,13 +357,17 @@ where version: TABLE_CATALOG_MIGRATION_VERSION, migration_id: Uuid::new_v4().to_string(), }; - self.write_entry(self.catalog_bucket(), fence_path, &fence, TableCatalogPutPrecondition::IfAbsent) + self.write_migration_entry(fence_path, &fence, TableCatalogPutPrecondition::IfAbsent, mutation_fence) .await?; Ok(fence) } - async fn clear_global_backing_migration_fence_if_unused(&self, fence_path: &str) -> TableCatalogStoreResult<()> { - let Some((fence, _)) = self + async fn clear_global_backing_migration_fence_if_unused( + &self, + fence_path: &str, + mutation_fence: &TableCatalogObjectMutationFence, + ) -> TableCatalogStoreResult<()> { + let Some((fence, etag)) = self .read_entry::(self.catalog_bucket(), fence_path) .await? else { @@ -353,7 +384,24 @@ where { return Ok(()); } - self.backend.delete_object(self.catalog_bucket(), fence_path).await + let etag = etag.ok_or_else(|| TableCatalogStoreError::Conflict("migration fence has no ETag".to_owned()))?; + self.backend + .delete_object_if_match_fenced(self.catalog_bucket(), fence_path, &etag, mutation_fence) + .await + } + + async fn write_migration_entry( + &self, + path: &str, + entry: &T, + precondition: TableCatalogPutPrecondition, + fence: &TableCatalogObjectMutationFence, + ) -> TableCatalogStoreResult<()> { + let data = serde_json::to_vec(entry) + .map_err(|err| TableCatalogStoreError::Internal(format!("cannot serialize migration entry: {err}")))?; + self.backend + .put_object_fenced(self.catalog_bucket(), path, data, precondition, fence) + .await } async fn collect_bucket_snapshot_with_locks( @@ -907,6 +955,16 @@ where pub(crate) async fn materialize_durable_strong_backing_migration( &self, table_bucket: &str, + ) -> TableCatalogStoreResult { + let store = self.clone(); + let table_bucket = table_bucket.to_owned(); + run_owned_backing_migration(async move { store.materialize_durable_strong_backing_migration_owned(&table_bucket).await }) + .await + } + + async fn materialize_durable_strong_backing_migration_owned( + &self, + table_bucket: &str, ) -> TableCatalogStoreResult { let fence_path = self.paths.backing_migration_fence_path(table_bucket); let fence_lock_path = self.paths.backing_migration_fence_lock_path(table_bucket); @@ -929,6 +987,10 @@ where .backend .acquire_write_lock(self.catalog_bucket(), &fence_lock_path) .await?; + let mut mutation_fence = TableCatalogObjectMutationFence::from_signals([ + _global_fence_guard.lock_lost_signal(), + _fence_guard.lock_lost_signal(), + ]); let existing_fence = self.read_backing_migration_fence(table_bucket).await?; let strong_store = StrongTableCatalogStore::new(self.backend.clone()); if let Some((fence, _)) = existing_fence.as_ref() @@ -955,6 +1017,10 @@ where let source = self .collect_bucket_snapshot_with_locks(table_bucket, &mut source_guards) .await?; + mutation_fence + .signals + .extend(source_guards.iter().filter_map(TableCatalogLockGuard::lock_lost_signal)); + mutation_fence.ensure_held()?; let source_fingerprint = table_catalog_bucket_snapshot_fingerprint(&source)?; if let Some((fence, _)) = existing_fence.as_ref() && fence.status == TableCatalogBackingMigrationFenceStatus::Materialized @@ -965,7 +1031,8 @@ where ))); } - self.ensure_global_backing_migration_fence(&global_fence_path).await?; + self.ensure_global_backing_migration_fence(&global_fence_path, &mutation_fence) + .await?; let (target_fingerprint, target_snapshot_etag) = Self::observe_durable_strong_migration_target( &strong_store, table_bucket, @@ -985,12 +1052,14 @@ where source_fingerprint: None, target_snapshot_etag, }; - self.write_entry(self.catalog_bucket(), &fence_path, &fence, TableCatalogPutPrecondition::IfAbsent) + self.write_migration_entry(&fence_path, &fence, TableCatalogPutPrecondition::IfAbsent, &mutation_fence) .await?; (fence.migration_id, target_bucket_existed) }; - let (target_snapshot_etag, created) = strong_store.materialize_bucket_snapshot(source.clone()).await?; + let (target_snapshot_etag, created) = strong_store + .materialize_bucket_snapshot_with_fence(source.clone(), &mutation_fence) + .await?; let completed_fence = TableCatalogBackingMigrationFence { version: TABLE_CATALOG_MIGRATION_VERSION, table_bucket: table_bucket.to_string(), @@ -1000,7 +1069,7 @@ where source_fingerprint: Some(source_fingerprint.clone()), target_snapshot_etag: Some(target_snapshot_etag.clone()), }; - self.write_entry(self.catalog_bucket(), &fence_path, &completed_fence, TableCatalogPutPrecondition::Any) + self.write_migration_entry(&fence_path, &completed_fence, TableCatalogPutPrecondition::Any, &mutation_fence) .await?; drop(source_guards); @@ -1030,6 +1099,15 @@ where pub(crate) async fn cancel_durable_strong_backing_migration( &self, table_bucket: &str, + ) -> TableCatalogStoreResult { + let store = self.clone(); + let table_bucket = table_bucket.to_owned(); + run_owned_backing_migration(async move { store.cancel_durable_strong_backing_migration_owned(&table_bucket).await }).await + } + + async fn cancel_durable_strong_backing_migration_owned( + &self, + table_bucket: &str, ) -> TableCatalogStoreResult { let fence_path = self.paths.backing_migration_fence_path(table_bucket); let fence_lock_path = self.paths.backing_migration_fence_lock_path(table_bucket); @@ -1043,8 +1121,12 @@ where .backend .acquire_write_lock(self.catalog_bucket(), &fence_lock_path) .await?; - let Some((fence, _)) = self.read_backing_migration_fence(table_bucket).await? else { - self.clear_global_backing_migration_fence_if_unused(&global_fence_path) + let mut mutation_fence = TableCatalogObjectMutationFence::from_signals([ + _global_fence_guard.lock_lost_signal(), + _fence_guard.lock_lost_signal(), + ]); + let Some((fence, fence_etag)) = self.read_backing_migration_fence(table_bucket).await? else { + self.clear_global_backing_migration_fence_if_unused(&global_fence_path, &mutation_fence) .await?; return Ok(TableCatalogBackingMigrationCancelReport { table_bucket: table_bucket.to_string(), @@ -1052,12 +1134,17 @@ where object_backed_writes_fenced: false, }); }; - self.ensure_global_backing_migration_fence(&global_fence_path).await?; + self.ensure_global_backing_migration_fence(&global_fence_path, &mutation_fence) + .await?; let mut source_guards = Vec::new(); let source = self .collect_bucket_snapshot_with_locks(table_bucket, &mut source_guards) .await?; + mutation_fence + .signals + .extend(source_guards.iter().filter_map(TableCatalogLockGuard::lock_lost_signal)); + mutation_fence.ensure_held()?; let source_fingerprint = table_catalog_bucket_snapshot_fingerprint(&source)?; if fence.status == TableCatalogBackingMigrationFenceStatus::Materialized && fence.source_fingerprint.as_deref() != Some(source_fingerprint.as_str()) @@ -1086,11 +1173,14 @@ where } if !fence.target_bucket_existed { strong_store - .remove_bucket_snapshot_if_unchanged(table_bucket, &source_fingerprint) + .remove_bucket_snapshot_if_unchanged_with_fence(table_bucket, &source_fingerprint, &mutation_fence) .await?; } - self.backend.delete_object(self.catalog_bucket(), &fence_path).await?; - self.clear_global_backing_migration_fence_if_unused(&global_fence_path) + let fence_etag = fence_etag.ok_or_else(|| TableCatalogStoreError::Conflict("migration fence has no ETag".to_owned()))?; + self.backend + .delete_object_if_match_fenced(self.catalog_bucket(), &fence_path, &fence_etag, &mutation_fence) + .await?; + self.clear_global_backing_migration_fence_if_unused(&global_fence_path, &mutation_fence) .await?; Ok(TableCatalogBackingMigrationCancelReport { table_bucket: table_bucket.to_string(), diff --git a/rustfs/src/table_catalog/store/mod.rs b/rustfs/src/table_catalog/store/mod.rs index c755c04bb..4217489fc 100644 --- a/rustfs/src/table_catalog/store/mod.rs +++ b/rustfs/src/table_catalog/store/mod.rs @@ -18,9 +18,15 @@ use crate::storage_api::table::{WriteCommitGuard, WriteCompletion}; mod backup; mod cleanup; +mod directory_index; mod migration; mod object; +mod publication_deadline; mod registration_cache; +pub(crate) use publication_deadline::TablePublicationPreparation; +use publication_deadline::{ + begin_publication_mutation, publication_preparation, publication_recovery, retain_publication_preparation_fence, +}; #[cfg(test)] mod registration_cache_tests; pub(crate) use registration_cache::TableCatalogRegistrationCache; @@ -464,6 +470,10 @@ pub(crate) trait TableCatalogStore: Send + Sync { #[async_trait::async_trait] pub(crate) trait TableCommitPublication: Send + Sync { + fn preparation_budget(&self) -> Arc { + Arc::new(TablePublicationPreparation::default()) + } + async fn begin_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()>; async fn prepare(&self, table_bucket: &str, namespace: &str, table: &str) -> TableCatalogStoreResult<()>; @@ -507,6 +517,7 @@ impl Drop for TableCommitPublicationCompletion<'_> { struct TableCommitLockPublication<'a, B> { backend: &'a B, + preparation: Arc, state: parking_lot::Mutex, } @@ -522,6 +533,7 @@ impl<'a, B> TableCommitLockPublication<'a, B> { fn new(backend: &'a B) -> Self { Self { backend, + preparation: Arc::new(TablePublicationPreparation::default()), state: parking_lot::Mutex::new(TableCommitLockPublicationState::default()), } } @@ -571,7 +583,13 @@ impl<'a, B> TableCommitPublication for TableCommitLockPublication<'a, B> where B: TableCatalogObjectBackend, { + fn preparation_budget(&self) -> Arc { + self.preparation.clone() + } + async fn begin_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { + // Includes an earlier acquisition even when a caller prepares before entering the store. + let _ = self.preparation.deadline(); { let mut state = self.state.lock(); if state @@ -677,12 +695,15 @@ where #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct TableCatalogObject { pub data: Vec, + /// Server-generated identity of the body returned by this read. + pub body_revision: Option, pub etag: Option, pub mod_time: Option, } #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct TableCatalogObjectMetadata { + pub body_revision: Option, pub etag: Option, pub mod_time: Option, pub size: u64, @@ -788,6 +809,11 @@ pub(crate) struct TableCatalogCleanupObject { pub delete_marker: bool, } +pub(crate) struct TableCatalogCleanupPage { + pub objects: Vec, + pub is_truncated: bool, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum TableCatalogPutPrecondition { Any, @@ -872,6 +898,7 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { .read_object(bucket, object) .await? .map(|object| TableCatalogObjectMetadata { + body_revision: object.body_revision, etag: object.etag, mod_time: object.mod_time, size: u64::try_from(object.data.len()).unwrap_or(u64::MAX), @@ -887,6 +914,7 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { .read_object_unlocked(bucket, object) .await? .map(|object| TableCatalogObjectMetadata { + body_revision: object.body_revision, etag: object.etag, mod_time: object.mod_time, size: u64::try_from(object.data.len()).unwrap_or(u64::MAX), @@ -911,6 +939,21 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { precondition: TableCatalogPutPrecondition, ) -> TableCatalogStoreResult<()>; + /// Write an object that precedes the catalog transaction, such as the + /// initial metadata file of a create. It is authorized and fenced like + /// `put_object`, but a publication-aware backend must not treat it as the + /// first durable catalog mutation: an orphan is reclaimed by maintenance, + /// so the preparation deadline keeps bounding the work that follows. + async fn put_object_before_publication( + &self, + bucket: &str, + object: &str, + data: Vec, + precondition: TableCatalogPutPrecondition, + ) -> TableCatalogStoreResult<()> { + self.put_object(bucket, object, data, precondition).await + } + async fn put_object_unlocked( &self, bucket: &str, @@ -1010,6 +1053,20 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { )) } + /// GET-equivalent, bounded enumeration for publishing authoritative indexes. + /// Backends without this contract must not fall back to an ordinary S3 list. + async fn list_catalog_objects_page( + &self, + _bucket: &str, + _prefix: &str, + _start_after: Option<&str>, + _limit: NonZeroUsize, + ) -> TableCatalogStoreResult { + Err(TableCatalogStoreError::Unsupported( + "authoritative catalog pagination is not supported by this backend".to_string(), + )) + } + async fn list_objects_page( &self, bucket: &str, @@ -1042,6 +1099,10 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { Ok(()) } + fn table_publication_preparation_budget(&self) -> Arc { + Arc::new(TablePublicationPreparation::default()) + } + async fn begin_table_bucket_commit_publication(&self, _table_bucket: &str) -> TableCatalogStoreResult<()> { Ok(()) } @@ -1178,6 +1239,10 @@ impl TableCommitPublication for B where B: TableCatalogObjectBackend, { + fn preparation_budget(&self) -> Arc { + self.table_publication_preparation_budget() + } + async fn begin_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { self.begin_table_bucket_commit_publication(table_bucket).await } @@ -2392,6 +2457,7 @@ where async fn object_metadata(&self, bucket: &str, object: &str) -> TableCatalogStoreResult> { match self.store.get_object_info(bucket, object, &ObjectOptions::default()).await { Ok(info) => Ok(Some(TableCatalogObjectMetadata { + body_revision: info.data_dir.filter(|id| !id.is_nil()), etag: info.etag, mod_time: info.mod_time, size: u64::try_from(info.size.max(0)).unwrap_or(u64::MAX), @@ -2419,6 +2485,7 @@ where .await { Ok(info) => Ok(Some(TableCatalogObjectMetadata { + body_revision: info.data_dir.filter(|id| !id.is_nil()), etag: info.etag, mod_time: info.mod_time, size: u64::try_from(info.size.max(0)).unwrap_or(u64::MAX), @@ -2644,6 +2711,36 @@ where }) } + async fn list_catalog_objects_page( + &self, + bucket: &str, + prefix: &str, + start_after: Option<&str>, + limit: NonZeroUsize, + ) -> TableCatalogStoreResult { + let max_keys = i32::try_from(limit.get()) + .map_err(|_| TableCatalogStoreError::Invalid("catalog page size exceeds storage API range".to_string()))?; + let page = self + .store + .clone() + .list_unversioned_objects_authoritative(bucket, prefix, start_after.map(str::to_string), max_keys) + .await + .map_err(|err| storage_error_to_catalog("list authoritative catalog page", err))?; + Ok(TableCatalogCleanupPage { + objects: page + .objects + .into_iter() + .map(|object| TableCatalogCleanupObject { + key: object.name, + version_id: object.version_id.filter(|id| !id.is_nil()).map(|id| id.to_string()), + etag: object.etag, + delete_marker: object.delete_marker, + }) + .collect(), + is_truncated: page.is_truncated, + }) + } + async fn list_objects_for_cleanup( &self, bucket: &str, @@ -2783,6 +2880,7 @@ where ))); } } + let body_revision = reader.object_info.data_dir.filter(|id| !id.is_nil()); let etag = reader.object_info.etag.clone(); let mod_time = reader.object_info.mod_time; let mut data = Vec::new(); @@ -2801,7 +2899,12 @@ where TableCatalogStoreError::Internal(format!("failed to read catalog object {bucket}/{object}: {err}")) })?; } - Ok(Some(TableCatalogObject { data, etag, mod_time })) + Ok(Some(TableCatalogObject { + data, + body_revision, + etag, + mod_time, + })) } async fn put_object_with_options( @@ -2926,33 +3029,52 @@ mod tests { )))), acquired: Mutex::new(None), }); - let lock = rustfs_lock::NamespaceLockWrapper::new( - rustfs_lock::NamespaceLock::with_clients_and_quorum( - "maintenance-probe".to_string(), - vec![client.clone() as Arc], - 1, - ), - rustfs_lock::ObjectKey::new( - RUSTFS_META_BUCKET, - TableCatalogObjectPaths::default().backing_migration_global_fence_lock_path(), - ), - "maintenance-migration-probe".to_string(), + let lock = rustfs_lock::NamespaceLock::with_clients_and_quorum( + "maintenance-probe".to_string(), + vec![client.clone() as Arc], + 1, ); - let guard = TableCatalogLockGuard::namespace(lock.get_read_lock(Duration::from_secs(5)).await.unwrap()); - let publication = TableCommitLockPublication::new(&backend); - publication.attach_catalog_migration_read_guard(guard).unwrap(); - publication.begin_table_bucket(bucket).await.unwrap(); - publication.prepare(bucket, "sales", "orders").await.unwrap(); - assert!(publication.holds_table(bucket, "sales", "orders")); + let resource = rustfs_lock::ObjectKey::new( + RUSTFS_META_BUCKET, + TableCatalogObjectPaths::default().backing_migration_global_fence_lock_path(), + ); + // Observe real lease loss before the separate publication preparation + // deadline. Keep the lease TTL unchanged; only speed up this probe. + let request = + rustfs_lock::LockRequest::new(resource.clone(), rustfs_lock::LockType::Shared, "maintenance-migration-probe") + .with_acquire_timeout(Duration::from_secs(5)) + .with_ttl(Duration::from_secs(30)) + .with_refresh_interval(Duration::from_millis(20)); + let guard = TableCatalogLockGuard::namespace(lock.acquire_guard(&request).await.unwrap().unwrap()); let id = client .acquired .lock() .unwrap() .clone() - .expect("actual migration lease acquired"); + .expect("actual publication migration lease acquired"); + // Completion clears the publication's guards. Keep another real lease + // to test rejection of an already-lost guard after that cleanup. + let attach_request = rustfs_lock::LockRequest::new(resource, rustfs_lock::LockType::Shared, "maintenance-attach-probe") + .with_acquire_timeout(Duration::from_secs(5)) + .with_ttl(Duration::from_secs(30)) + .with_refresh_interval(Duration::from_millis(20)); + let lost = TableCatalogLockGuard::namespace(lock.acquire_guard(&attach_request).await.unwrap().unwrap()); + let attach_id = client + .acquired + .lock() + .unwrap() + .clone() + .expect("actual attachment migration lease acquired"); + assert_ne!(id, attach_id, "the attachment probe must use its own lease"); + let publication = TableCommitLockPublication::new(&backend); + publication.attach_catalog_migration_read_guard(guard).unwrap(); + publication.begin_table_bucket(bucket).await.unwrap(); + publication.prepare(bucket, "sales", "orders").await.unwrap(); + assert!(publication.holds_table(bucket, "sales", "orders")); assert!(client.force_release(&id).await.unwrap()); - tokio::time::timeout(Duration::from_secs(15), async { - while publication.catalog_migration_read_permit_status() != Some(false) { + assert!(client.force_release(&attach_id).await.unwrap()); + tokio::time::timeout(Duration::from_secs(2), async { + while publication.catalog_migration_read_permit_status() != Some(false) || !lost.is_lock_lost() { tokio::time::sleep(Duration::from_millis(10)).await; } }) diff --git a/rustfs/src/table_catalog/store/object.rs b/rustfs/src/table_catalog/store/object.rs index 6a9e3a12b..8c65a7ab2 100644 --- a/rustfs/src/table_catalog/store/object.rs +++ b/rustfs/src/table_catalog/store/object.rs @@ -415,15 +415,19 @@ pub(in crate::table_catalog) fn bounded_table_entry_objects_for_data_plane_scan( is_truncated: bool, max_catalog_objects: usize, ) -> TableCatalogStoreResult> { - if is_truncated || objects.len() > max_catalog_objects { + // Only table entries count: maintenance reports, bridges and other + // per-table objects share the prefix and are controlled by table-scoped + // principals, so they must not be able to trip the data-plane limit. + let entries: Vec = objects + .into_iter() + .filter(|object| object.ends_with(TABLE_ENTRY_FILE)) + .collect(); + if is_truncated || entries.len() > max_catalog_objects { return Err(TableCatalogStoreError::Unavailable(format!( "table data-plane warehouse-index miss scan exceeds the {max_catalog_objects}-catalog-object safety limit" ))); } - Ok(objects - .into_iter() - .filter(|object| object.ends_with(TABLE_ENTRY_FILE)) - .collect()) + Ok(entries) } #[derive(Clone)] @@ -1102,6 +1106,8 @@ where )); } } + fence.ensure_held()?; + begin_publication_mutation()?; for (suffix, value) in current { self.backend .delete_object_if_match_fenced(self.catalog_bucket(), &format!("{prefix}{suffix}"), &value.etag, fence) @@ -1191,6 +1197,7 @@ where { let data = serde_json::to_vec(entry) .map_err(|err| TableCatalogStoreError::Internal(format!("failed to serialize catalog entry {object}: {err}")))?; + begin_publication_mutation()?; self.backend.put_object(bucket, object, data, precondition).await } @@ -1251,6 +1258,11 @@ where { let data = serde_json::to_vec(entry) .map_err(|err| TableCatalogStoreError::Internal(format!("failed to serialize catalog entry {object}: {err}")))?; + if let Some(fence) = fence { + fence.ensure_held()?; + } + begin_publication_mutation()?; + self.prepare_table_directory_candidate(bucket, object, &data).await?; match fence { Some(fence) => { self.backend @@ -1427,6 +1439,14 @@ where &self, table_bucket: &str, publication: &(dyn TableCommitPublication + Sync), + ) -> TableCatalogStoreResult<()> { + publication_recovery(self.recover_active_table_rename_inner(table_bucket, publication)).await + } + + async fn recover_active_table_rename_inner( + &self, + table_bucket: &str, + publication: &(dyn TableCommitPublication + Sync), ) -> TableCatalogStoreResult<()> { if !publication.holds_table_bucket(table_bucket) { return Err(TableCatalogStoreError::Internal( @@ -1875,6 +1895,7 @@ where index: &TableWarehouseIndexEntry, reason: &'static str, ) -> TableCatalogStoreResult<()> { + begin_publication_mutation()?; self.backend.delete_object_unlocked(self.catalog_bucket(), object).await?; tracing::warn!( table_bucket = %index.table_bucket, @@ -2558,7 +2579,7 @@ where .await } - async fn require_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { + pub(super) async fn require_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { if self .get_table_bucket(table_bucket) .await? @@ -2609,15 +2630,17 @@ where max_catalog_objects: usize, ) -> TableCatalogStoreResult> { let mut objects = Vec::new(); + let mut entries = 0_usize; + let mut pages = 0_usize; let mut cursor = None; + let limit = NonZeroUsize::new(TABLE_CATALOG_LIST_MAX_KEYS) + .ok_or_else(|| TableCatalogStoreError::Internal("table catalog listing page size must be positive".to_string()))?; loop { - let remaining = max_catalog_objects.saturating_sub(objects.len()); - let page_size = remaining.min(TABLE_CATALOG_LIST_MAX_KEYS); - let limit = NonZeroUsize::new(page_size).ok_or_else(|| { - TableCatalogStoreError::Unavailable(format!( - "table data-plane warehouse-index miss scan exceeds the {max_catalog_objects}-catalog-object safety limit" - )) - })?; + if pages >= TABLE_DATA_PLANE_INDEX_MISS_SCAN_MAX_PAGES { + return Err(TableCatalogStoreError::Unavailable(format!( + "table data-plane warehouse-index miss scan exceeds the {TABLE_DATA_PLANE_INDEX_MISS_SCAN_MAX_PAGES}-page listing budget" + ))); + } let page = self .backend .list_objects_page( @@ -2627,12 +2650,18 @@ where limit, ) .await?; + pages += 1; let next_cursor = page.objects.last().cloned(); + entries += page + .objects + .iter() + .filter(|object| object.ends_with(TABLE_ENTRY_FILE)) + .count(); objects.extend(page.objects); if !page.is_truncated { return bounded_table_entry_objects_for_data_plane_scan(objects, false, max_catalog_objects); } - if objects.len() >= max_catalog_objects { + if entries > max_catalog_objects { return bounded_table_entry_objects_for_data_plane_scan(objects, true, max_catalog_objects); } if next_cursor.is_none() || next_cursor == cursor { @@ -2651,18 +2680,22 @@ where locked_table_entry: Option<&str>, read_concurrency: usize, ) -> TableCatalogStoreResult> { - let table_objects = match max_catalog_objects { - Some(max_catalog_objects) => { - self.list_table_entry_objects_for_data_plane_scan(table_bucket, max_catalog_objects) - .await? + let directory = self.table_directory_objects(table_bucket, max_catalog_objects).await?; + let (table_objects, directory_epoch) = match directory { + Some(scan) => (scan.objects, Some(scan.activation_id)), + None => { + let objects = match max_catalog_objects { + Some(limit) => self.list_table_entry_objects_for_data_plane_scan(table_bucket, limit).await?, + None => self + .backend + .list_objects(self.catalog_bucket(), &self.paths.namespace_entries_prefix(table_bucket)) + .await? + .into_iter() + .filter(|object| object.ends_with(TABLE_ENTRY_FILE)) + .collect(), + }; + (objects, None) } - None => self - .backend - .list_objects(self.catalog_bucket(), &self.paths.namespace_entries_prefix(table_bucket)) - .await? - .into_iter() - .filter(|object| object.ends_with(TABLE_ENTRY_FILE)) - .collect(), }; let mut entries = stream::iter(table_objects) .map(|object| async move { @@ -2682,6 +2715,9 @@ where .try_filter_map(|entry| async move { Ok(entry) }) .try_collect::>() .await?; + if let Some(epoch) = directory_epoch { + self.ensure_table_directory_epoch(table_bucket, epoch).await?; + } entries.sort_by(|left, right| (&left.namespace, &left.table).cmp(&(&right.namespace, &right.table))); Ok(entries) } @@ -2779,95 +2815,100 @@ where precondition: TableCatalogPutPrecondition, publication: &(dyn TableCommitPublication + Sync), ) -> TableCatalogStoreResult<()> { - validate_table_entry_version_and_id(&entry)?; - let namespace = parse_namespace_for_store(&entry.namespace)?; - let table = parse_table_for_store(&entry.table)?; - validate_table_warehouse_location(&entry.table_bucket, &entry.warehouse_location)?; - self.require_table_bucket(&entry.table_bucket).await?; - let _migration_guard = self - .begin_table_bucket_with_migration_permit(&entry.table_bucket, publication) - .await?; - if !publication.holds_table_bucket(&entry.table_bucket) { - return Err(TableCatalogStoreError::Internal( - "table registration requires a table-bucket publication fence".to_string(), - )); - } - let _publication_completion = TableCommitPublicationCompletion::new(publication); - self.recover_active_table_rename(&entry.table_bucket, publication).await?; - let namespace_path = self.paths.namespace_entry_path(&entry.table_bucket, &namespace); - let _namespace_guard = self - .backend - .acquire_write_lock(self.catalog_bucket(), &namespace_path) - .await?; - self.require_active_namespace_unlocked(&entry.table_bucket, &namespace, &namespace_path) - .await?; - let table_path = self.paths.table_entry_path(&entry.table_bucket, &namespace, &table); - let _table_guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; - let view_path = self.paths.view_entry_path(&entry.table_bucket, &namespace, &table); - if self - .read_entry_unlocked::(self.catalog_bucket(), &view_path) - .await? - .is_some() - { - return Err(TableCatalogStoreError::Conflict(format!( - "catalog object already exists: view {}/{}/{}", - entry.table_bucket, entry.namespace, entry.table - ))); - } - let mut precondition = precondition; - if matches!(precondition, TableCatalogPutPrecondition::IfAbsent) - && let Some((current, etag)) = self - .read_entry_unlocked::(self.catalog_bucket(), &table_path) + publication_preparation(publication, async { + validate_table_entry_version_and_id(&entry)?; + let namespace = parse_namespace_for_store(&entry.namespace)?; + let table = parse_table_for_store(&entry.table)?; + validate_table_warehouse_location(&entry.table_bucket, &entry.warehouse_location)?; + self.require_table_bucket(&entry.table_bucket).await?; + let _migration_guard = self + .begin_table_bucket_with_migration_permit(&entry.table_bucket, publication) + .await?; + if !publication.holds_table_bucket(&entry.table_bucket) { + return Err(TableCatalogStoreError::Internal( + "table registration requires a table-bucket publication fence".to_string(), + )); + } + retain_publication_preparation_fence(publication); + let _publication_completion = TableCommitPublicationCompletion::new(publication); + self.recover_active_table_rename(&entry.table_bucket, publication).await?; + let namespace_path = self.paths.namespace_entry_path(&entry.table_bucket, &namespace); + let _namespace_guard = self + .backend + .acquire_write_lock(self.catalog_bucket(), &namespace_path) + .await?; + self.require_active_namespace_unlocked(&entry.table_bucket, &namespace, &namespace_path) + .await?; + let table_path = self.paths.table_entry_path(&entry.table_bucket, &namespace, &table); + let _table_guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; + let view_path = self.paths.view_entry_path(&entry.table_bucket, &namespace, &table); + if self + .read_entry_unlocked::(self.catalog_bucket(), &view_path) .await? - { - validate_table_entry_object(&self.paths, &table_path, ¤t)?; - if current.state != TableCatalogEntryState::Deleted { + .is_some() + { return Err(TableCatalogStoreError::Conflict(format!( - "catalog object already exists: table {}/{}/{}", + "catalog object already exists: view {}/{}/{}", entry.table_bucket, entry.namespace, entry.table ))); } - precondition = etag - .map(TableCatalogPutPrecondition::IfMatch) - .ok_or_else(|| TableCatalogStoreError::Internal(format!("catalog table entry has no etag: {table_path}")))?; - } - // Preserve catalog -> publication -> object lock order across rolling upgrades. - publication - .prepare(&entry.table_bucket, &entry.namespace, &entry.table) - .await?; - if !publication.holds_table(&entry.table_bucket, &entry.namespace, &entry.table) { - return Err(TableCatalogStoreError::Internal( - "table registration requires a table publication fence".to_string(), - )); - } - self.check_registration_ownership(&entry, &table_path, publication).await?; - let reservation = self.reserve_table_warehouse_index(&entry, true, &table_path).await?; - if !publication.holds_table_bucket(&entry.table_bucket) - || !publication.holds_table(&entry.table_bucket, &entry.namespace, &entry.table) - { - self.rollback_table_warehouse_index_reservation(&entry, reservation, "table publication fence lost") + let mut precondition = precondition; + if matches!(precondition, TableCatalogPutPrecondition::IfAbsent) + && let Some((current, etag)) = self + .read_entry_unlocked::(self.catalog_bucket(), &table_path) + .await? + { + validate_table_entry_object(&self.paths, &table_path, ¤t)?; + if current.state != TableCatalogEntryState::Deleted { + return Err(TableCatalogStoreError::Conflict(format!( + "catalog object already exists: table {}/{}/{}", + entry.table_bucket, entry.namespace, entry.table + ))); + } + precondition = etag + .map(TableCatalogPutPrecondition::IfMatch) + .ok_or_else(|| TableCatalogStoreError::Internal(format!("catalog table entry has no etag: {table_path}")))?; + } + // Preserve catalog -> publication -> object lock order across rolling upgrades. + publication + .prepare(&entry.table_bucket, &entry.namespace, &entry.table) + .await?; + if !publication.holds_table(&entry.table_bucket, &entry.namespace, &entry.table) { + return Err(TableCatalogStoreError::Internal( + "table registration requires a table publication fence".to_string(), + )); + } + retain_publication_preparation_fence(publication); + self.check_registration_ownership(&entry, &table_path, publication).await?; + let reservation = self.reserve_table_warehouse_index(&entry, true, &table_path).await?; + if !publication.holds_table_bucket(&entry.table_bucket) + || !publication.holds_table(&entry.table_bucket, &entry.namespace, &entry.table) + { + self.rollback_table_warehouse_index_reservation(&entry, reservation, "table publication fence lost") + .await; + return Err(TableCatalogStoreError::Internal( + "table registration publication fence was lost before catalog update".to_string(), + )); + } + let mutation_fence = publication.object_mutation_fence().unwrap_or_default(); + let result = self + .write_entry_unlocked_with_fence( + self.catalog_bucket(), + &table_path, + &entry, + precondition, + _table_guard.write_commit_guards(), + Some(&mutation_fence), + ) .await; - return Err(TableCatalogStoreError::Internal( - "table registration publication fence was lost before catalog update".to_string(), - )); - } - let mutation_fence = publication.object_mutation_fence().unwrap_or_default(); - let result = self - .write_entry_unlocked_with_fence( - self.catalog_bucket(), - &table_path, - &entry, - precondition, - _table_guard.write_commit_guards(), - Some(&mutation_fence), - ) - .await; - if result.is_err() { - self.rollback_table_warehouse_index_reservation(&entry, reservation, "table entry write failed") - .await; - } - result?; - Ok(()) + if result.is_err() { + self.rollback_table_warehouse_index_reservation(&entry, reservation, "table entry write failed") + .await; + } + result?; + Ok(()) + }) + .await } async fn write_view_entry(&self, entry: ViewEntry, precondition: TableCatalogPutPrecondition) -> TableCatalogStoreResult<()> { @@ -4249,6 +4290,7 @@ where table.as_str() ))); }; + self.ensure_exclusive_maintenance_roots(&entry, Some(&table_path)).await?; self.run_conditional_table_maintenance_worker_for_entry(&entry, worker_id, condition, clock, &publication) .await } @@ -5095,6 +5137,10 @@ where P: TableCatalogObjectBackend, { validate_table_compaction_planning_config(&config)?; + // Never queue while holding publication locks. Workers share this + // permit so dropping the request cannot admit another decoder early. + let admission = try_admit_compaction()?; + let namespace = parse_namespace_for_store(namespace)?; let table = parse_table_for_store(table)?; let Some((entry, _)) = self.read_table_with_etag(table_bucket, &namespace, &table).await? else { @@ -5124,8 +5170,16 @@ where if report.status != TableCompactionPlanningStatus::RewriteCandidates { return Err(TableCatalogStoreError::Invalid("compaction has no safe rewrite candidates".to_string())); } - let current_data_files = - compaction_current_data_files(object_backend, table_bucket, &namespace, &table, &entry, ¤t_metadata).await?; + let current_data_files = compaction_current_data_files( + object_backend, + table_bucket, + &namespace, + &table, + &entry, + ¤t_metadata, + &admission, + ) + .await?; let current_data_files_by_key = current_data_files .iter() .map(|file| (file.object_key.as_str(), file)) @@ -5149,6 +5203,10 @@ where let compaction_id = Uuid::new_v4().to_string(); let mut compacted_files = Vec::with_capacity(report.rewrite_groups.len()); for rewrite_group in &mut report.rewrite_groups { + // The stored-input ceiling applies to one rewrite group: inputs are + // released after each group, so a multi-group job must not fail late + // with outputs already written. + let mut stored_input_bytes = 0_usize; let output_prefix = rewrite_group .input_file_locations .first() @@ -5160,23 +5218,46 @@ where let sort_order_id = compaction_rewrite_group_sort_order(¤t_data_files_by_key, rewrite_group)?; let mut input_files = Vec::with_capacity(rewrite_group.input_file_locations.len()); for input_file in &rewrite_group.input_file_locations { - let Some(input_object) = object_backend.read_object(table_bucket, input_file).await? else { + let remaining = TABLE_COMPACTION_MAX_STORED_BYTES.saturating_sub(stored_input_bytes); + let Some(input_object) = object_backend + .read_object_limited(table_bucket, input_file, remaining) + .await? + else { return Err(TableCatalogStoreError::NotFound(format!("compaction input data file {input_file}"))); }; + stored_input_bytes = stored_input_bytes + .checked_add(input_object.data.len()) + .filter(|size| *size <= TABLE_COMPACTION_MAX_STORED_BYTES) + .ok_or_else(|| { + TableCatalogStoreError::Invalid("compaction stored inputs exceed the server limit".to_owned()) + })?; input_files.push((input_file.clone(), input_object.data)); } - let compacted_file = compact_parquet_data_files(&input_files)?; + let compacted_file = run_compaction_worker(input_files, Arc::clone(&admission)).await?; let output_bytes = u64::try_from(compacted_file.data.len()).unwrap_or(u64::MAX); - object_backend - .put_object(table_bucket, &output_file, compacted_file.data, TableCatalogPutPrecondition::IfAbsent) - .await?; + // Keep output memory and admission with the actual write owner + // even if the request is cancelled during storage I/O. + let output_backend = object_backend.clone(); + let output_bucket = table_bucket.to_owned(); + let output_key = output_file.clone(); + let output_admission = Arc::clone(&admission); + let record_count = compacted_file.record_count; + run_compaction_output( + async move { + output_backend + .put_object(&output_bucket, &output_key, compacted_file.data, TableCatalogPutPrecondition::IfAbsent) + .await + }, + output_admission, + ) + .await?; rewrite_group.output_file_location = Some(output_file_path.clone()); rewrite_group.output_bytes = Some(output_bytes); compacted_files.push(CompactedDataFile { object_key: output_file, file_path: output_file_path, file_size_bytes: output_bytes, - record_count: compacted_file.record_count, + record_count, partition_spec_id, partition, sort_order_id, @@ -5345,6 +5426,35 @@ where diagnose_table_catalog_from_export(&self.backend, catalog, commit_recovery, retain_recent_metadata_files).await } + async fn ensure_exclusive_maintenance_roots( + &self, + entry: &TableEntry, + locked_table_entry: Option<&str>, + ) -> TableCatalogStoreResult<()> { + let roots = maintenance_cleanup_roots(entry)?; + for other in self + .list_all_table_entries_with_limit(&entry.table_bucket, None, locked_table_entry, TABLE_REGISTRATION_READ_CONCURRENCY) + .await? + { + if other.state == TableCatalogEntryState::Deleted + || (other.namespace == entry.namespace && other.table == entry.table && other.table_id == entry.table_id) + { + continue; + } + let other_roots = maintenance_cleanup_roots(&other)?; + if roots.iter().any(|root| { + other_roots + .iter() + .any(|other_root| warehouse_object_prefixes_overlap(root, other_root)) + }) { + return Err(TableCatalogStoreError::Conflict( + "table maintenance requires exclusive metadata and warehouse roots".to_string(), + )); + } + } + Ok(()) + } + pub(crate) async fn plan_table_metadata_maintenance( &self, table_bucket: &str, @@ -5362,6 +5472,7 @@ where table.as_str() ))); }; + self.ensure_exclusive_maintenance_roots(&entry, None).await?; self.plan_table_metadata_maintenance_for_entry_with_scope(&entry, retain_recent_metadata_files, false) .await } @@ -5999,10 +6110,16 @@ where )); } + let publication = TableCommitLockPublication::new(&self.backend); + publication.begin_table_bucket(table_bucket).await?; + let _publication_completion = TableCommitPublicationCompletion::new(&publication); + self.ensure_no_active_table_rename(table_bucket).await?; let table_path = self.paths.table_entry_path(table_bucket, &namespace, &table); - let _guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; - let publication_lock = default_table_publication_lock_path(&namespace, &table); - let _publication_guard = self.backend.acquire_write_lock(table_bucket, &publication_lock).await?; + let guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; + publication.attach_catalog_write_guard(guard)?; + publication + .prepare(table_bucket, &namespace.public_name(), table.as_str()) + .await?; let Some((entry, _)) = self.read_table_with_etag_unlocked(table_bucket, &namespace, &table).await? else { return Err(TableCatalogStoreError::NotFound(format!( "table {}/{}/{}", @@ -6011,15 +6128,8 @@ where table.as_str() ))); }; - self.delete_table_metadata_maintenance_report_for_entry(&entry, report).await - } - - pub(super) async fn delete_table_metadata_maintenance_report_for_entry( - &self, - entry: &TableEntry, - report: TableMetadataMaintenanceReport, - ) -> TableCatalogStoreResult { - self.delete_table_metadata_maintenance_report_for_entry_with_publication(entry, report, None) + self.ensure_exclusive_maintenance_roots(&entry, Some(&table_path)).await?; + self.delete_table_metadata_maintenance_report_for_entry_with_publication(&entry, report, Some(&publication)) .await } @@ -6033,6 +6143,7 @@ where let table_bucket = &entry.table_bucket; let namespace = parse_namespace_for_store(&entry.namespace)?; let table = parse_table_for_store(&entry.table)?; + validate_table_maintenance_report_owner(&report, table_bucket, &namespace, &table, &entry.table_id)?; let is_valid_location = |location: &str| { if publication.is_some() { is_valid_table_metadata_location_for_entry(entry, location) @@ -6185,7 +6296,7 @@ where "cleanup object candidate {object_location} is retained by current metadata" ))); } - let Some(candidate_object) = self.backend.read_object(table_bucket, object_location).await? else { + let Some(candidate_object) = self.backend.object_metadata(table_bucket, object_location).await? else { continue; }; if !planned_deletable_object_locations.contains(object_location.as_str()) { @@ -6973,131 +7084,43 @@ where request: TableCommitRequest, publication: &(dyn TableCommitPublication + Sync), ) -> TableCatalogStoreResult { - let commit_started = Instant::now(); - record_table_commit_attempt(&request.operation); - let namespace = parse_namespace_for_store(&request.namespace)?; - let table = parse_table_for_store(&request.table)?; - let _migration_guard = self - .begin_table_bucket_with_migration_permit(&request.table_bucket, publication) - .await?; - if !publication.holds_table_bucket(&request.table_bucket) { - return Err(TableCatalogStoreError::Internal( - "table commit requires a table-bucket publication fence".to_string(), - )); - } - if publication.holds_table_bucket(&request.table_bucket) { - self.recover_active_table_rename(&request.table_bucket, publication).await?; - } else { - self.ensure_no_active_table_rename(&request.table_bucket).await?; - } - let table_path = self.paths.table_entry_path(&request.table_bucket, &namespace, &table); - let _guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; - // Preserve catalog -> publication -> object lock order across rolling upgrades. - publication - .prepare(&request.table_bucket, &request.namespace, &request.table) - .await?; - if !publication.holds_table(&request.table_bucket, &request.namespace, &request.table) { - return Err(TableCatalogStoreError::Internal( - "table commit requires a table publication fence".to_string(), - )); - } - let _publication_completion = TableCommitPublicationCompletion::new(publication); - - let Some((current, current_etag)) = self - .read_table_with_etag_unlocked(&request.table_bucket, &namespace, &table) - .await? - else { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::NotFound(format!( - "table {}/{}/{}", - request.table_bucket, request.namespace, request.table - ))), - ); - }; - if current.state != TableCatalogEntryState::Active { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::NotFound(format!( - "table {}/{}/{}", - request.table_bucket, request.namespace, request.table - ))), - ); - } - - let commit_path = self - .paths - .commit_log_entry_path(&request.table_bucket, ¤t.table_id, &request.commit_id); - let existing_commit = self - .read_commit_log_entry(&request.table_bucket, ¤t.table_id, &request.commit_id) - .await?; - let idempotency_key = request.idempotency_key.as_deref(); - let idempotency_path = idempotency_key.map(|idempotency_key| { - self.paths - .commit_idempotency_entry_path(&request.table_bucket, ¤t.table_id, idempotency_key) - }); - let existing_idempotency_commit = match idempotency_key { - Some(idempotency_key) => { - self.read_commit_idempotency_entry(&request.table_bucket, ¤t.table_id, idempotency_key) - .await? + publication_preparation(publication, async { + let commit_started = Instant::now(); + record_table_commit_attempt(&request.operation); + let namespace = parse_namespace_for_store(&request.namespace)?; + let table = parse_table_for_store(&request.table)?; + let _migration_guard = self + .begin_table_bucket_with_migration_permit(&request.table_bucket, publication) + .await?; + if !publication.holds_table_bucket(&request.table_bucket) { + return Err(TableCatalogStoreError::Internal( + "table commit requires a table-bucket publication fence".to_string(), + )); } - None => None, - }; + retain_publication_preparation_fence(publication); + if publication.holds_table_bucket(&request.table_bucket) { + self.recover_active_table_rename(&request.table_bucket, publication).await?; + } else { + self.ensure_no_active_table_rename(&request.table_bucket).await?; + } + let table_path = self.paths.table_entry_path(&request.table_bucket, &namespace, &table); + let _guard = self.backend.acquire_write_lock(self.catalog_bucket(), &table_path).await?; + // Preserve catalog -> publication -> object lock order across rolling upgrades. + publication + .prepare(&request.table_bucket, &request.namespace, &request.table) + .await?; + if !publication.holds_table(&request.table_bucket, &request.namespace, &request.table) { + return Err(TableCatalogStoreError::Internal( + "table commit requires a table publication fence".to_string(), + )); + } + retain_publication_preparation_fence(publication); + let _publication_completion = TableCommitPublicationCompletion::new(publication); - if let (Some(existing), Some(indexed)) = (&existing_commit, &existing_idempotency_commit) - && !commit_logs_share_recovery_payload(existing, indexed) - { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Conflict( - "commit record and idempotency index contain different payloads".to_string(), - )), - ); - } - if let Some(existing) = existing_idempotency_commit.as_ref() - && !commit_log_matches_request(existing, &request, ¤t.table_id) - { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Conflict("idempotency key already exists".to_string())), - ); - } - if existing_commit.is_none() && existing_idempotency_commit.is_some() { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Conflict( - "idempotency key exists without a recoverable commit record".to_string(), - )), - ); - } - - if let Some(existing) = existing_commit.as_ref() { - if !commit_log_matches_request(existing, &request, ¤t.table_id) { + let Some((current, current_etag)) = self + .read_table_with_etag_unlocked(&request.table_bucket, &namespace, &table) + .await? + else { return table_commit_result( &request.table_bucket, &request.namespace, @@ -7105,24 +7128,49 @@ where &request.commit_id, &request.operation, commit_started, - Err(TableCatalogStoreError::Conflict(format!( - "commit id already exists: {}", - request.commit_id + Err(TableCatalogStoreError::NotFound(format!( + "table {}/{}/{}", + request.table_bucket, request.namespace, request.table + ))), + ); + }; + if current.state != TableCatalogEntryState::Active { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::NotFound(format!( + "table {}/{}/{}", + request.table_bucket, request.namespace, request.table ))), ); } - if matches!(existing.status, CommitLogStatus::Failed) { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Conflict("failed commit record cannot be replayed".to_string())), - ); - } - if matches!(existing.status, CommitLogStatus::Committed) && table_matches_staged_base(¤t, existing) { + + let commit_path = self + .paths + .commit_log_entry_path(&request.table_bucket, ¤t.table_id, &request.commit_id); + let existing_commit = self + .read_commit_log_entry(&request.table_bucket, ¤t.table_id, &request.commit_id) + .await?; + let idempotency_key = request.idempotency_key.as_deref(); + let idempotency_path = idempotency_key.map(|idempotency_key| { + self.paths + .commit_idempotency_entry_path(&request.table_bucket, ¤t.table_id, idempotency_key) + }); + let existing_idempotency_commit = match idempotency_key { + Some(idempotency_key) => { + self.read_commit_idempotency_entry(&request.table_bucket, ¤t.table_id, idempotency_key) + .await? + } + None => None, + }; + + if let (Some(existing), Some(indexed)) = (&existing_commit, &existing_idempotency_commit) + && !commit_logs_share_recovery_payload(existing, indexed) + { return table_commit_result( &request.table_bucket, &request.namespace, @@ -7131,32 +7179,270 @@ where &request.operation, commit_started, Err(TableCatalogStoreError::Conflict( - "committed record still matches the pre-commit table state".to_string(), + "commit record and idempotency index contain different payloads".to_string(), )), ); } - let historically_committed = if matches!(existing.status, CommitLogStatus::Staged) - && !table_matches_staged_base(¤t, existing) - && !table_matches_committed_log(¤t, existing) + if let Some(existing) = existing_idempotency_commit.as_ref() + && !commit_log_matches_request(existing, &request, ¤t.table_id) { - let commit_logs = self - .read_table_commit_logs(¤t) - .await? - .into_iter() - .map(|(_, commit_log)| commit_log) - .collect::>(); - TableCommitHistoryIndex::new(¤t, commit_logs.iter()).proves_committed(existing) - } else { - false + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict("idempotency key already exists".to_string())), + ); + } + if existing_commit.is_none() && existing_idempotency_commit.is_some() { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict( + "idempotency key exists without a recoverable commit record".to_string(), + )), + ); + } + + if let Some(existing) = existing_commit.as_ref() { + if !commit_log_matches_request(existing, &request, ¤t.table_id) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict(format!( + "commit id already exists: {}", + request.commit_id + ))), + ); + } + if matches!(existing.status, CommitLogStatus::Failed) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict("failed commit record cannot be replayed".to_string())), + ); + } + if matches!(existing.status, CommitLogStatus::Committed) && table_matches_staged_base(¤t, existing) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict( + "committed record still matches the pre-commit table state".to_string(), + )), + ); + } + let historically_committed = if matches!(existing.status, CommitLogStatus::Staged) + && !table_matches_staged_base(¤t, existing) + && !table_matches_committed_log(¤t, existing) + { + let commit_logs = self + .read_table_commit_logs(¤t) + .await? + .into_iter() + .map(|(_, commit_log)| commit_log) + .collect::>(); + TableCommitHistoryIndex::new(¤t, commit_logs.iter()).proves_committed(existing) + } else { + false + }; + if matches!(existing.status, CommitLogStatus::Committed) + || (matches!(existing.status, CommitLogStatus::Staged) + && (table_matches_committed_log(¤t, existing) || historically_committed)) + { + let mut committed = existing.clone(); + committed.status = CommitLogStatus::Committed; + let _ = self + .finalize_commit_log(&commit_path, idempotency_path.as_deref(), &committed) + .await; + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Ok(TableCommitResult { + table: current, + commit_log: committed, + }), + ); + } + if !matches!(existing.status, CommitLogStatus::Staged) || !table_matches_staged_base(¤t, existing) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict( + "existing commit record does not match current table state".to_string(), + )), + ); + } + } + + if current.version_token != request.expected_version_token { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict( + "current table version token does not match expected token".to_string(), + )), + ); + } + if current.metadata_location != request.expected_metadata_location { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Conflict( + "current table metadata location does not match expected location".to_string(), + )), + ); + } + if !is_valid_table_metadata_location_for_entry(¤t, &request.new_metadata_location) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Invalid( + "new metadata location must be inside the table metadata directory".to_string(), + )), + ); + } + let Some(new_metadata_object) = self + .backend + .read_object_limited(&request.table_bucket, &request.new_metadata_location, TABLE_METADATA_JSON_MAX_SIZE) + .await? + else { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::NotFound(format!( + "new metadata object {}", + request.new_metadata_location + ))), + ); }; - if matches!(existing.status, CommitLogStatus::Committed) - || (matches!(existing.status, CommitLogStatus::Staged) - && (table_matches_committed_log(¤t, existing) || historically_committed)) - { - let mut committed = existing.clone(); - committed.status = CommitLogStatus::Committed; - let _ = self - .finalize_commit_log(&commit_path, idempotency_path.as_deref(), &committed) + validate_commit_metadata_digest(&request, &new_metadata_object)?; + let table_bucket = request.table_bucket.clone(); + let metadata_location = request.new_metadata_location.clone(); + let next_metadata_state = tokio::task::spawn_blocking(move || { + table_metadata_commit_state(&table_bucket, &metadata_location, &new_metadata_object) + }) + .await + .map_err(|err| TableCatalogStoreError::Internal(format!("table metadata parser task failed: {err}")))??; + let warehouse_relocation = next_metadata_state + .warehouse_location + .as_ref() + .is_some_and(|warehouse_location| warehouse_location != ¤t.warehouse_location); + if warehouse_relocation && !publication.holds_table_bucket(&request.table_bucket) { + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(TableCatalogStoreError::Internal( + "table warehouse relocation requires a table-bucket publication fence".to_string(), + )), + ); + } + + let has_existing_commit = existing_commit.is_some(); + let mut staged_commit_log = existing_commit.unwrap_or_else(|| CommitLogEntry { + version: TABLE_CATALOG_ENTRY_VERSION, + commit_id: request.commit_id.clone(), + idempotency_key: request.idempotency_key.clone(), + table_id: current.table_id.clone(), + operation: request.operation.clone(), + expected_version_token: request.expected_version_token.clone(), + new_version_token: format!("token-{}", Uuid::new_v4()), + previous_metadata_location: current.metadata_location.clone(), + new_metadata_location: request.new_metadata_location.clone(), + requirements: request.requirements.clone(), + status: CommitLogStatus::Staged, + writer: request.writer.clone(), + created_at: None, + updated_at: None, + }); + staged_commit_log.status = CommitLogStatus::Staged; + + let mut next = current.clone(); + next.metadata_location = staged_commit_log.new_metadata_location.clone(); + if let Some(warehouse_location) = next_metadata_state.warehouse_location { + next.warehouse_location = warehouse_location; + } + if let Some(format_version) = next_metadata_state.format_version { + next.format_version = format_version; + } + next.version_token = staged_commit_log.new_version_token.clone(); + next.generation = current.generation.saturating_add(1); + if next.warehouse_location != current.warehouse_location { + self.ensure_table_warehouse_prefix_available(&next, Some(&table_path)).await?; + } + let reservation = self.reserve_table_warehouse_index(&next, false, &table_path).await?; + + let staged_write_result = async { + if !has_existing_commit { + self.write_entry( + self.catalog_bucket(), + &commit_path, + &staged_commit_log, + TableCatalogPutPrecondition::IfAbsent, + ) + .await?; + } + if let Some(idempotency_path) = idempotency_path.as_deref() + && existing_idempotency_commit.is_none() + { + self.write_entry( + self.catalog_bucket(), + idempotency_path, + &staged_commit_log, + TableCatalogPutPrecondition::IfAbsent, + ) + .await?; + } + Ok(()) + } + .await; + if let Err(err) = staged_write_result { + self.rollback_table_warehouse_index_reservation(&next, reservation, "commit staging failed") .await; return table_commit_result( &request.table_bucket, @@ -7165,13 +7451,15 @@ where &request.commit_id, &request.operation, commit_started, - Ok(TableCommitResult { - table: current, - commit_log: committed, - }), + Err(err), ); } - if !matches!(existing.status, CommitLogStatus::Staged) || !table_matches_staged_base(¤t, existing) { + + if !publication.holds_table(&request.table_bucket, &request.namespace, &request.table) + || (warehouse_relocation && !publication.holds_table_bucket(&request.table_bucket)) + { + self.rollback_table_warehouse_index_reservation(&next, reservation, "table publication fence lost") + .await; return table_commit_result( &request.table_bucket, &request.namespace, @@ -7179,231 +7467,59 @@ where &request.commit_id, &request.operation, commit_started, - Err(TableCatalogStoreError::Conflict( - "existing commit record does not match current table state".to_string(), + Err(TableCatalogStoreError::Internal( + "table commit publication fence was lost before pointer update".to_string(), )), ); } - } - if current.version_token != request.expected_version_token { - return table_commit_result( + let cas_started = Instant::now(); + let mutation_fence = publication.object_mutation_fence().unwrap_or_default(); + let cas_result = self + .write_entry_unlocked_with_fence( + self.catalog_bucket(), + &table_path, + &next, + TableCatalogPutPrecondition::IfMatch(current_etag), + _guard.write_commit_guards(), + Some(&mutation_fence), + ) + .await; + record_table_commit_cas_result(&request.operation, cas_started, &cas_result); + if let Err(err) = cas_result { + self.rollback_table_warehouse_index_reservation(&next, reservation, "table pointer CAS failed") + .await; + return table_commit_result( + &request.table_bucket, + &request.namespace, + &request.table, + &request.commit_id, + &request.operation, + commit_started, + Err(err), + ); + } + self.delete_table_warehouse_index_if_changed(¤t, &next).await; + + let mut commit_log = staged_commit_log; + commit_log.status = CommitLogStatus::Committed; + // After the table CAS succeeds, the staged record is the durable recovery source. + // A finalization failure must not turn an externally committed pointer into a failed commit response. + let _ = self + .finalize_commit_log(&commit_path, idempotency_path.as_deref(), &commit_log) + .await; + + table_commit_result( &request.table_bucket, &request.namespace, &request.table, &request.commit_id, &request.operation, commit_started, - Err(TableCatalogStoreError::Conflict( - "current table version token does not match expected token".to_string(), - )), - ); - } - if current.metadata_location != request.expected_metadata_location { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Conflict( - "current table metadata location does not match expected location".to_string(), - )), - ); - } - if !is_valid_table_metadata_location_for_entry(¤t, &request.new_metadata_location) { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Invalid( - "new metadata location must be inside the table metadata directory".to_string(), - )), - ); - } - let Some(new_metadata_object) = self - .backend - .read_object_limited(&request.table_bucket, &request.new_metadata_location, TABLE_METADATA_JSON_MAX_SIZE) - .await? - else { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::NotFound(format!( - "new metadata object {}", - request.new_metadata_location - ))), - ); - }; - validate_commit_metadata_digest(&request, &new_metadata_object)?; - let table_bucket = request.table_bucket.clone(); - let metadata_location = request.new_metadata_location.clone(); - let next_metadata_state = tokio::task::spawn_blocking(move || { - table_metadata_commit_state(&table_bucket, &metadata_location, &new_metadata_object) + Ok(TableCommitResult { table: next, commit_log }), + ) }) .await - .map_err(|err| TableCatalogStoreError::Internal(format!("table metadata parser task failed: {err}")))??; - let warehouse_relocation = next_metadata_state - .warehouse_location - .as_ref() - .is_some_and(|warehouse_location| warehouse_location != ¤t.warehouse_location); - if warehouse_relocation && !publication.holds_table_bucket(&request.table_bucket) { - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Internal( - "table warehouse relocation requires a table-bucket publication fence".to_string(), - )), - ); - } - - let has_existing_commit = existing_commit.is_some(); - let mut staged_commit_log = existing_commit.unwrap_or_else(|| CommitLogEntry { - version: TABLE_CATALOG_ENTRY_VERSION, - commit_id: request.commit_id.clone(), - idempotency_key: request.idempotency_key.clone(), - table_id: current.table_id.clone(), - operation: request.operation.clone(), - expected_version_token: request.expected_version_token.clone(), - new_version_token: format!("token-{}", Uuid::new_v4()), - previous_metadata_location: current.metadata_location.clone(), - new_metadata_location: request.new_metadata_location.clone(), - requirements: request.requirements.clone(), - status: CommitLogStatus::Staged, - writer: request.writer.clone(), - created_at: None, - updated_at: None, - }); - staged_commit_log.status = CommitLogStatus::Staged; - - let mut next = current.clone(); - next.metadata_location = staged_commit_log.new_metadata_location.clone(); - if let Some(warehouse_location) = next_metadata_state.warehouse_location { - next.warehouse_location = warehouse_location; - } - if let Some(format_version) = next_metadata_state.format_version { - next.format_version = format_version; - } - next.version_token = staged_commit_log.new_version_token.clone(); - next.generation = current.generation.saturating_add(1); - if next.warehouse_location != current.warehouse_location { - self.ensure_table_warehouse_prefix_available(&next, Some(&table_path)).await?; - } - let reservation = self.reserve_table_warehouse_index(&next, false, &table_path).await?; - - let staged_write_result = async { - if !has_existing_commit { - self.write_entry( - self.catalog_bucket(), - &commit_path, - &staged_commit_log, - TableCatalogPutPrecondition::IfAbsent, - ) - .await?; - } - if let Some(idempotency_path) = idempotency_path.as_deref() - && existing_idempotency_commit.is_none() - { - self.write_entry( - self.catalog_bucket(), - idempotency_path, - &staged_commit_log, - TableCatalogPutPrecondition::IfAbsent, - ) - .await?; - } - Ok(()) - } - .await; - if let Err(err) = staged_write_result { - self.rollback_table_warehouse_index_reservation(&next, reservation, "commit staging failed") - .await; - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(err), - ); - } - - if !publication.holds_table(&request.table_bucket, &request.namespace, &request.table) - || (warehouse_relocation && !publication.holds_table_bucket(&request.table_bucket)) - { - self.rollback_table_warehouse_index_reservation(&next, reservation, "table publication fence lost") - .await; - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(TableCatalogStoreError::Internal( - "table commit publication fence was lost before pointer update".to_string(), - )), - ); - } - - let cas_started = Instant::now(); - let mutation_fence = publication.object_mutation_fence().unwrap_or_default(); - let cas_result = self - .write_entry_unlocked_with_fence( - self.catalog_bucket(), - &table_path, - &next, - TableCatalogPutPrecondition::IfMatch(current_etag), - _guard.write_commit_guards(), - Some(&mutation_fence), - ) - .await; - record_table_commit_cas_result(&request.operation, cas_started, &cas_result); - if let Err(err) = cas_result { - self.rollback_table_warehouse_index_reservation(&next, reservation, "table pointer CAS failed") - .await; - return table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Err(err), - ); - } - self.delete_table_warehouse_index_if_changed(¤t, &next).await; - - let mut commit_log = staged_commit_log; - commit_log.status = CommitLogStatus::Committed; - // After the table CAS succeeds, the staged record is the durable recovery source. - // A finalization failure must not turn an externally committed pointer into a failed commit response. - let _ = self - .finalize_commit_log(&commit_path, idempotency_path.as_deref(), &commit_log) - .await; - - table_commit_result( - &request.table_bucket, - &request.namespace, - &request.table, - &request.commit_id, - &request.operation, - commit_started, - Ok(TableCommitResult { table: next, commit_log }), - ) } async fn drop_table(&self, table_bucket: &str, namespace: &str, table: &str) -> TableCatalogStoreResult<()> { diff --git a/rustfs/src/table_catalog/store/publication_deadline.rs b/rustfs/src/table_catalog/store/publication_deadline.rs new file mode 100644 index 000000000..3827963ca --- /dev/null +++ b/rustfs/src/table_catalog/store/publication_deadline.rs @@ -0,0 +1,185 @@ +// 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::*; +use std::{ + future::Future, + sync::{ + OnceLock, + atomic::{AtomicBool, Ordering}, + }, + task::Poll, +}; + +/// One admission budget, retained across repeated preparation and completed recovery. +/// It never cancels a transaction after its first possible durable side effect. +#[derive(Default)] +pub(crate) struct TablePublicationPreparation { + deadline: OnceLock, + mutation_started: AtomicBool, + fence: parking_lot::Mutex>, +} + +tokio::task_local! { + static PUBLICATION_PREPARATION: Arc; +} + +/// Budget for one publication preparation: the configured interval, never +/// shorter than the lock-acquire timeout that queueing alone may consume. +pub(crate) fn publication_preparation_timeout() -> std::time::Duration { + std::time::Duration::from_secs(rustfs_utils::get_env_u64( + crate::table_catalog::ENV_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS, + crate::table_catalog::DEFAULT_TABLE_CATALOG_PUBLICATION_PREPARATION_TIMEOUT_SECS, + )) + .max(get_lock_acquire_timeout()) +} + +impl TablePublicationPreparation { + pub(super) fn deadline(&self) -> tokio::time::Instant { + *self + .deadline + .get_or_init(|| tokio::time::Instant::now() + publication_preparation_timeout()) + } + + fn expired() -> TableCatalogStoreError { + TableCatalogStoreError::Unavailable("table publication preparation deadline exceeded; retry the operation".to_string()) + } + + fn check(&self) -> TableCatalogStoreResult<()> { + if !self.mutation_started.load(Ordering::Acquire) && tokio::time::Instant::now() >= self.deadline() { + return Err(Self::expired()); + } + Ok(()) + } + + /// Fail once the budget is spent without committing the transaction. + pub(crate) fn check_deadline(&self) -> TableCatalogStoreResult<()> { + self.check() + } + + pub(crate) fn begin_mutation(&self) -> TableCatalogStoreResult<()> { + self.check()?; + if let Some(fence) = self.fence.lock().as_ref() { + fence.ensure_held()?; + } + self.mutation_started.store(true, Ordering::Release); + Ok(()) + } + + // Only use for pure acquisition/validation. The publication is cleared on + // timeout or caller cancellation, including partially acquired guards. + pub(crate) async fn read( + &self, + publication: &(dyn TableCommitPublication + Sync), + future: impl Future>, + ) -> TableCatalogStoreResult { + let mut cleanup = PreparationReadCleanup { + publication, + armed: true, + }; + let result = if self.mutation_started.load(Ordering::Acquire) { + future.await + } else { + tokio::time::timeout_at(self.deadline(), future) + .await + .map_err(|_| Self::expired())? + }; + if result.is_ok() { + self.check()?; + } + cleanup.armed = false; + result + } + + async fn run(&self, future: impl Future>) -> TableCatalogStoreResult { + let timer = tokio::time::sleep_until(self.deadline()); + tokio::pin!(timer, future); + std::future::poll_fn(|cx| { + // Check before polling new work. A mutation boundary also checks + // synchronously, so CPU work cannot publish after an expired timer. + if !self.mutation_started.load(Ordering::Acquire) && timer.as_mut().poll(cx).is_ready() { + return Poll::Ready(Err(Self::expired())); + } + match future.as_mut().poll(cx) { + Poll::Ready(result) => Poll::Ready(self.check().and(result)), + Poll::Pending => Poll::Pending, + } + }) + .await + } +} + +struct PreparationReadCleanup<'a> { + publication: &'a (dyn TableCommitPublication + Sync), + armed: bool, +} + +impl Drop for PreparationReadCleanup<'_> { + fn drop(&mut self) { + if self.armed { + self.publication.complete(); + } + } +} + +pub(super) async fn publication_preparation( + publication: &(dyn TableCommitPublication + Sync), + future: impl Future>, +) -> TableCatalogStoreResult { + let _completion = TableCommitPublicationCompletion::new(publication); + let preparation = publication.preparation_budget(); + PUBLICATION_PREPARATION + .scope(preparation.clone(), preparation.run(future)) + .await +} + +pub(super) fn retain_publication_preparation_fence(publication: &(dyn TableCommitPublication + Sync)) { + let _ = PUBLICATION_PREPARATION.try_with(|preparation| { + *preparation.fence.lock() = publication.object_mutation_fence(); + }); +} + +/// Call immediately before the first possible persistent side effect, after +/// read-only validation and existing lease checks. A failed/ambiguous write is +/// still a mutation: recovery must retain the original cancellation semantics. +pub(super) fn begin_publication_mutation() -> TableCatalogStoreResult<()> { + PUBLICATION_PREPARATION + .try_with(|preparation| preparation.begin_mutation()) + .unwrap_or(Ok(())) +} + +/// Recovery is a preceding transaction, not permission for the new transaction +/// to bypass admission. Complete its original protocol once a write starts, +/// then resume the unchanged deadline before any new preparation. +pub(super) async fn publication_recovery( + future: impl Future>, +) -> TableCatalogStoreResult { + let preparation = PUBLICATION_PREPARATION.try_with(Arc::clone).ok(); + let already_mutating = preparation + .as_ref() + .is_some_and(|budget| budget.mutation_started.load(Ordering::Acquire)); + let result = future.await; + if result.is_ok() + && !already_mutating + && let Some(preparation) = preparation + { + preparation.mutation_started.store(false, Ordering::Release); + preparation.check()?; + } + result +} + +#[cfg(test)] +#[path = "publication_deadline_tests.rs"] +mod tests; diff --git a/rustfs/src/table_catalog/store/publication_deadline_tests.rs b/rustfs/src/table_catalog/store/publication_deadline_tests.rs new file mode 100644 index 000000000..98709f408 --- /dev/null +++ b/rustfs/src/table_catalog/store/publication_deadline_tests.rs @@ -0,0 +1,331 @@ +// 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::*; +use crate::table_catalog::store::registration_cache_tests::{cache_fixture, registration_entry}; +use crate::table_catalog::test_support::TestCatalogObjectBackend; +use crate::table_catalog::tests::{test_bucket_entry, test_namespace_entry, test_table_entry}; + +fn is_preparation_timeout(result: &TableCatalogStoreResult) -> bool { + matches!(result, Err(TableCatalogStoreError::Unavailable(message)) if message.contains("preparation deadline")) +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_registration_inventory_releases_fence_without_writes() { + let (backend, store, namespace) = cache_fixture().await; + backend.registration_cache.clear(); + let path = store + .paths + .table_entry_path("analytics", &namespace, &IdentifierSegment::parse("orders").unwrap()); + let pause = backend.pause_before_next_read(RUSTFS_META_BUCKET, &path).await; + let before = backend.state.lock().await.put_attempts.clone(); + let registering = { + let store = store.clone(); + let entry = registration_entry(&namespace, "candidate"); + tokio::spawn(async move { store.register_table(entry).await }) + }; + pause.wait_started().await; + let writer = { + let backend = backend.clone(); + tokio::spawn(async move { + // Data-plane mutations acquire this same read fence before writing. + let _guard = backend + .acquire_read_lock("analytics", &default_table_bucket_publication_lock_path()) + .await?; + backend + .put_object( + "analytics", + "tables/other-tenant/data/new", + b"new".to_vec(), + TableCatalogPutPrecondition::IfAbsent, + ) + .await + }) + }; + tokio::task::yield_now().await; + assert!(!writer.is_finished()); + tokio::time::advance(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + assert!(is_preparation_timeout(®istering.await.unwrap())); + writer.await.unwrap().unwrap(); + pause.release(); + assert!(store.load_table("analytics", "sales", "candidate").await.unwrap().is_none()); + let after = backend.state.lock().await.put_attempts.clone(); + for (key, attempts) in after { + if key.0 == RUSTFS_META_BUCKET { + assert_eq!(Some(&attempts), before.get(&key), "timed-out preparation must not stage catalog writes"); + } + } + store + .create_table(registration_entry(&namespace, "after_timeout")) + .await + .unwrap(); +} + +async fn commit_fixture(strong: bool) -> (TestCatalogObjectBackend, Arc, TableCommitRequest) { + let backend = TestCatalogObjectBackend::default(); + let store: Arc = if strong { + Arc::new(StrongTableCatalogStore::new(backend.clone())) + } else { + Arc::new(ObjectTableCatalogStore::new(backend.clone())) + }; + let namespace = Namespace::parse("sales").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let current = default_table_metadata_file_path(&namespace, &table, "00001.metadata.json"); + let next = default_table_metadata_file_path(&namespace, &table, "00002.metadata.json"); + store.put_table_bucket(test_bucket_entry("analytics")).await.unwrap(); + store + .create_namespace(test_namespace_entry("analytics", &namespace)) + .await + .unwrap(); + store + .create_table(test_table_entry("analytics", &namespace, &table, current.clone())) + .await + .unwrap(); + backend.seed_object("analytics", &next, b"{}".to_vec()).await; + ( + backend, + store, + TableCommitRequest { + table_bucket: "analytics".to_string(), + namespace: "sales".to_string(), + table: "orders".to_string(), + commit_id: "deadline-commit".to_string(), + idempotency_key: Some("deadline-idempotency".to_string()), + operation: "append".to_string(), + expected_version_token: "token-v1".to_string(), + expected_metadata_location: current, + new_metadata_location: next, + requirements: Vec::new(), + writer: None, + }, + ) +} + +// The durable-strong backing does not take part in the preparation deadline; it is retired separately, +// so only the object backing is exercised here. +#[tokio::test(start_paused = true)] +async fn publication_deadline_commit_metadata_read_does_not_publish_object_backing() { + let (backend, store, request) = commit_fixture(false).await; + let pause = backend + .pause_before_next_read("analytics", &request.new_metadata_location) + .await; + let before = backend.state.lock().await.put_attempts.clone(); + let committing = { + let store = store.clone(); + let request = request.clone(); + tokio::spawn(async move { store.commit_table(request).await }) + }; + pause.wait_started().await; + tokio::time::advance(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + assert!(is_preparation_timeout(&committing.await.unwrap())); + assert_eq!(backend.state.lock().await.put_attempts, before); + assert_eq!( + store + .load_table("analytics", "sales", "orders") + .await + .unwrap() + .unwrap() + .generation, + 1 + ); + pause.release(); + // Positive retry exercises parsing a nontrivial valid metadata body. + backend + .seed_object( + "analytics", + &request.new_metadata_location, + serde_json::to_vec(&serde_json::json!({"properties": {"padding": "x".repeat(1024 * 1024)}})).unwrap(), + ) + .await; + tokio::time::resume(); + let committed = store.commit_table(request.clone()).await.unwrap(); + assert_eq!(committed.table.generation, 2); + assert_eq!( + store.commit_table(request).await.unwrap().table.generation, + 2, + "retry must remain idempotent" + ); +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_create_finishes_after_first_write_on_either_backing() { + for strong in [false, true] { + let (backend, store, _) = commit_fixture(strong).await; + let namespace = Namespace::parse("sales").unwrap(); + let candidate = registration_entry(&namespace, "candidate"); + let paths = TableCatalogObjectPaths::default(); + let first_write = if strong { + StrongTableCatalogStore::::snapshot_object_path() + } else { + paths.warehouse_index_entry_path("analytics", "tables/id-candidate/") + }; + let pause = backend.pause_next_put(RUSTFS_META_BUCKET, &first_write).await; + let creating = { + let store = store.clone(); + tokio::spawn(async move { store.create_table(candidate).await }) + }; + pause.wait_started().await; + tokio::time::advance(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + tokio::task::yield_now().await; + assert!(!creating.is_finished(), "deadline must not cancel a possible durable write"); + pause.release(); + creating.await.unwrap().unwrap(); + assert!(store.load_table("analytics", "sales", "candidate").await.unwrap().is_some()); + } +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_queueing_and_repeated_prepare_share_one_budget() { + let backend = TestCatalogObjectBackend::default(); + let publication = TableCommitLockPublication::new(&backend); + let budget = publication.preparation_budget(); + let first_deadline = budget.deadline(); + tokio::time::advance(publication_preparation_timeout() - StdDuration::from_millis(1)).await; + publication.begin_table_bucket("analytics").await.unwrap(); + publication.prepare("analytics", "sales", "orders").await.unwrap(); + assert_eq!(budget.deadline(), first_deadline); + let result = publication_preparation(&publication, async { + publication.prepare("analytics", "sales", "orders").await?; + tokio::time::sleep(StdDuration::from_millis(2)).await; + begin_publication_mutation() + }) + .await; + assert!(is_preparation_timeout(&result)); + assert!(!publication.holds_table_bucket("analytics")); + assert_eq!(budget.deadline(), first_deadline, "cleanup must not renew the admission budget"); +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_completed_recovery_cannot_exempt_a_new_transaction() { + let backend = TestCatalogObjectBackend::default(); + let publication = TableCommitLockPublication::new(&backend); + let result = publication_preparation(&publication, async { + publication.begin_table_bucket("analytics").await?; + publication_recovery(async { + begin_publication_mutation()?; + backend + .put_object("analytics", "recovery", b"recovered".to_vec(), TableCatalogPutPrecondition::IfAbsent) + .await?; + tokio::time::sleep(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + Ok(()) + }) + .await?; + backend + .put_object( + "analytics", + "new-transaction", + b"unexpected".to_vec(), + TableCatalogPutPrecondition::IfAbsent, + ) + .await + }) + .await; + assert!(is_preparation_timeout(&result)); + assert!(backend.object_exists("analytics", "recovery").await.unwrap()); + assert!(!backend.object_exists("analytics", "new-transaction").await.unwrap()); + assert!(!publication.holds_table_bucket("analytics")); +} + +// The durable-strong backing does not take part in the preparation deadline; it is retired separately, +// so only the object backing is exercised here. +#[tokio::test(start_paused = true)] +async fn publication_deadline_create_queue_wait_cannot_restart_after_admission() { + let (backend, store, _) = commit_fixture(false).await; + let key = default_table_bucket_publication_lock_path(); + let blocker = backend.acquire_read_lock("analytics", &key).await.unwrap(); + let attempts = backend.write_lock_acquisition_count("analytics", &key).await; + let candidate = registration_entry(&Namespace::parse("sales").unwrap(), "queued"); + let creating = { + let store = store.clone(); + tokio::spawn(async move { store.create_table(candidate).await }) + }; + while backend.write_lock_acquisition_count("analytics", &key).await == attempts { + tokio::task::yield_now().await; + } + tokio::time::advance(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + assert!(is_preparation_timeout(&creating.await.unwrap())); + drop(blocker); + assert!(store.load_table("analytics", "sales", "queued").await.unwrap().is_none()); + store + .create_table(registration_entry(&Namespace::parse("sales").unwrap(), "fresh")) + .await + .unwrap(); +} + +#[tokio::test(start_paused = true)] +async fn publication_deadline_active_rename_recovery_finishes_before_rejecting_new_registration() { + let (backend, store, namespace) = cache_fixture().await; + let source = store + .paths + .table_entry_path("analytics", &namespace, &IdentifierSegment::parse("orders").unwrap()); + backend.fail_next_put(RUSTFS_META_BUCKET, &source).await; + assert!( + store + .rename_table("analytics", "sales", "orders", "sales", "renamed_orders") + .await + .is_err() + ); + assert!( + store + .get_table_bucket("analytics") + .await + .unwrap() + .unwrap() + .active_rename_id + .is_some() + ); + let pause = backend.pause_next_put(RUSTFS_META_BUCKET, &source).await; + let registering = { + let store = store.clone(); + let candidate = registration_entry(&namespace, "after_recovery"); + tokio::spawn(async move { store.register_table(candidate).await }) + }; + pause.wait_started().await; + tokio::time::advance(publication_preparation_timeout() + StdDuration::from_millis(1)).await; + tokio::task::yield_now().await; + assert!( + !registering.is_finished(), + "in-flight recovery must not be cancelled by the preparation deadline" + ); + pause.release(); + assert!(is_preparation_timeout(®istering.await.unwrap())); + assert!( + store + .get_table_bucket("analytics") + .await + .unwrap() + .unwrap() + .active_rename_id + .is_none() + ); + assert!(store.load_table("analytics", "sales", "orders").await.unwrap().is_none()); + assert!( + store + .load_table("analytics", "sales", "renamed_orders") + .await + .unwrap() + .is_some() + ); + assert!( + store + .load_table("analytics", "sales", "after_recovery") + .await + .unwrap() + .is_none() + ); + store + .register_table(registration_entry(&namespace, "fresh_request")) + .await + .unwrap(); +} diff --git a/rustfs/src/table_catalog/store/registration_cache.rs b/rustfs/src/table_catalog/store/registration_cache.rs index 7b897dc42..0ff132f7c 100644 --- a/rustfs/src/table_catalog/store/registration_cache.rs +++ b/rustfs/src/table_catalog/store/registration_cache.rs @@ -171,10 +171,44 @@ impl ObjectTableCatalogStore { let index_prefix = self.paths.warehouse_index_entries_prefix(&entry.table_bucket); // The backend must preserve system-bucket committed-read semantics as // well as completeness; ordinary S3 LIST is insufficient for cache hits. - let mut tables = self - .backend - .list_objects_for_cleanup(self.catalog_bucket(), &namespace_prefix) - .await?; + let directory = self.table_directory_objects(&entry.table_bucket, None).await?; + let directory_epoch = directory.as_ref().map(|scan| scan.activation_id); + let mut tables = match directory { + Some(scan) => { + // The directory proves which keys to inspect, but cached owners + // still require the current GET-equivalent committed ETag. Do + // not replace this exact version probe with an ordinary stat. + stream::iter(scan.objects) + .map(|key| async move { + let page = self + .backend + .list_catalog_objects_page( + self.catalog_bucket(), + &key, + None, + NonZeroUsize::new(2).ok_or_else(|| { + TableCatalogStoreError::Internal("exact inventory page size must be positive".to_string()) + })?, + ) + .await?; + if page.is_truncated || page.objects.len() > 1 || page.objects.iter().any(|object| object.key != key) { + return Err(TableCatalogStoreError::Invalid( + "table directory target has an ambiguous authoritative inventory".to_string(), + )); + } + Ok(page.objects.into_iter().next()) + }) + .buffered(TABLE_REGISTRATION_READ_CONCURRENCY) + .try_filter_map(|object| async move { Ok(object) }) + .try_collect::>() + .await? + } + None => { + self.backend + .list_objects_for_cleanup(self.catalog_bucket(), &namespace_prefix) + .await? + } + }; let mut indexes = self .backend .list_objects_for_cleanup(self.catalog_bucket(), &index_prefix) @@ -252,6 +286,9 @@ impl ObjectTableCatalogStore { })?; table_warehouse_index_state_ready(&state, &entry.table_bucket)?; } + if let Some(epoch) = directory_epoch { + self.ensure_table_directory_epoch(&entry.table_bucket, epoch).await?; + } if !publication.holds_table_bucket(&entry.table_bucket) { return Err(TableCatalogStoreError::Unavailable("registration publication fence was lost".to_string())); } diff --git a/rustfs/src/table_catalog/store/strong.rs b/rustfs/src/table_catalog/store/strong.rs index 4412cad9a..591863ee3 100644 --- a/rustfs/src/table_catalog/store/strong.rs +++ b/rustfs/src/table_catalog/store/strong.rs @@ -1532,9 +1532,19 @@ where } } + #[cfg(test)] pub(in crate::table_catalog) async fn materialize_bucket_snapshot( &self, source: StrongTableCatalogBucketSnapshot, + ) -> TableCatalogStoreResult<(String, bool)> { + self.materialize_bucket_snapshot_with_fence(source, &TableCatalogObjectMutationFence::default()) + .await + } + + pub(in crate::table_catalog) async fn materialize_bucket_snapshot_with_fence( + &self, + source: StrongTableCatalogBucketSnapshot, + fence: &TableCatalogObjectMutationFence, ) -> TableCatalogStoreResult<(String, bool)> { let _write_guard = self.write_lock.lock().await; self.hydrate_state().await?; @@ -1562,7 +1572,8 @@ where postcondition, ) }; - self.finalize_snapshot_write(snapshot, precondition, postcondition).await?; + self.finalize_snapshot_write_with_fence(snapshot, precondition, postcondition, Some(fence)) + .await?; self.hydrate_state().await?; let state = self.state.lock().await; let snapshot_etag = state @@ -1572,11 +1583,27 @@ where Ok((snapshot_etag, true)) } + #[cfg(test)] pub(in crate::table_catalog) async fn remove_bucket_snapshot_if_unchanged( &self, table_bucket: &str, expected_fingerprint: &str, ) -> TableCatalogStoreResult<()> { + self.remove_bucket_snapshot_if_unchanged_with_fence( + table_bucket, + expected_fingerprint, + &TableCatalogObjectMutationFence::default(), + ) + .await + } + + pub(in crate::table_catalog) async fn remove_bucket_snapshot_if_unchanged_with_fence( + &self, + table_bucket: &str, + expected_fingerprint: &str, + fence: &TableCatalogObjectMutationFence, + ) -> TableCatalogStoreResult<()> { + fence.ensure_held()?; let _write_guard = self.write_lock.lock().await; self.hydrate_state().await?; let (snapshot, precondition, postcondition) = { @@ -1597,7 +1624,8 @@ where StrongSnapshotWritePostcondition::TableBucketAbsent(table_bucket.to_string()), ) }; - self.finalize_snapshot_write(snapshot, precondition, postcondition).await + self.finalize_snapshot_write_with_fence(snapshot, precondition, postcondition, Some(fence)) + .await } pub(super) async fn restore_absent_migration_snapshot_baseline(&self) -> TableCatalogStoreResult<()> { diff --git a/rustfs/src/table_catalog/test_support.rs b/rustfs/src/table_catalog/test_support.rs index 0d91be28c..9cc459de7 100644 --- a/rustfs/src/table_catalog/test_support.rs +++ b/rustfs/src/table_catalog/test_support.rs @@ -524,8 +524,16 @@ impl TableCommitPublication for BlockingObjectPublication { } } +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct TestCatalogWorkerContext { + pub(crate) access_key: Option, + pub(crate) operation: Option<&'static str>, + pub(crate) span_name: Option<&'static str>, +} + #[derive(Default)] pub(crate) struct TestCatalogObjectState { + pub(crate) observed_worker_contexts: Option>, pub(crate) objects: BTreeMap<(String, String), TestCatalogObjectRecord>, pub(crate) etagless_objects: BTreeSet<(String, String)>, pub(crate) fail_read_attempts: BTreeMap<(String, String), BTreeSet>, @@ -546,6 +554,10 @@ pub(crate) struct TestCatalogObjectState { pub(crate) read_calls: usize, pub(crate) read_bytes: usize, pub(crate) metadata_calls: usize, + pub(crate) pause_metadata: BTreeMap<(String, String), TestCatalogObjectPause>, + pub(crate) pause_after_metadata: BTreeMap<(String, String), TestCatalogObjectPause>, + pub(crate) fail_put_prefix: Option<(String, String)>, + pub(crate) corrupt_put_prefix: Option<(String, String)>, pub(crate) list_calls: usize, pub(crate) failed_inventory_prefixes: BTreeSet, pub(crate) inventory_overrides: BTreeMap>, @@ -554,6 +566,7 @@ pub(crate) struct TestCatalogObjectState { #[derive(Clone)] pub(crate) struct TestCatalogObjectRecord { + pub(crate) body_revision: Option, pub(crate) data: Vec, pub(crate) etag: String, pub(crate) mod_time: Option, @@ -574,9 +587,15 @@ impl TestCatalogObjectBackend { ) { let mut state = self.state.lock().await; let etag = state.next_etag(); - state - .objects - .insert((bucket.to_string(), object.to_string()), TestCatalogObjectRecord { data, etag, mod_time }); + state.objects.insert( + (bucket.to_string(), object.to_string()), + TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), + data, + etag, + mod_time, + }, + ); } pub(crate) async fn fail_put_attempt(&self, bucket: &str, object: &str, attempt: usize) { @@ -781,6 +800,23 @@ impl TestCatalogObjectBackend { if let Some(fence) = fence { fence.ensure_held()?; } + let (fail_prefix, corrupt_prefix) = { + let mut state = self.state.lock().await; + let matches = |key: &(String, String)| key.0 == bucket && object.starts_with(&key.1); + let fail = state.fail_put_prefix.as_ref().is_some_and(matches); + let corrupt = state.corrupt_put_prefix.as_ref().is_some_and(matches); + if fail { + state.fail_put_prefix.take(); + } + if corrupt { + state.corrupt_put_prefix.take(); + } + (fail, corrupt) + }; + if fail_prefix { + return Err(TableCatalogStoreError::Internal("injected candidate write failure".to_owned())); + } + let data = if corrupt_prefix { b"{}".to_vec() } else { data }; let mut fail_put_object_path = self.fail_put_object_path.lock().await; if fail_put_object_path.as_deref() == Some(object) { fail_put_object_path.take(); @@ -854,6 +890,7 @@ impl TestCatalogObjectBackend { state.objects.insert( key.clone(), TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), data, etag, mod_time: Some(OffsetDateTime::now_utc()), @@ -995,6 +1032,16 @@ impl TableCatalogObjectBackend for TestCatalogObjectBackend { let (attempt, pause_before) = { let mut state = self.state.lock().await; state.read_calls += 1; + if let Some(observations) = state.observed_worker_contexts.as_mut() { + observations.push(TestCatalogWorkerContext { + access_key: rustfs_keystone::KEYSTONE_CREDENTIALS + .try_with(|credentials| credentials.as_ref().map(|credentials| credentials.access_key.clone())) + .ok() + .flatten(), + operation: rustfs_io_metrics::s3_http_metrics::current_s3_http_operation_name(), + span_name: tracing::Span::current().metadata().map(|metadata| metadata.name()), + }); + } let attempt = { let attempts = state.read_attempts.entry(key.clone()).or_default(); *attempts += 1; @@ -1023,6 +1070,7 @@ impl TableCatalogObjectBackend for TestCatalogObjectBackend { let mut state = self.state.lock().await; let etagless = state.etagless_objects.contains(&key); let result = state.objects.get(&key).map(|record| TableCatalogObject { + body_revision: record.body_revision, data: record.data.clone(), etag: (!etagless).then(|| record.etag.clone()), mod_time: record.mod_time, @@ -1068,15 +1116,42 @@ impl TableCatalogObjectBackend for TestCatalogObjectBackend { } async fn object_metadata(&self, bucket: &str, object: &str) -> TableCatalogStoreResult> { + let pause = self + .state + .lock() + .await + .pause_metadata + .remove(&(bucket.to_string(), object.to_string())); + if let Some(pause) = pause { + pause.started.notify_one(); + pause.release.notified().await; + } let mut state = self.state.lock().await; state.metadata_calls += 1; let key = (bucket.to_string(), object.to_string()); let etagless = state.etagless_objects.contains(&key); - Ok(state.objects.get(&key).map(|record| TableCatalogObjectMetadata { + let metadata = state.objects.get(&key).map(|record| TableCatalogObjectMetadata { + body_revision: record.body_revision, etag: (!etagless).then(|| record.etag.clone()), mod_time: record.mod_time, size: record.data.len() as u64, - })) + }); + let pause = state.pause_after_metadata.remove(&key); + drop(state); + if let Some(pause) = pause { + pause.started.notify_one(); + pause.release.notified().await; + } + Ok(metadata) + } + + async fn object_metadata_unlocked( + &self, + bucket: &str, + object: &str, + ) -> TableCatalogStoreResult> { + // Match the ECStore stat path while publication already holds the object lock. + self.object_metadata(bucket, object).await } async fn object_exists(&self, bucket: &str, object: &str) -> TableCatalogStoreResult { @@ -1202,6 +1277,25 @@ impl TableCatalogObjectBackend for TestCatalogObjectBackend { .collect()) } + async fn list_catalog_objects_page( + &self, + bucket: &str, + prefix: &str, + start_after: Option<&str>, + limit: NonZeroUsize, + ) -> TableCatalogStoreResult { + // Only this in-memory fixture materializes the complete inventory. + // The production adapter uses a bounded authoritative storage page. + let mut objects = self.list_objects_for_cleanup(bucket, prefix).await?; + objects.sort_by(|left, right| left.key.cmp(&right.key)); + objects.retain(|object| start_after.is_none_or(|after| object.key.as_str() > after)); + // Production stops on a full page without consuming lookahead. Only + // a subsequent shorter (possibly empty) page proves EOF. + let is_truncated = objects.len() >= limit.get(); + objects.truncate(limit.get()); + Ok(TableCatalogCleanupPage { objects, is_truncated }) + } + async fn ensure_no_multipart_uploads(&self, _bucket: &str) -> TableCatalogStoreResult<()> { Ok(()) } @@ -1281,6 +1375,7 @@ impl TestCatalogObjectBackend { self.state.lock().await.objects.insert( (bucket.to_string(), object.to_string()), TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), data, etag, mod_time: None, @@ -1311,11 +1406,15 @@ impl TestCatalogObjectBackend { ) { let data = serde_json::to_vec(&value).expect("metadata JSON should serialize"); let etag = content_etag(&data); - self.state - .lock() - .await - .objects - .insert((bucket.to_string(), object.to_string()), TestCatalogObjectRecord { data, etag, mod_time }); + self.state.lock().await.objects.insert( + (bucket.to_string(), object.to_string()), + TestCatalogObjectRecord { + body_revision: Some(Uuid::new_v4()), + data, + etag, + mod_time, + }, + ); } pub(crate) async fn write_lock_is_held(&self, bucket: &str, object: &str) -> bool { diff --git a/rustfs/src/table_catalog/tests.rs b/rustfs/src/table_catalog/tests.rs index 5d9895cef..73f361385 100644 --- a/rustfs/src/table_catalog/tests.rs +++ b/rustfs/src/table_catalog/tests.rs @@ -32,16 +32,17 @@ fn table_data_plane_index_miss_scan_rejects_catalogs_above_its_object_limit() { "catalog/table-2/table-entry.json".to_string(), ]; + // Only table entries count toward the limit; the namespace object is free. assert_eq!( - bounded_table_entry_objects_for_data_plane_scan(objects.clone(), false, 3).unwrap(), + bounded_table_entry_objects_for_data_plane_scan(objects.clone(), false, 2).unwrap(), vec![ "catalog/table-1/table-entry.json".to_string(), "catalog/table-2/table-entry.json".to_string(), ] ); assert_matches!( - bounded_table_entry_objects_for_data_plane_scan(objects, false, 2), - Err(TableCatalogStoreError::Unavailable(message)) if message.contains("2-catalog-object safety limit") + bounded_table_entry_objects_for_data_plane_scan(objects, false, 1), + Err(TableCatalogStoreError::Unavailable(message)) if message.contains("1-catalog-object safety limit") ); assert_matches!( bounded_table_entry_objects_for_data_plane_scan(vec!["catalog/table-entry.json".to_string()], true, 1), @@ -4874,6 +4875,7 @@ async fn maintenance_test_object_versions(backend: &TestCatalogObjectBackend) -> ( key.clone(), TableCatalogObject { + body_revision: object.body_revision, data: object.data.clone(), etag: Some(object.etag.clone()), mod_time: object.mod_time, @@ -7047,6 +7049,7 @@ async fn table_metadata_data_plane_resource_resolves_only_current_metadata() { seed_table_for_metadata_maintenance(&store, bucket, &namespace, &table, current.clone()).await; backend.reset_call_counts().await; + backend.state.lock().await.read_attempts.clear(); let resource = table_metadata_data_plane_resource_for_object(&store, bucket, ¤t) .await @@ -7055,7 +7058,20 @@ async fn table_metadata_data_plane_resource_resolves_only_current_metadata() { assert_eq!(resource.namespace, "sales"); assert_eq!(resource.table, "orders"); assert_eq!(backend.list_call_count().await, 1); - assert_eq!(backend.read_call_count().await, 2); + let paths = TableCatalogObjectPaths::default(); + // One bucket generation read, one directory mode probe, and one authoritative + // table entry. The legacy fixture must never load a candidate object. + assert_eq!( + backend.state.lock().await.read_attempts, + [ + paths.table_bucket_entry_path(bucket), + paths.backing_migration_fence_path(bucket), + paths.table_entry_path(bucket, &namespace, &table), + ] + .into_iter() + .map(|key| ((RUSTFS_META_BUCKET.to_owned(), key), 1)) + .collect() + ); assert_eq!(backend.metadata_call_count().await, 1); let historical = default_table_metadata_file_path(&namespace, &table, "00000.metadata.json"); @@ -8125,10 +8141,26 @@ async fn operator_backfill_reconciles_a_missing_ready_index_above_the_data_plane .delete_object(RUSTFS_META_BUCKET, &index_path) .await .expect("warehouse index entry should be removed"); - let catalog_prefix = store.paths.namespace_entries_prefix(bucket); + // Only table entries count toward the data-plane limit; seed enough real + // entries that, with the table under test, the catalog exceeds it. + let noise_namespace = Namespace::parse("noise").expect("namespace should parse"); for index in 0..TABLE_DATA_PLANE_INDEX_MISS_SCAN_MAX_CATALOG_OBJECTS { + let noise_table = IdentifierSegment::parse(format!("noise-{index}")).expect("table should parse"); + let mut entry = test_table_entry( + bucket, + &noise_namespace, + &noise_table, + default_table_metadata_file_path(&noise_namespace, &noise_table, "00001.metadata.json"), + ); + entry.table_id = format!("noise-{index}"); + entry.table_uuid = format!("noise-uuid-{index}"); + entry.warehouse_location = format!("s3://{bucket}/tables/noise-{index}"); backend - .seed_object(RUSTFS_META_BUCKET, &format!("{catalog_prefix}noise-{index}.json"), b"{}".to_vec()) + .seed_object( + RUSTFS_META_BUCKET, + &store.paths.table_entry_path(bucket, &noise_namespace, &noise_table), + serde_json::to_vec(&entry).expect("entry should encode"), + ) .await; } @@ -13031,6 +13063,207 @@ async fn maintenance_reachability_fails_closed_when_retained_metadata_is_unreada ); } +#[tokio::test] +async fn object_maintenance_refuses_roots_shared_after_cross_namespace_rename() { + let backend = TestCatalogObjectBackend::default(); + let store = ObjectTableCatalogStore::new(backend.clone()); + let bucket = "analytics"; + let namespace = Namespace::parse("sales").unwrap(); + let destination = Namespace::parse("private").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let original_metadata = default_table_metadata_file_path(&namespace, &table, "00001.metadata.json"); + let replacement_metadata = default_table_metadata_file_path(&namespace, &table, "00002.metadata.json"); + seed_table_for_metadata_maintenance(&store, bucket, &namespace, &table, original_metadata.clone()).await; + backend + .seed_object(bucket, &original_metadata, br#"{"metadata-log":[]}"#.to_vec()) + .await; + store + .create_namespace(test_namespace_entry(bucket, &destination)) + .await + .unwrap(); + store + .rename_table(bucket, "sales", "orders", "private", "orders") + .await + .unwrap(); + let mut replacement = test_table_entry(bucket, &namespace, &table, replacement_metadata.clone()); + replacement.table_id = "replacement-id".to_string(); + replacement.table_uuid = "replacement-uuid".to_string(); + replacement.warehouse_location = format!("s3://{bucket}/tables/replacement-id"); + backend + .seed_object(bucket, &replacement_metadata, br#"{"metadata-log":[]}"#.to_vec()) + .await; + store.create_table(replacement).await.unwrap(); + assert_matches!(store.plan_table_metadata_maintenance(bucket, "sales", "orders", 0).await, + Err(TableCatalogStoreError::Conflict(message)) if message.contains("exclusive metadata")); + assert_matches!(store.delete_table_metadata_maintenance_candidates(bucket, "sales", "orders", 0).await, + Err(TableCatalogStoreError::Conflict(message)) if message.contains("exclusive metadata")); + assert!(backend.object_exists(bucket, &original_metadata).await.unwrap()); + assert_eq!( + store + .load_table(bucket, "private", "orders") + .await + .unwrap() + .unwrap() + .metadata_location, + original_metadata + ); + assert!(backend.state.lock().await.delete_attempts.is_empty()); +} + +#[tokio::test] +async fn object_maintenance_rechecks_root_ownership_under_publication_fence() { + let backend = TestCatalogObjectBackend::default(); + let store = ObjectTableCatalogStore::new(backend.clone()); + let bucket = "analytics"; + let namespace = Namespace::parse("sales").unwrap(); + let other_namespace = Namespace::parse("private").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let current = default_table_metadata_file_path(&namespace, &table, "00002.metadata.json"); + let orphan = default_table_metadata_file_path(&namespace, &table, "00001.metadata.json"); + seed_table_for_metadata_maintenance(&store, bucket, &namespace, &table, current.clone()).await; + backend + .seed_object(bucket, ¤t, br#"{"metadata-log":[]}"#.to_vec()) + .await; + backend.seed_object(bucket, &orphan, br#"{"metadata-log":[]}"#.to_vec()).await; + let report = store + .plan_table_metadata_maintenance(bucket, "sales", "orders", 0) + .await + .unwrap(); + assert_eq!(report.deletable_metadata_locations, vec![orphan.clone()]); + store + .create_namespace(test_namespace_entry(bucket, &other_namespace)) + .await + .unwrap(); + let mut other = test_table_entry(bucket, &other_namespace, &table, orphan.clone()); + other.table_id = "other-id".to_string(); + other.table_uuid = "other-uuid".to_string(); + other.warehouse_location = format!("s3://{bucket}/tables/other-id"); + store.create_table(other.clone()).await.unwrap(); + assert_matches!(store.delete_table_metadata_maintenance_report(bucket, "sales", "orders", report.clone()).await, + Err(TableCatalogStoreError::Conflict(message)) if message.contains("exclusive metadata")); + assert!(backend.object_exists(bucket, &orphan).await.unwrap()); + store.drop_table(bucket, "private", "orders").await.unwrap(); + + let pause = backend.pause_next_delete(bucket, &orphan).await; + let deleting_store = store.clone(); + let deleting = tokio::spawn(async move { + deleting_store + .delete_table_metadata_maintenance_report(bucket, "sales", "orders", report) + .await + }); + tokio::time::timeout(TABLE_CATALOG_TEST_TIMEOUT, pause.wait_started()) + .await + .unwrap(); + let publication_lock = default_table_bucket_publication_lock_path(); + let attempts = backend.write_lock_acquisition_count(bucket, &publication_lock).await; + let creating_store = store.clone(); + let creating = tokio::spawn(async move { creating_store.create_table(other).await }); + tokio::time::timeout(TABLE_CATALOG_TEST_TIMEOUT, async { + while backend.write_lock_acquisition_count(bucket, &publication_lock).await == attempts { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + assert!(!creating.is_finished(), "new owner cannot publish between root validation and deletion"); + creating.abort(); + let _ = creating.await; + pause.release(); + let deleted = tokio::time::timeout(TABLE_CATALOG_TEST_TIMEOUT, deleting) + .await + .unwrap() + .unwrap() + .unwrap(); + assert_eq!(deleted.deletable_metadata_locations, vec![orphan.clone()]); + assert!(!backend.object_exists(bucket, &orphan).await.unwrap()); + assert!(backend.object_exists(bucket, ¤t).await.unwrap()); +} + +#[tokio::test] +async fn object_maintenance_rejects_a_report_from_a_replaced_table_incarnation() { + let backend = TestCatalogObjectBackend::default(); + let store = ObjectTableCatalogStore::new(backend.clone()); + let bucket = "analytics"; + let namespace = Namespace::parse("sales").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let current = default_table_metadata_file_path(&namespace, &table, "00002.metadata.json"); + let orphan = default_table_metadata_file_path(&namespace, &table, "00001.metadata.json"); + seed_table_for_metadata_maintenance(&store, bucket, &namespace, &table, current.clone()).await; + backend + .seed_object(bucket, ¤t, br#"{"metadata-log":[]}"#.to_vec()) + .await; + backend.seed_object(bucket, &orphan, br#"{"metadata-log":[]}"#.to_vec()).await; + let report = store + .plan_table_metadata_maintenance(bucket, "sales", "orders", 0) + .await + .unwrap(); + store.drop_table(bucket, "sales", "orders").await.unwrap(); + let mut replacement = test_table_entry(bucket, &namespace, &table, current.clone()); + replacement.table_id = "new-incarnation".to_string(); + replacement.table_uuid = "new-uuid".to_string(); + replacement.warehouse_location = format!("s3://{bucket}/tables/new-incarnation"); + store.create_table(replacement).await.unwrap(); + assert_matches!(store.delete_table_metadata_maintenance_report(bucket, "sales", "orders", report).await, + Err(TableCatalogStoreError::Invalid(message)) if message.contains("identity does not match")); + assert!(backend.object_exists(bucket, ¤t).await.unwrap()); + assert!(backend.object_exists(bucket, &orphan).await.unwrap()); +} + +#[tokio::test] +async fn maintenance_orphan_planning_and_deletion_never_read_object_bodies() { + let backend = TestCatalogObjectBackend::default(); + let store = ObjectTableCatalogStore::new(backend.clone()); + let bucket = "analytics"; + let namespace = Namespace::parse("sales").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let current = default_table_metadata_file_path(&namespace, &table, "00002.metadata.json"); + seed_table_for_metadata_maintenance(&store, bucket, &namespace, &table, current.clone()).await; + backend + .seed_object(bucket, ¤t, br#"{"metadata-log":[]}"#.to_vec()) + .await; + let root = format!("{}{}/", default_table_root_prefix(&namespace), table.as_str()); + let old = format!("{root}data/old.parquet"); + let fresh = format!("{root}delete/fresh.parquet"); + let unknown = format!("{root}metadata/unknown.avro"); + backend.seed_object(bucket, &old, b"body must not be read".to_vec()).await; + backend + .seed_object_with_mod_time(bucket, &fresh, vec![1], Some(OffsetDateTime::now_utc())) + .await; + backend.seed_object_with_mod_time(bucket, &unknown, vec![2], None).await; + for object in [&old, &fresh, &unknown] { + backend.fail_next_read(bucket, object).await; + } + let report = store + .plan_table_metadata_maintenance(bucket, "sales", "orders", 0) + .await + .expect("stat-only dry run"); + assert_eq!(report.deletable_object_locations, vec![old.clone()]); + assert_eq!( + object_cleanup_report(&report, &fresh).state, + TableMetadataMaintenanceObjectState::PendingSafetyWindow + ); + assert_eq!( + object_cleanup_report(&report, &unknown).state, + TableMetadataMaintenanceObjectState::PendingSafetyWindow + ); + let deleted = store + .delete_table_metadata_maintenance_candidates(bucket, "sales", "orders", 0) + .await + .expect("stat-only deletion recheck"); + assert_eq!(deleted.deletable_object_locations, vec![old.clone()]); + assert!(!backend.object_exists(bucket, &old).await.unwrap()); + assert!(backend.object_exists(bucket, &fresh).await.unwrap()); + assert!(backend.object_exists(bucket, &unknown).await.unwrap()); + let state = backend.state.lock().await; + for object in [&old, &fresh, &unknown] { + assert_eq!( + state.read_attempts.get(&(bucket.to_string(), object.to_string())), + None, + "orphan body must never be fetched" + ); + } +} + #[tokio::test] async fn maintenance_dry_run_reports_unreachable_manifest_data_and_delete_candidates() { let backend = TestCatalogObjectBackend::default(); @@ -13570,6 +13803,7 @@ async fn compaction_plan_reports_manifest_reader_gap_without_rewrite_candidates( } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_plan_reports_row_level_delete_files_without_rewrite_candidates() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -13744,6 +13978,7 @@ async fn compaction_plan_requires_current_snapshot_metadata() { } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_commit_rewrites_small_data_files_and_advances_pointer() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -13918,6 +14153,7 @@ async fn compaction_commit_rewrites_small_data_files_and_advances_pointer() { } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_commit_keeps_partition_rewrite_groups_isolated() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -14099,6 +14335,7 @@ async fn compaction_commit_keeps_partition_rewrite_groups_isolated() { } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_commit_preserves_sort_order_and_keeps_groups_isolated() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -14238,6 +14475,7 @@ async fn compaction_commit_preserves_sort_order_and_keeps_groups_isolated() { } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_commit_rejects_schema_mismatch_without_advancing_pointer() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -14313,6 +14551,7 @@ async fn compaction_commit_rejects_schema_mismatch_without_advancing_pointer() { } #[tokio::test] +#[serial_test::serial(table_compaction)] async fn compaction_commit_rejects_deleted_manifest_entries_without_advancing_pointer() { let backend = TestCatalogObjectBackend::default(); let store = ObjectTableCatalogStore::new(backend.clone()); @@ -18514,7 +18753,8 @@ async fn strong_catalog_snapshot_rejects_unknown_fields_and_versions() { .as_object_mut() .expect("snapshot should be an object") .remove("future-field"); - value["version"] = serde_json::Value::from(STRONG_TABLE_CATALOG_SNAPSHOT_VERSION.saturating_add(1)); + // Only snapshot versions up to the current one are readable. + value["version"] = serde_json::Value::from(4); backend .seed_object( RUSTFS_META_BUCKET, @@ -22089,58 +22329,108 @@ fn table_metadata_pointer_json_round_trips() { #[test] fn object_mutation_entrypoints_call_reserved_prefix_guard() { - let source = [ - include_str!("../app/object/mod.rs"), - include_str!("../app/object/shared.rs"), - include_str!("../app/object/extract.rs"), - include_str!("../app/object/put.rs"), - include_str!("../app/object/copy.rs"), - include_str!("../app/object/delete.rs"), - include_str!("../app/object/head.rs"), - include_str!("../app/object/restore.rs"), - ] - .concat(); - let delete_module = include_str!("../app/object/delete.rs"); - let delete_object = delete_module - .split_once("pub async fn execute_delete_object") - .map(|(_, remainder)| { - remainder - .split_once("\nmod tests") - .map_or(remainder, |(entrypoints, _)| entrypoints) - }) - .expect("delete object entrypoints should remain in app/object/delete.rs"); - - for expected in [ - "validate_new_object_key(&key, request_method_name)?;\n validate_table_catalog_object_mutation(&bucket, &key).await?;", - "if let Err(err) = validate_table_catalog_object_mutation(&bucket, &obj_id.key).await", - "validate_table_catalog_object_mutation(&bucket, &object).await?;", - "validate_new_object_key(&key, \"PUT\")?;\n validate_table_catalog_object_mutation(&bucket, &key).await?;", - "extract_try!(validate_table_catalog_object_mutation(&bucket, &fpath).await);", - ] { - assert!(source.contains(expected), "missing object mutation guard: {expected}"); + // Scope every assertion to one implementation method: a guard in a sibling + // method (or its tests) must not satisfy another entrypoint's contract. + fn method<'a>(source: &'a str, signature: &str) -> &'a str { + source + .split_once(signature) + .unwrap_or_else(|| panic!("missing object mutation method: {signature}")) + .1 + .split_once("\n }\n") + .unwrap_or_else(|| panic!("missing method boundary: {signature}")) + .0 } - let copy_object = include_str!("../app/object/copy.rs") - .split_once("async fn execute_copy_object_body(") - .map(|(_, body)| body.split_once("\n#[cfg(test)]").map_or(body, |(entrypoint, _)| entrypoint)) - .expect("copy object entrypoint should remain in app/object/copy.rs"); - let copy_key = copy_object - .find("validate_new_object_key(&key, \"COPY (dest)\")?;") - .expect("copy destination key validation"); - let copy_guard = copy_object - .find("validate_table_catalog_object_mutation(&bucket, &key).await?;") - .expect("copy destination catalog guard"); - let copy_store = copy_object - .find("let Some(store) = self.object_store()") - .expect("copy object store resolution"); - assert!( - copy_key < copy_guard && copy_guard < copy_store, - "copy must guard the destination before store access" - ); - assert!( - delete_object.contains("validate_object_key(&key, \"DELETE\")?;") - && delete_object.contains("validate_table_catalog_object_mutation(&bucket, &key).await?;"), - "delete object entrypoint must validate the object key and reserved catalog prefix" + fn assert_ordered(body: &str, expected: &[&str]) { + let mut remainder = body; + for statement in expected { + remainder = remainder + .split_once(statement) + .unwrap_or_else(|| panic!("missing or out-of-order object mutation guard: {statement}")) + .1; + } + } + + let put = include_str!("../app/object/put.rs"); + let copy = include_str!("../app/object/copy.rs"); + let delete = include_str!("../app/object/delete.rs"); + let extract = include_str!("../app/object/extract.rs"); + let restore = include_str!("../app/object/restore.rs"); + + for (source, signature, delegate) in [ + (put, "fn execute_put_object(", "self.execute_put_object_boxed(_fs, req)"), + (put, "fn execute_put_object_boxed<'a>(", "self.execute_put_object_inner(_fs, req)"), + (put, "fn execute_put_object_inner(", "self.execute_put_object_extract(req)"), + (copy, "fn execute_copy_object(", "self.execute_copy_object_body_boxed(req)"), + (copy, "fn execute_copy_object_body_boxed(", "self.execute_copy_object_body(req)"), + (delete, "fn execute_delete_object(", "owner.execute_delete_object_owned(req)"), + (delete, "fn execute_delete_objects(", "owner.execute_delete_objects_owned(req)"), + (extract, "fn execute_put_object_extract(", "self.execute_put_object_extract_boxed(req)"), + ( + extract, + "fn execute_put_object_extract_boxed(", + "self.execute_put_object_extract_inner(req)", + ), + ] { + assert!( + method(source, signature).contains(delegate), + "{signature} must call its guarded implementation" + ); + } + + let key_guard = "validate_table_catalog_object_mutation(&bucket, &key).await?;"; + let store_resolution = "let Some(store) = self.object_store()"; + for (source, signature, key_validation, mutation_boundary) in [ + ( + put, + "fn execute_put_object_inner(", + "validate_new_object_key(&key, request_method_name)?;", + "self.put_object_core(write, body, start_time).await?;", + ), + ( + copy, + "fn execute_copy_object_body(", + "validate_new_object_key(&key, \"COPY (dest)\")?;", + store_resolution, + ), + ( + delete, + "fn execute_delete_object_owned(", + "validate_object_key(&key, \"DELETE\")?;", + store_resolution, + ), + ( + extract, + "fn execute_put_object_extract_inner(", + "validate_new_object_key(&key, \"PUT\")?;", + store_resolution, + ), + ] { + assert_ordered(method(source, signature), &[key_validation, key_guard, mutation_boundary]); + } + assert_ordered( + method(restore, "fn execute_restore_object("), + &[ + "validate_table_catalog_object_mutation(&bucket, &object).await?;", + store_resolution, + ], + ); + assert_ordered( + method(delete, "fn execute_delete_objects_owned("), + &[ + "validate_object_key(&obj_id.key, \"DeleteObjects\")", + "if let Err(err) = validate_table_catalog_object_mutation(&bucket, &obj_id.key).await {", + "continue;\n }", + "authorized_deletes.push(AuthorizedDelete { idx, object });", + ], + ); + assert_ordered( + method(extract, "fn execute_put_object_extract_inner("), + &[ + "validate_extract_member_key(&fpath, extract_limits)", + "extract_try!(validate_table_catalog_object_mutation(&bucket, &fpath).await);", + "batch.push(ExtractPreparedMember {", + ], ); } @@ -25256,3 +25546,82 @@ async fn disable_empty_table_bucket_failure_keeps_active_catalog() { restarted.disable_empty_table_bucket(test_bucket_entry(bucket)).await.unwrap(); } } + +#[tokio::test] +async fn compaction_planner_stats_data_files_without_reading_their_bodies() { + let backend = TestCatalogObjectBackend::default(); + let bucket = "analytics"; + let namespace = Namespace::parse("sales").unwrap(); + let table = IdentifierSegment::parse("orders").unwrap(); + let metadata_dir = default_table_metadata_dir_path(&namespace, &table); + let current = default_table_metadata_file_path(&namespace, &table, "00004.metadata.json"); + let entry = test_table_entry(bucket, &namespace, &table, current); + let data_dir = default_table_data_dir_path(&namespace, &table); + let files = [format!("{data_dir}/left.parquet"), format!("{data_dir}/right.parquet")]; + let manifest = format!("{metadata_dir}/manifest-20.avro"); + let manifest_list = format!("{metadata_dir}/snap-20.avro"); + let manifest_bytes = manifest_avro_bytes(&[(&files[0], 0), (&files[1], 0)]); + backend + .seed_object(bucket, &manifest_list, manifest_list_avro_bytes(&[(&manifest, manifest_bytes.len())])) + .await; + backend.seed_object(bucket, &manifest, manifest_bytes).await; + for file in &files { + backend.seed_object(bucket, file, vec![0; 8192]).await; + backend.fail_next_read(bucket, file).await; + } + let metadata = serde_json::json!({ + "current-snapshot-id": 20, + "snapshots": [{"snapshot-id": 20, "manifest-list": manifest_list}], + "refs": {"main": {"snapshot-id": 20, "type": "branch"}} + }); + backend.reset_call_counts().await; + let report = table_compaction_planning_report( + &backend, + bucket, + &namespace, + &table, + &entry, + &metadata, + TableCompactionPlanningConfig { + target_file_size_bytes: 65536, + small_file_threshold_bytes: 16384, + min_input_files: 2, + max_rewrite_bytes_per_job: 65536, + }, + ) + .await + .unwrap(); + assert_eq!(report.status, TableCompactionPlanningStatus::RewriteCandidates); + assert_eq!(report.candidate_file_count, 2); + assert_eq!(backend.metadata_call_count().await, 2); + assert_eq!(backend.read_call_count().await, 2, "only the manifest list and manifest may be read"); +} + +#[tokio::test] +async fn compaction_backend_read_limit_is_enforced_on_real_storage() { + use crate::storage_api::table::test_support::{ + BucketOperations as _, MakeBucketOptions, init_bucket_metadata_sys, isolated_store_over_temp_disks, + }; + let (_disks, store) = Box::pin(isolated_store_over_temp_disks()).await; + init_bucket_metadata_sys(store.clone(), Vec::new()).await; + store + .make_bucket("bounded-compaction", &MakeBucketOptions::default()) + .await + .unwrap(); + let backend = EcStoreTableCatalogObjectBackend::new_with_strong_runtime(store, StrongTableCatalogRuntime::default()); + backend + .put_object("bounded-compaction", "data.parquet", vec![7; 8192], TableCatalogPutPrecondition::IfAbsent) + .await + .unwrap(); + let error = backend + .read_object_limited("bounded-compaction", "data.parquet", 1024) + .await + .expect_err("oversized stored input"); + assert!(matches!(error, TableCatalogStoreError::Invalid(_))); + let data = backend + .read_object_limited("bounded-compaction", "data.parquet", 8192) + .await + .unwrap() + .unwrap(); + assert_eq!(data.data, vec![7; 8192]); +}