geyanggang opened a new pull request, #13769:
URL: https://github.com/apache/gravitino/pull/13769
### What changes were proposed in this pull request?
Relational JDBC catalog creation now verifies the backend is reachable,
consistently
across JDBC providers, instead of silently succeeding and failing later on
an unrelated
operation. The check is bounded so it fails fast and reports the failure to
the caller
naming the host.
Three coordinated changes:
1. **Bounded JDBC connect/socket timeout (`catalog-jdbc-common`).**
- Add `jdbc.pool.connect-timeout-ms` to `JdbcConfig` (default `30000`,
positive-value
check), plus a `getConnectTimeoutMs()` getter.
- Add a pure, driver-aware utility `JdbcConnectionTimeouts` that
translates this
canonical milliseconds value into each driver family's connection
parameters, since
the parameter unit differs: MySQL Connector/J
(`com.mysql.cj.jdbc.Driver` /
legacy `com.mysql.jdbc.Driver`, also used by Doris/StarRocks) takes
milliseconds;
PostgreSQL (`org.postgresql.Driver`) takes seconds (ms rounded up,
minimum 1). An
unknown driver family yields no properties, so adding a new provider
never breaks.
Any value the operator already set — as a connection property or a
`jdbc-url` query
parameter — is preserved (defaults, never overrides; the URL check is
case-sensitive
and boundary-anchored).
- `DataSourceUtils.createDBCPDataSource` applies the translated
properties via
`addConnectionProperty`, so they are per-connection bounds, do not
collide with the
unsafe-pool-property guard, and keep the pool lazy.
2. **`gravitino.catalog.verify-on-create` control property (`core`).**
Registered in `BaseCatalogPropertiesMetadata` as a hidden, non-reserved
boolean with
default `true`, with the key constant on `Catalog`. It is a registered
property (so it
is not silently dropped during create validation) that lets an
administrator opt out of
create-time verification to register a catalog whose backend is
temporarily unavailable.
3. **Create-time verification via a `validateOnCreate()` hook (`core` +
`jdbc`).**
- Add `BaseCatalog.validateOnCreate()`, whose default body eagerly
initializes `ops()`,
preserving current behavior for existing opt-ins (e.g. Iceberg REST).
- `CatalogManager.initCatalogWrapper` now calls `validateOnCreate()`
instead of `ops()`
on the existing `shouldValidateOnCreate()` opt-in path (one-line
change).
- `JdbcCatalog` overrides `shouldValidateOnCreate()` (reads
`verify-on-create`,
defaulting to `true`) and `validateOnCreate()` to run a single
`testConnection`
(one `listDatabases` round trip). On failure a
`ConnectionFailedException` propagates,
and the existing create rollback removes the just-persisted catalog
entity.
- `JdbcCatalogOperations.testConnection` enriches the
`ConnectionFailedException`
message with the `host:port` parsed from the `jdbc-url` (same exception
type).
Docs: `docs/jdbc-catalog-connection-validation.md` documents the create-time
verification,
the override, and the bounded connect timeout.
### Why are the changes needed?
Creating a relational JDBC catalog returned success without any check that
the backend was
reachable, and the behavior was not consistent across providers. An
administrator who
mistyped a host, port, or credential got a success response and a catalog
that did not
work; the failure then surfaced later on an unrelated operation.
The cause is that `CatalogManager.createCatalog` performed no connection
check — whether
the backend was touched during create depended entirely on each connector's
`initialize()`
side effects. JDBC catalogs build a lazy DBCP2 connection pool
(`DataSourceUtils.createDBCPDataSource`, with `initialSize` blocked), so
create opened no
connection and never reached the backend. The existing
`BaseCatalog.shouldValidateOnCreate()`
hook only eagerly called `ops()`, which for JDBC still opened no connection,
so even
opting in did not verify reachability.
There was also no bounded JDBC connect/socket timeout: `JdbcConfig` exposed
only pool
settings and `DataSourceUtils` set only the pool `maxWait`, so a connection
attempt against
an unreachable host could block for the driver default (effectively
unbounded for the
MySQL-family default `connectTimeout=0`). Bounding it is a prerequisite for
safe create-time
verification (which runs inside the metalake write lock) and a useful
hardening on its own.
Fix: #13767
### Does this PR introduce _any_ user-facing change?
Yes.
- **Behavior:** By default, creating a relational JDBC catalog now verifies
the backend is
reachable and returns a connection-failed error (naming the host) if it is
not, instead of
succeeding. Set `gravitino.catalog.verify-on-create=false` on the create
request to restore
the previous unverified behavior.
- **New property keys:**
- `gravitino.catalog.verify-on-create` (catalog property, boolean, default
`true`, hidden).
- `jdbc.pool.connect-timeout-ms` (JDBC catalog config, long, default
`30000`).
- **Unaffected:** Iceberg REST keeps its existing selective create-time
validation; model
and generic catalogs, and any catalog that does not opt in, are never
force-verified. The
standalone `testConnection` API is unchanged.
### How was this patch tested?
New and extended unit tests:
- `catalog-jdbc-common`:
- `TestJdbcConnectionTimeouts` — MySQL family emits milliseconds;
PostgreSQL converts
ms→seconds (round up, minimum 1); unknown driver emits nothing; an
operator-set
`connectTimeout`/`socketTimeout` is preserved whether set as a config
key or a `jdbc-url`
query parameter (case-sensitive, boundary-anchored); custom-value
propagation; null-URL
safe.
- `TestJdbcConfig` — the new setting's default and override.
- `TestDataSourceValidation` — an unknown driver injects nothing and the
pool still borrows
a connection.
- `core`:
- `TestCatalogManager` — an opted-in catalog whose backend probe fails
propagates
`ConnectionFailedException` and rolls back the persisted entity (store
no longer contains
it, identifier is reusable); a non-opted-in catalog is not verified; the
existing backend
validation test is unchanged.
- `TestCatalog` / `TestCatalogOperations` — simulate a failing
`testConnection` without a
real backend.
Commands run (from the repo root):
- `./gradlew spotlessApply` — SUCCESS.
- `./gradlew :catalogs:catalog-jdbc-common:test :core:test -PskipITs` —
BUILD SUCCESSFUL
(core: 2699 tests, 1 skipped, 0 failed), modules compiled with checkstyle
`-Werror`.
--
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]
