Skip to content

[rest] Add table commit leases with unified catalog locks - #10491

Merged
JingsongLi merged 11 commits into
apache:masterfrom
JingsongLi:codex/rest-commit-lock-protocol
Oct 13, 2026
Merged

JingsongLi merged 11 commits into
apache:masterfrom
JingsongLi:codex/rest-commit-lock-protocol

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

REST catalog writers currently coordinate snapshot publication optimistically. Add an opt-in renewable table commit lease through Catalog.createLock, CatalogLockFactory and the unified SnapshotCommit contract. The persisted table property rest.commit.lock-enabled enables server-side enforcement; the independent writer option rest.commit.acquire-lock requests 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 fromSnapshotUuid so 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:

  • JDK 8 and JDK 11: 304 focused API/core tests passed on each with normal Maven checks; the final legacy wire assertion was verified separately on both JDKs. The updated REST suite also passed 48 tests on each JDK, including 32 lease lifecycle tests. Covers Jackson requests, snapshot publication and retries, rollback, catalog serialization and branch identity.
  • Actual file-store commits cover acquisition before head reads, reacquisition after conflicts and separate scopes for subsequent commits. A real COMPACT conflict rolls back and retries with the exact writer, branch, head ID and UUID in both acquisition modes; tests assert the exact remaining rows.
  • Lease tests cover renewal failures, expiry, interruption, executor rejection, cancellation, one in-flight RPC, blocked-worker isolation, nested publication and late replies. A real worker executor verifies that cross-thread renewal extends the acquiring thread's lease beyond its original deadline and expires at the renewed deadline. Real-scheduler tests verify idle thread exit, restart on fresh acquisition, and continued renewal after another scope closes, including when the renewal interval exceeds the idle timeout. Rejected rollback and legacy snapshots without UUIDs leave the conflict unresolved.
  • REST YAML parsing and internal reference checks passed for 62 catalog and 10 management operations. Apache RAT passed.
  • Local integration against an H2/filesystem REST service using this PR's client passed real renewal, owner exclusion, actual FileStoreCommit, owned rollback, snapshot ID reuse protection, branch conflict rollback in all three supported configuration combinations, and accepted publication with a deliberately dropped HTTP response.
  • The supporting service's 191 focused tests passed, including disabled-table zero lease SQL, writer rollback without a lease, expired-lease admission, stale UUID rejection, and a manifest-scan race where another writer rolls back and republishes the same ID. Rejection occurs before file cleanup and preserves the replacement snapshot. An existing-transaction test verifies that the locking read rejects the new UUID even when ordinary reads still return a cached old head.

@JingsongLi
JingsongLi marked this pull request as draft October 10, 2026 14:32
@JingsongLi JingsongLi changed the title [rest] Add table commit lease protocol [rest] Add table commit locks through Catalog Oct 10, 2026
@JingsongLi JingsongLi changed the title [rest] Add table commit locks through Catalog [rest] Add table commit leases with unified catalog locks Oct 12, 2026
@JingsongLi
JingsongLi marked this pull request as ready for review October 13, 2026 06:38

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@leaves12138
leaves12138 dismissed their stale review October 13, 2026 08:17

Dismissed: the inline comment was incorrect because Lease.renew() resolves ensureValid() to Lease.ensureValid(), not the outer ThreadLocal-based method.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@JingsongLi
JingsongLi merged commit 740d322 into apache:master Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants