dennismdejong opened a new pull request, #13716:
URL: https://github.com/apache/gravitino/pull/13716
### What changes were proposed in this pull request?
The catalog datasource no longer configures a DBCP2 validation query;
borrowed connections are validated through the JDBC4 `Connection.isValid(int)`
call instead (`testOnBorrow` is unchanged, so `jdbc.pool.test-on-borrow` still
decides whether validation happens).
The defect is a statement/database binding interaction between DBCP2 and
MySQL Connector/J:
1. DBCP2 prepares the validation query once per physical connection and
reuses that `PreparedStatement` for every later validation
(`PoolableConnection.validate`).
2. MySQL Connector/J binds every `Statement`/`PreparedStatement` to the
database that was current when the statement was created, and on execution
switches the connection back to that database
(`ClientPreparedStatement.execute` →
`locallyScopedConn.setDatabase(getCurrentDatabase())`).
3. The JDBC catalog borrows a pooled connection and then switches its
catalog for the operation (`JdbcTableOperations.getConnection`,
`JdbcTablePartitionOperations`). With a JDBC URL that has no default database,
the connection is catalog-less when the validation statement is created, so
every later validation of that connection fails with `SQLState 3D000 / error
1046` ("No database selected").
4. `PoolableConnectionFactory.validateObject` then returns `false`, DBCP2
destroys the connection, and the pool opens a new physical connection on every
borrow.
Dropping the validation query makes DBCP2 validate through
`Connection.isValid(int)`, which MySQL implements as `COM_PING`, evaluated
against the connection's *current* catalog rather than against a statement
bound to a stale one.
### Why are the changes needed?
Fix: #13592
With the documented URL form `jdbc:mysql://host:port/` (no default database)
the pool never reuses a connection: every borrow opens a new physical one,
which under Table API load exhausts ephemeral ports (`java.net.BindException:
Can't assign requested address`). Picking an otherwise unused default database
was only a workaround.
### Does this PR introduce _any_ user-facing change?
No API, REST or property-key changes. The only observable difference is that
`jdbc.pool.test-on-borrow` validation no longer issues `SELECT 1` for MySQL (it
uses the driver's `isValid`, i.e. `COM_PING`), and that connections are reused
as intended. No configuration key was added or removed.
### How was this patch tested?
New regression test `TestDataSourceConnectionReuse` in
`catalog-jdbc-common`: a fake JDBC driver reproduces the Connector/J binding
semantics (a statement fails once the connection switched catalogs,
`Connection.isValid(int)` does not) and asserts, after the pool is warmed up,
that 20 borrow/`setCatalog`/close cycles open **zero** further physical
connections, never execute a statement against a stale catalog, and still
validate every borrow. It fails on the previous code (`expected: <0> but was:
<20>` borrow-induced connections) and passes with this change.
Additionally verified end-to-end against a real MySQL 8.0.46 /
mysql-connector-j 8.0.33, driving the production `DataSourceUtils` for 100
sequential borrow/`setCatalog("perf_tables")`/close cycles and counting
server-side connections:
| Configuration | Physical connections created by the app |
| --- | --- |
| before the fix (`validationQuery=SELECT 1`) | 100 |
| after the fix, URL without default database | 1 |
| after the fix, URL with default database | 1 |
A connection killed server side (`KILL <id>`) is still detected on the next
borrow and replaced, i.e. validation is not silently skipped. The report's raw
driver observations were reproduced as well: a `PreparedStatement` prepared
while no database is selected fails with `3D000/1046` after `setCatalog`, and a
plain `Statement` behaves the same way.
`./gradlew :catalogs:catalog-jdbc-common:spotlessCheck
:catalogs:catalog-jdbc-common:test` passes.
--
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]
