laserninja opened a new pull request, #12195:
URL: https://github.com/apache/gravitino/pull/12195
### What changes were proposed in this pull request?
Adds a design document for the Iceberg REST catalog `commitTransaction`
endpoint
(`POST /v1/{prefix}/transactions/commit`):
`design-docs/iceberg-multi-table-atomic-commit-design.md`.
No production code changes.
The document proposes **backend-capability dispatch** rather than one
uniform best-effort
implementation:
| Backend | Strategy |
|---------|----------|
| `JDBC` | Batched CAS on a single `Connection` in one DB transaction |
| `REST` | Forward the client's request body unmodified to the upstream
catalog |
| `MEMORY` | Single lock over the backing map |
| `HIVE` | Not supported, returns `501`; HMS Thrift has no multi-table
transactional alter |
| `CUSTOM` | Not supported unless the user's `Catalog` opts in |
`Endpoint.V1_COMMIT_TRANSACTION` is advertised in `/v1/config` if and only
if the resolved backend
can commit atomically, following the existing precedent where
`getEndpoints(supportsViewOperations)`
already varies the advertised set by catalog capability. This means a client
never sees the endpoint
advertised unless the guarantee behind it is real.
The document also covers:
- Per-table authorization, closing the privilege-escalation gap raised in
review. The
`@AuthorizationExpression` interceptor resolves entities from path
parameters, but this endpoint
carries its tables in the request body, so authorization is done
programmatically via
`MetadataAuthzHelper.checkAccess` against the same expression
`updateTable` uses. The expression is
extracted into a shared constant so the two paths cannot drift.
- Rejected alternatives, including the two-phase `validate-then-commit`
approach from #10675 and a
Polaris-style Gravitino-owned pointer store, with concrete reasons.
- Orphaned-metadata semantics on rollback, and the lost-response case a
client cannot detect.
- A task breakdown across five phases, one task per follow-up issue/PR.
### Why are the changes needed?
`POST /v1/{prefix}/transactions/commit` is part of the Iceberg REST Catalog
spec and Gravitino does
not implement it, so engines cannot perform cross-table atomic operations
against Gravitino.
The first implementation attempt (#10675) was found not to deliver atomic
semantics: the commit phase
remained N independent compare-and-swap operations, so a failure partway
through left a partial state.
Reviewers concluded that multi-table atomicity needs a design document
before implementation, and
asked for one:
- https://github.com/apache/gravitino/pull/10675#issuecomment-4205268980
- https://github.com/apache/gravitino/pull/10675#issuecomment-4277155901
- https://github.com/apache/gravitino/pull/10675#issuecomment-5020220127
The design work also answers the question raised in
https://github.com/apache/gravitino/pull/10675#issuecomment-5020501240 about
which backends can
achieve true CAS: the capability differs per backend, and that asymmetry is
what drives the proposed
design.
Fix: #10674
### Does this PR introduce _any_ user-facing change?
No. Documentation only.
The design it describes would introduce a new REST endpoint and a new entry
in the `/v1/config`
`endpoints` list, but no code is changed in this PR.
### How was this patch tested?
N/A, documentation only.
Claims about backend behaviour in the document were verified against the
Iceberg 1.11.0 artifacts
this repo depends on rather than from memory, including `JdbcUtil`'s commit
SQL and discriminator
column, `HiveTableOperations.doCommit`'s per-table `alter_table` path,
`TableCommit.create`'s
requirement derivation, and the visibility of
`ResourcePaths.commitTransaction()` and
`RESTClient.post`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
