Skip to content

Delta commit coordination splits _delta_log across two roots: UC facade uses legacy tables/{id} while arco-api... #361

Description

@ethan-tyler

Severity: High · Category: bug

Affected code

  • crates/arco-uc/src/routes/delta_commits.rs:786
  • crates/arco-api/src/routes/delta.rs:181-182
  • crates/arco-api/src/routes/delta.rs:223
  • crates/arco-delta/src/coordinator.rs:62
  • crates/arco-delta/src/coordinator.rs:322-341
  • crates/arco-core/src/flow_paths.rs:325
  • crates/arco-core/src/flow_paths.rs:351
  • crates/arco-uc/src/routes/delta_commits.rs:108-110

Problem

The UC facade builds its coordinator with let coordinator = DeltaCommitCoordinator::new(storage.clone(), table_id); (delta_commits.rs:786), and DeltaCommitCoordinator::new hardcodes DeltaPaths::legacy(table_id) (crates/arco-delta/src/coordinator.rs:62 -> crates/arco-core/src/flow_paths.rs:265), i.e. table_root = "tables/{table_id}". The arco-api Delta route for the same table builds its coordinator from the table's registered location: resolve_delta_table_paths(...) -> DeltaPaths::from_table_location(table_id, table.location.as_deref(), tenant, workspace) (crates/arco-api/src/routes/delta.rs:181-182 and 223), which resolves e.g. gs://bucket/warehouse/orders to table_root = "warehouse/orders" (crates/arco-core/src/flow_paths.rs:372-395).

Only delta_log_json() depends on table_root (flow_paths.rs:351). Every other coordinator object is keyed by table_id alone: coordinator_state() = delta/coordinator/{table_id}.json (flow_paths.rs:325), staging, and idempotency markers. So both endpoints share ONE latest_version/inflight slot while writing commit files into two different directories. Both routers are mounted in the same arco-api process (crates/arco-api/src/routes/mod.rs:34 and crates/arco-api/src/server.rs:834-838).

The UC route also requires a registered location and requires it to equal the client table_uri (require_catalog_managed_delta_table, delta_commits.rs:167-177), and post_tables takes storage_location as a required client-supplied field (crates/arco-uc/src/routes/tables.rs:365-366) — so the location is essentially never literally tables/{uuid}. The UC list path is hardcoded to the legacy root too: fn delta_log_prefix(table_id) -> format!("tables/{table_id}/_delta_log/") (delta_commits.rs:108-110).

Why it matters

A Delta log must be a contiguous version sequence in one directory. Here a single monotonically increasing version counter is spread over two disjoint directories, so neither directory is a valid Delta log. Readers pointed at the table's real location silently miss commits (data loss), and the coordinator's crash-recovery path actively writes commits into the wrong directory.

Failure scenario

Table T (Delta, location gs://bucket/warehouse/orders) in tenant/workspace W.
(a) Split log: a writer commits v1 through POST /api/v1/delta/tables/{T}/commits -> file lands at warehouse/orders/_delta_log/00000000000000000001.json, coordinator latest_version=1. A second writer commits through the UC facade POST /delta/preview/commits -> reserve_or_resume_inflight sees latest_version=1, reserves 2, and writes tables/{T}/_delta_log/00000000000000000002.json. Spark reading gs://bucket/warehouse/orders never sees v2; the UC GET /delta/preview/commits reports latest_table_version=2 from shared coordinator state but returns an empty commits array because load_delta_log_objects lists only the legacy prefix.
(b) Takeover corrupts the log: an arco-api commit reserves version N (inflight persisted to delta/coordinator/{T}.json) and stalls past inflight_ttl (default 300s, coordinator.rs:33). A UC delta request for T then runs recover_inflight (coordinator.rs:309-341); it checks self.paths.delta_log_json(inflight.version) — the LEGACY path — finds nothing, and materializes the staged payload at tables/{T}/_delta_log/{N}.json, then CASes latest_version = N. Version N now exists only under the legacy root; warehouse/orders/_delta_log/ permanently has a hole at N, and the next arco-api commit writes N+1 there, producing an unreadable log.

Suggested fix

Make the Delta log root part of the coordinator's identity, not per-caller. Either (1) have the UC route resolve paths the same way arco-api does — it already loads the catalog table in require_catalog_managed_delta_table, so pass DeltaPaths::from_table_location(table_id, table.location.as_deref(), &ctx.tenant, &ctx.workspace) into DeltaCommitCoordinator::with_paths(...) and derive delta_log_prefix from paths.table_root() instead of the hardcoded tables/{table_id}/_delta_log/; or (2) persist table_root inside DeltaCoordinatorState and have commit/recover_inflight reject or adopt a coordinator whose table_root differs from the persisted one, so a mismatched caller fails closed instead of writing to a second directory. Also fix crates/arco-uc/tests/delta_catalog_binding.rs:190, which currently bakes in the legacy prefix and therefore cannot catch this.

Verification notes

Existing mitigations found (do not fully close it): None that neutralize the defect. The shared CAS is on delta/coordinator/{table_id}.json itself, which is the shared resource being split. validate_staged_path (coordinator.rs:621-629) is table_id-scoped and does not stop cross-root inflight takeover. No test asserts agreement between the two roots. The only real limiter is configuration: UnityCatalogApiConfig::default() has enabled=false (crates/arco-api/src/config.rs:470-477) and no infra/terraform or cloudbuild file sets ARCO_UNITY_CATALOG_ENABLED, so the UC facade is not mounted in the deployed posture today.

Caveat / scope: Could not confirm any production/UAT environment that sets ARCO_UNITY_CATALOG_ENABLED=true; the split is therefore latent rather than live at this commit. I also did not verify how an external Delta engine consumes the UC GET response (it returns file_name but no path/URI), so the precise reader-visible symptom for a UC-only deployment is inferred from the storage layout rather than from an engine integration test.

Found during a staff-level audit of origin/main @ c3c0867. Mechanism confirmed by an independent adversarial verifier.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingseverity: mediumReal but bounded impact

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions