Repository navigation
[rest] Add table commit leases with unified catalog locks - #10491
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
Request changes. The lease renewal path still has a cross-thread validity bug, so a writer can lose its lease after the initial lease duration even when the REST server is healthy.
| return; | ||
| } | ||
| try { | ||
| ensureValid(); |
There was a problem hiding this comment.
renew() runs on a worker thread, but ensureValid() reads currentLease, which is a ThreadLocal populated only by the thread that called acquire(). Therefore, with the production executor, every renewal calls ensureValid() without a lease, catches the resulting IllegalStateException, stores it in renewalFailure, and permanently stops renewal. Any commit whose preparation/publication lasts beyond the initial lease will then fail even if the server renewal would have succeeded. Please validate the lease state directly inside Lease (or otherwise propagate the lease context) instead of calling the thread-local entry point here, and add a test where workers.execute runs the renewal on a different thread.
Dismissed: the inline comment was incorrect because Lease.renew() resolves ensureValid() to Lease.ensureValid(), not the outer ThreadLocal-based method.
leaves12138
left a comment
There was a problem hiding this comment.
Reviewed the latest head (bfa282c). The commit lease lifecycle, cross-thread renewal scheduling, branch-aware identity propagation, rollback head UUID guard, and REST protocol changes look correct. Focused validation passed: RESTCatalogLockTest (31 tests). Approving.
REST catalog writers currently coordinate snapshot publication optimistically. Add an opt-in renewable table commit lease through
Catalog.createLock,CatalogLockFactoryand the unifiedSnapshotCommitcontract. The persisted table propertyrest.commit.lock-enabledenables server-side enforcement; the independent writer optionrest.commit.acquire-lockrequests acquisition for each publication attempt and can be supplied dynamically. Both default to false. Requesting acquisition without the table capability fails before any REST request.An acquiring writer locks before reading the head and preparing changes. Ownership uses the exact
commitUser, table UUID, runtime branch and authenticated caller. The acquire and renew endpoints use no lock token or retry-only policy. Scopes share a timer and cached worker pool, permit one queued or running renewal per scope, and cancel their own tasks on closure. Idle timer and worker threads exit after one minute and restart when needed; scheduled renewals keep the timer alive. Monotonic lease deadlines and fixed per-scope durations prevent late responses from reviving an expired scope. Failed scopes stop renewal and expire; successful new publications clear the lease atomically. Rejected publications and idempotent replays cannot clear a newer lease.Automatic conflict rollback always forwards the runtime branch and writer identity, including when client acquisition is disabled. Extend the Catalog rollback API, TableRollback and REST request with
fromSnapshotUuidso servers can compare both ID and UUID: another rollback can reuse an ID for a different snapshot. Writer identity is admission context, not a requirement for a lease to exist. A live lease requires its owner and authenticated caller; no or expired lease permits conditional optimistic rollback. Servers must validate the current head inside the mutation transaction before partition changes or file cleanup. Automatic rollback skips legacy snapshots without UUIDs. Manual rollback may omit writer identity, while known table UUIDs remain independent of ownership.Servers use their own persisted capability: disabled tables can skip lease storage entirely; enabled tables enforce active leases for every writer. Capability changes serialize with acquisition and publication, and disabling enforcement cannot bypass a live lease. This is an admission lease, not a generation fencing token; commit users must be unique per logical writer and preserved across recovery. Acquiring on a branch requires its catalog-resolved UUID. The generic Lock, SnapshotCommit and FileStoreCommitImpl implementations are unchanged; external tables retain filesystem publication.
This PR builds on merged #10498 and #10520. A REST server must implement the documented lease admission and ID/UUID rollback checks before using this protocol. This PR supplies the Paimon client and wire contract.
Validation: