SteNicholas opened a new pull request, #474:
URL: https://github.com/apache/paimon-cpp/pull/474
<!-- PR titles must follow Conventional Commits: <type>(<optional-scope>):
<description> -->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #470
<!-- What is the purpose of the change -->
Java Paimon added table commit leases to the REST catalog
(apache/paimon#10491, ownership
clarified in apache/paimon#10588). paimon-cpp knew neither option, so a
writer configured with
`rest.commit.acquire-lock=true` committed without a lease and without an
error, raced writers
that hold one, and was rejected by the server while a Java writer held the
lease.
Port the client side of REST commit leases for commits through
`WithCatalog()`, aligned with
Java `RESTCatalogLock` and `RESTCatalogLockFactory`.
- Add `rest.commit.lock-enabled` (table property) and
`rest.commit.acquire-lock` (writer
option), both default `false`. Asking to acquire without the table
capability fails with the
Java message before any REST request; the table property alone does not
make a writer acquire.
A commit lease also requires the table UUID passed to `WithTableId()`, and
a request-only
commit built with `UseRESTCatalogCommit()` rejects
`rest.commit.acquire-lock=true`.
- Add the `commit-lock` and `commit-lock/renew` endpoints with
`CommitLockRequest` and
`CommitLockResponse`.
- Add an internal `CommitLock`/`CommitLockScope` interface mirroring Java
`Lock`.
`SnapshotCommit::AcquireLock()` returns the scope of one publication
attempt,
`CatalogSnapshotCommit` holds the lock created by
`VersionManagedCatalog::CreateCommitLock()`,
and `RestCatalog` returns a `RestCommitLock` bound to the branch
identifier, table UUID and
commit user.
- `RestCommitLock` polls every `min(100ms, commit.timeout)` while another
writer holds the
lease, without using up `commit.max-retries`, and rejects a grant for
another commit user or
without a positive `leaseMillis`. Each scope renews its lease every
`leaseMillis / 3` on a
detached thread and tracks the deadline on a monotonic clock from when
each grant was
requested, so a refused or failed renewal, a changed `leaseMillis` or a
local expiry
invalidates it. Ending a scope only stops renewing; there is no unlock
request. A new
`RestHttpClient::Cancellation` aborts the renewal transfer and its retry
backoff.
- `FileStoreCommitImpl` acquires a scope before reading the head of each
attempt, checks it
right before `SnapshotCommit::Commit()` and releases it before waiting to
retry. A failed
acquisition or a lost scope publishes nothing and is retried within the
`commit.max-retries`
and `commit.timeout` budget of a conflict; an attempt that lost its scope
removes the
manifests it wrote, now including changelog manifests and their list.
`RollbackToAsLatest`
holds a scope too and fails when it cannot acquire or loses it.
File-system commits are unchanged.
### Tests
<!-- List UT and IT cases to verify this change -->
- `RestCommitLockTest` in `src/paimon/rest/rest_commit_lock_test.cpp`,
ported from Java
`RESTCatalogLockTest`:
- `MissingIdentityIsRejectedBeforeAnyRequest`: no table UUID, commit user
or valid timeout.
- `AcquirePollsUntilGranted` and `ContentionTimesOutWithoutRenewing`:
contention polls until
granted, or fails once the timeout elapses.
- `FailedAcquisitionStartsNoLease`: 400, 403 and 404 map to invalid
argument, no permission
and table not found; a grant for another commit user or with
`leaseMillis=0` and a renewal
thread that cannot start fail the acquisition without renewing.
- `RenewalExtendsTheLeaseFromEachRequest`,
`FailedRenewalInvalidatesTheLease` and
`LocalExpiryInvalidatesTheLease`: the deadline is measured from each
request; a renewal
answered `acquired=false`, an error, another owner or a changed
`leaseMillis`, and a local
expiry, invalidate the lease.
- `EndingTheScopeDoesNotWaitForARenewalInFlight`,
`LateAnswerToAnEndedScopeCannotAffectTheNextScope` and
`LeasesOfDifferentWritersDoNotInterfere`.
- `src/paimon/rest/rest_catalog_test.cpp`, following Java
`RESTCommitLockIntegrationTest` and
`CommitLockProtocolTest`:
- `RestCatalogTest.CommitLockAddressesTheTableOrBranch`: leases address
the table or branch,
and system tables are rejected.
-
`RestCatalogCommitLeaseTest.LeaseIsAcquiredBeforeTheHeadAndAgainAfterAConflict`,
`LeaseNeedsBothOptionsAndTheTableUuid`,
`ContendedLeaseIsAwaitedUntilCommitTimeout`,
`LeaseExpiredLocallyBeforePublicationSendsNoCommit` and
`RenewalRefusedBeforePublicationSendsNoCommit`.
- `FileStoreCommitImplTest` in
`src/paimon/core/operation/file_store_commit_impl_test.cpp`:
`TestCatalogCommitHoldsALockScopePerAttempt`,
`TestCatalogCommitRetriesOnceTheLockScopeIsLost`,
`TestCatalogCommitRetriesAFailedLockAcquisition`,
`TestCommitWithoutAUsableLockIsNotCreated`
and `TestRollbackToAsLatestHoldsALockScope`;
`TestCatalogCommitToBranchTakesTheBranchSnapshot`
checks the lock is bound to the branch identifier.
- `RestHttpClientTest.CancellationStopsWaitingForAResponse`,
`CancellationEndsARetryBackoff`
and `CancelledRequestIsNotSent` in
`src/paimon/rest/rest_http_client_test.cpp`.
- `CatalogSnapshotCommitTest.TestAcquireLockUsesTheLockItWasGiven`.
- `RestMessagesTest.CommitLockRequestRoundTrip` and
`CommitLockResponseRoundTrip`,
`ResourcePathsTest.WithPrefix`, and `CoreOptionsTest.TestDefaultValue` and
`TestFromMap`.
### API and Format
<!-- Does this change affect API in include dir or storage format or
protocol -->
`include/paimon/defs.h` adds `Options::REST_COMMIT_LOCK_ENABLED` and
`Options::REST_COMMIT_ACQUIRE_LOCK`. No storage format changes.
On the wire, a writer with `rest.commit.acquire-lock=true` sends `POST
.../commit-lock` and
`POST .../commit-lock/renew` with `tableId` and `commitUser`, as the Java
client does. Other
requests are unchanged.
### Documentation
<!-- Does this change introduce a new feature -->
Yes. `docs/source/user_guide/catalog.rst` describes both options, how a
lease is acquired,
renewed and checked per attempt, how `commit.timeout` bounds the wait, and
that a request-only
commit cannot hold a lease.
### Generative AI tooling
<!--
If generative AI tooling has been used in the process of authoring this
patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling
Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
Generated-by: Claude Code 2.1.311 (Claude Opus 5.5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
