SteNicholas opened a new issue, #470: URL: https://github.com/apache/paimon-cpp/issues/470
## Search before asking - [x] I searched in the [issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar. ## Motivation Java Paimon added table commit leases to the REST catalog (apache/paimon#10491, ownership clarified in apache/paimon#10588). Two options control them, both default `false`: - `rest.commit.lock-enabled`: persisted table property. It turns on server-side lease enforcement for the table. - `rest.commit.acquire-lock`: writer option, usually set as a dynamic option. With it, the writer acquires a lease before each publication attempt and renews it until the attempt ends. It requires `rest.commit.lock-enabled` on the table. The lease is an admission lock bound to the table UUID, the branch and the exact `commitUser`. The server rejects a publication whose `snapshot.commitUser` does not own an active lease, and clears the lease when a new snapshot is published. There is no unlock RPC; an abandoned lease expires. paimon-cpp has none of this: - `CoreOptions` does not know either key. Unknown keys are kept in the raw options and ignored, so a writer configured with `rest.commit.acquire-lock=true` commits without a lease and gives no error. - `ResourcePaths`, `RestApi` and `rest_messages` have no `commit-lock` endpoints or messages. - There is no lock abstraction around `SnapshotCommit` or the retry loop in `FileStoreCommitImpl::TryCommit`. As a result: 1. A C++ writer cannot get the mutual exclusion that Java writers get on a lease-enabled table. Its attempts race with other writers and only rely on the snapshot CAS. 2. While a Java writer holds a lease, the server rejects C++ publications, which can only retry until the lease ends. 3. Setting `rest.commit.acquire-lock=true` on a C++ writer looks like it works but has no effect. Java fails fast when the table lacks the capability, and rejects acquisition when the server does not support the protocol. ## Solution Port the client side of apache/paimon#10491 for REST catalog commits (`RestCatalog` through `WithCatalog()`). **Options** (`include/paimon/defs.h`, `src/paimon/common/defs.cpp`, `src/paimon/core/core_options.{h,cpp}`) - Add `rest.commit.lock-enabled` and `rest.commit.acquire-lock`, both boolean, default `false`. - If `rest.commit.acquire-lock=true` and `rest.commit.lock-enabled` is not `true` in the merged table options, fail before any REST request with the Java message: `rest.commit.acquire-lock requires rest.commit.lock-enabled on the table.` - `rest.commit.lock-enabled` alone must not make a writer acquire. **Protocol** (`src/paimon/rest/resource_paths.*`, `rest_messages.*`, `rest_api.*`) - `POST /v1/{prefix}/databases/{database}/tables/{table}/commit-lock` (acquire) and `.../commit-lock/renew` (renew). - `CommitLockRequest`: `tableId` (resolved table or branch UUID), `commitUser`. - `CommitLockResponse`: `acquired`, `commitUser` (nullable), `expiresAtMillis`, `leaseMillis`, `snapshot` (nullable head at grant time). - Contention returns HTTP 200 with `acquired=false`. Map 404 to table not found, 403 to no permission, and 400 to invalid argument. Never fall back to committing without a lease. **Lease client** (new, for example `src/paimon/rest/rest_commit_lease.{h,cpp}`), mirroring Java `RESTCatalogLock`: - Require a non-empty table UUID (`WithTableId` / `Table::CatalogUuid()`) and commit user. - Acquire: poll every `min(100ms, commit.timeout)` while `acquired=false`, and fail once `commit.timeout` elapses. This wait does not count against `commit.max-retries`. - Validate each grant: `commitUser` must equal ours and `leaseMillis > 0`. - Renew in the background every `leaseMillis / 3`, with at most one renewal in flight. Fail the lease if a renewal returns `acquired=false` or a different `leaseMillis`, or errors. - Track the deadline locally with a monotonic clock, measured from when each acquire or renew request was sent. Before publishing, check the lease is still valid; if it is not, fail the attempt instead of publishing. - Ending a scope stops its renewal. There is no release RPC. **Commit wiring** (`src/paimon/core/table/table.cpp` `NewSnapshotCommit`, `src/paimon/core/operation/file_store_commit_impl.cpp`) - Hold one lease scope per publication attempt, as Java does with `snapshotCommit.runWithLock(...)`. Acquire it at the top of each `TryCommit` iteration, before `LatestSnapshotWithSource()`. Check it right before `SnapshotCommit::Commit()`, and end it when the attempt ends. After a conflict, the next attempt acquires a new lease. - Apply the same scope to `RollbackToAsLatest`, which also publishes through `SnapshotCommit`. - Use the branch identifier that the commit targets, matching `CatalogSnapshotCommit`. - Request-only mode (`UseRESTCatalogCommit(true)`, no catalog) cannot call the lock endpoints. Reject `rest.commit.acquire-lock=true` there instead of ignoring it. - File-system commits are unchanged. **Docs**: describe both options and the lease behavior in `docs/source/user_guide/catalog.rst`, next to the existing commit retry text. **Tests**, ported from Java `RESTCatalogLockTest`, `CommitLockProtocolTest` and `RESTCommitLockIntegrationTest` onto `mock_rest_server`: - option defaults and parsing in `core_options_test.cpp`; JSON round trip of the request and response in `rest_messages_test.cpp`; the new paths in `resource_paths_test.cpp`; - acquisition without table capability fails before any REST request; capability alone does not acquire; - contention polls until granted, or fails at `commit.timeout`; - acquire happens before reading the head, and a new lease is acquired after a conflict; - an invalid grant (wrong `commitUser`, `leaseMillis <= 0`) is rejected; - a failed renewal, `acquired=false`, a changed `leaseMillis` or local expiry blocks the next publication; - request-only mode with `rest.commit.acquire-lock=true` fails. ## Anything else? - Java first added a unified lock interface (`Lock`, `Catalog.createLock`, lock-scoped `SnapshotCommit`) in apache/paimon#10498, together with JDBC and file-system catalog locks. This issue only needs a small internal seam for the REST lease. Porting the other catalog locks is out of scope. - apache/paimon#10577 later removed `commitUser` and `fromSnapshotUuid` from rollback requests. paimon-cpp has no catalog rollback endpoint or automatic conflict rollback, so no rollback protocol change is needed here. - Lease enforcement itself is server-side. This issue only covers what the client must send and check. - References: `docs/static/rest-catalog-open-api.yaml` (`acquireCommitLock`, `renewCommitLock`, `CommitLockRequest`, `CommitLockResponse`), `RESTCatalogLock.java`, `RESTCatalogLockFactory.java`, and `CoreOptions.REST_COMMIT_LOCK_ENABLED` / `REST_COMMIT_ACQUIRE_LOCK`. -- 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]
