ODriscollBoys opened a new pull request, #68870:
URL: https://github.com/apache/doris/pull/68870
### What problem does this PR solve?
Issue Number: close #68080
Related PR: None
Problem Summary:
A JDBC catalog scan, write or connection test borrows a connection from the
catalog's HikariCP pool, and the connection goes back to the pool only when it
is closed. Two paths never closed it, so its pool entry stayed `IN_USE`
forever. Once every entry of a pool is held like that (`total = active =
connection_pool_max_size`, `idle = 0`), every later query on the catalog fails
with `Connection is not available, request timed out`, until the pool is
rebuilt, e.g. by changing `connection_pool_max_size`. The issue was reported
against `BaseJdbcExecutor` on 2.1; on master the same lifecycle lives in
`JdbcJniScanner`, `JdbcJniWriter` and `JdbcConnectionTester`.
1. **A failed open.** These classes borrow the connection in
`openInternal()` and release it in `closeInternal()`. But BE only calls the
Java `close()` for a scanner or writer whose `open()` succeeded
(`JniReader::close()` and `VJniFormatTransformer::close()` return early when it
was never opened). So when anything after `getConnection()` fails, e.g.
preparing the statement on a connection the database already dropped, or the
query of a connection test, the borrowed connection is never closed.
2. **A failed close.** `closeInternal()` closed the result set, the
statement and the connection in one `try` block, so an exception from closing
the result set or the statement skipped `conn.close()`. This is common right
after `abortReadConnection()`, which aborts MySQL and SQL Server connections
whose result set was not fully read. HikariCP untracks a statement before
closing it, which matches the leaked entries in the issue: `IN_USE`, physical
connection closed, no open statements. `JdbcJniWriter` had the same problem
when the commit or a close failed, and `JdbcConnectionTester` when closing the
statement failed.
The fix:
- A failed `openInternal()` now releases what it acquired before rethrowing:
the scanner and the connection tester call their `closeInternal()` (as
`MaxComputeJniScanner` already does), and the writer closes its statement and
connection without committing.
- `closeInternal()` closes the result set, the statement and the connection
independently through the new `JdbcResources.closeQuietly`, so the connection
is always closed. The writer still commits first and still reports a failed
commit or close, but now closes the connection in every case. Closing an
already closed JDBC resource is a no-op.
### Release note
Fix JDBC catalog connection pool exhaustion caused by connections that were
never returned to the pool when opening a scan, a write or a connection test
failed, or when closing the statement of a scan failed.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
Added `JdbcConnectionReleaseTest`, which runs the scanner, the writer
(with and without a transaction) and the connection tester against a fake
driver and a pool of one connection. Without this change all four cases fail
because the next open times out with `active=1`; with it they pass and the pool
is left with no active connection. `mvn -pl be-java-extensions/jdbc-scanner -am
test` and `checkstyle:check` pass for the module.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
