Improve DatabaseHealthCheck robustness and connection cleanup

[spbolton]

Proposed Changes

• Replace prepareStatement("SELECT 1") with JDBC4 isValid() for more efficient database health checks
• Bypass ThreadLocal connection caching by calling getDataSource().getConnection() directly to test actual pool connectivity
• Implement interrupt-resilient connection cleanup that preserves thread interrupt status during timeout scenarios
• Replace aggressive shutdownNow() with graceful shutdown() followed by awaitTermination() to allow in-flight cleanup to complete
• Add comprehensive comments explaining the rationale for connection handling and executor shutdown strategy
• Organize imports alphabetically for consistency

Rationale

The previous implementation had several issues:

1. Inefficient health check: prepareStatement("SELECT 1") allocates unnecessary statement resources, while isValid() is the standard JDBC4 approach that pgjdbc implements without statement allocation
2. Cached connection testing: Using DbConnectionFactory.getConnection() may return cached ThreadLocal connections, not testing actual pool availability
3. Connection leak risk: Aggressive shutdownNow() could interrupt connection cleanup, potentially orphaning connections when health checks timeout
4. Thread interrupt loss: The previous cleanup didn't preserve the interrupted flag, which could mask timeout conditions in calling code

Checklist

• Tests
• Translations
• Security Implications Contemplated

Additional Info

The changes maintain backward compatibility while improving reliability. The health check now:

• Tests actual database pool connectivity rather than cached state
• Uses the standard JDBC4 validation method
• Properly handles thread interruption during executor shutdown
• Allows graceful cleanup before forcing termination

No functional behavior changes from a user perspective—the health check still validates database availability, just more robustly.

https://claude.ai/code/session_01KKx8BrFmhNVAA4JSTvhDXu

This PR fixes: #34839

[wezell]

Good.

[wezell]

fine. lotta logic to run a test query.

[wezell]

Seems like a lot of code to run a db query.

[spbolton]

Seems like a lot of code to run a db query.

Fair point — the code isn't dramatically shorter, but each piece now has a clear reason for being there, and the accidental complexity is gone. The previous version manually managed connection cleanup with a Thread.interrupted() /
restore pattern to prevent pgjdbc from throwing during close() on an interrupted thread. But try-with-resources works fine here because we're calling getDataSource().getConnection() directly and nothing inside the try block calls
deeper code that relies on this connection. We just borrow it from the pool, validate, and release it — close() returns the HikariCP proxy straight back to the pool.

It's interactions with the ThreadLocal connection (via DbConnectionFactory.getConnection()) that make try-with-resources tricky — if code called at a deeper level expects that same connection to still be open, closing it out from
under them breaks the flow. That doesn't apply here since no other code touches this connection.

Kept isValid() as the validation — while HikariCP validates connections on borrow, it skips validation for connections idle less than 500ms and doesn't validate newly created connections. Since the health check's job is to confirm
the database can actually process requests (not just that a TCP connection exists), isValid() closes that gap. pgjdbc implements it with the empty query protocol — one round-trip, no statement allocation.

The timeout is now configurable via health.check.database.timeout.seconds (default 2s). This is intentionally lower than HikariCP's connectionTimeout (default 5s) — the health check should fail fast and report status quickly, not
wait as long as a normal request would. If the pool can't hand out and validate a connection within 2s, that's already a signal for the readiness probe to remove the pod from load balancing.

Also replaced the per-call Executors.newSingleThreadExecutor() with HealthCheckUtils.executeWithTimeout() — a shared cached thread pool with daemon threads, the same pattern used by CacheHealthCheck and DatabaseHealthEventManager.
This eliminates thread allocation on every health check poll and the manual executor shutdown/awaitTermination boilerplate that was needed to prevent pgjdbc interrupt-during-close issues (#34490). The shared pool reuses threads
across polls and lets them expire after 60s idle.

This also aligns with the direction in #34832 — once keepaliveTime and tcpKeepAlive land, HikariCP will proactively validate idle connections, further strengthening the signal from a simple borrow + isValid() check.

[dcolina]

👍🏽