soulmachine opened a new pull request, #11020:
URL: https://github.com/apache/gravitino/pull/11020
### What changes were proposed in this pull request?
This PR addresses TreeLock's in-process-only limitation for HA deployments
(#10474) by introducing a pluggable lock backend layer:
1. **`LockBackend` SPI** under `org.apache.gravitino.lock`. The default
`InProcessLockBackend` wraps the existing in-JVM `TreeLock` machinery
byte-for-byte — single-node deployments see no behavior change.
2. **`JdbcLockBackend`** (opt-in via `gravitino.lock.backend.type=jdbc`)
coordinates locks across nodes using `SELECT ... FOR SHARE / FOR UPDATE`
on a single `gravitino_lock(lock_path PRIMARY KEY)` table. Supports H2
(with shared→exclusive degradation), MySQL, and PostgreSQL via a small
`JdbcDialect` enum.
3. **Design document** `design-docs/treelock-ha.md` (~550 lines) walking
through the current implementation, HA failure modes, the Option A/B/C
trade-off space, the recommended hybrid, JDBC backend internals,
correctness analysis, and a 4-phase migration plan.
No new dependency: `JdbcLockBackend` uses `commons-dbcp2` which is already
on `core`'s classpath.
### Why are the changes needed?
`TreeLock` is per-process — a write lock taken on server A is invisible to
server B, so HA deployments behind a load balancer can race on the same
metadata resource and corrupt state. Detailed failure-mode analysis is in
§2 of the design document.
This PR introduces the foundation (SPI + opt-in JDBC backend + design doc)
without forcing a behavior change. Phase B in §8 of the design doc is
optimistic CAS in `RelationalEntityStore` — the next staged PR — which is
the real correctness fix for the dominant case (internal metadata races)
and works regardless of which lock backend is configured.
Fix: #10474
### Does this PR introduce _any_ user-facing change?
Yes — additive only:
- **New configs** (defaults preserve current behavior):
- `gravitino.lock.backend.type` (default `inprocess`)
-
`gravitino.lock.backend.jdbc.{url,user,password,driver,poolSize,acquireTimeoutMs}`
— only consulted when `type=jdbc`; URL/user/password/driver fall back to
the existing `gravitino.entity.store.relational.jdbc*` keys when unset.
- **New public Java APIs** in `org.apache.gravitino.lock`:
`LockBackend`, `LockHandle`, `LockBackendException`. All marked
`@Evolving`.
- **`LockManager.close()`** added and wired into `GravitinoEnv.shutdown()`.
- **No removed or renamed APIs.** All existing call sites of
`TreeLockUtils.doWithTreeLock` are unchanged in semantics.
### How was this patch tested?
- `./gradlew :core:test -PskipITs --tests "org.apache.gravitino.lock.*"` —
all lock tests pass. Existing `TestLockManager`, `TestTreeLock`,
`TestTreeLockUtils` are unchanged. New tests:
- `TestInProcessLockBackend` — exercises the default backend and the
null/empty-config defensive path.
- `TestJdbcLockBackend` — exercises acquire/release, schema initialization,
root-only locks, idempotent close, reentrancy detection (rejected with a
clear error in v1), and WRITE/WRITE serialization between two threads
against H2 in-memory.
- `TestJdbcDialect` — covers the SQL strings and timeout-clamping for all
three supported dialects.
- Regression sample: `./gradlew :core:test -PskipITs --tests
"org.apache.gravitino.catalog.TestCatalogManager"
"org.apache.gravitino.catalog.TestOperationDispatcher"` — green.
- `./gradlew :core:spotlessApply` — clean.
- MySQL/PostgreSQL dialects are covered by `TestJdbcDialect` at the
SQL-string
level. Container-based integration tests against real MySQL/Postgres are
intentionally deferred to land alongside Phase B (optimistic CAS) when the
end-to-end HA story is exercised.
--
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]
