Use query-less datasource validation by default

[ckoutsouridis]

To add a bit more context on why i ask this. We have noticed that in our database there are a lot of select 1 from dual queries. which is neither a heavy query or bad..but do we really need to run a query to validate the datasource?

isn't a simple (with the reasonable try-catch blocks of-course)

datasource.getConnections().close();

enough?

or for extra validation

datasource.getConnections().isValid();

both the above 2 approaches will result in the same behaviour as the existing one with better timeout handing in some jdbc drivers (for example for oracle).
Besides, all datasource pools, are already doing validation according to their configs in an optimal way.

for example have a look at micronaut's health indicator

single simple file, database agnostic.

I believe this will simplify a lot the health check, and dba's around the world will be happy as well

If the community does not feel comfortable enough with jdbc driver validation mechanisms in general ( i don't see why not) , maybe we could add a property to control whether datasourceHelathIndicator should run a query, or rely on the jdbc driver isValid method.

besides, this looks like a double config problem,
we specify in one place how hikari (personally i chose the isValid method of the jdbc driver) validates, and then there is the datasourceHealthIndicator which is overriding this validation scheme.

this probably fixes: #4391 . At least for oracle, as in oracle drivers, the connection.isValid() respects the timeout that is configured in the pool. Probably same applies in other jdbc drivers

[wilkinsona]

both the above 2 approaches will result in the same behaviour as the existing one

This would not result in the same behaviour. For example, we know that we have users that are using a custom query to meet their interpretation of what it means for the DataSource to be healthy.

besides, this looks like a double config problem,
we specify in one place how hikari (personally i chose the isValid method of the jdbc driver) validates, and then there is the datasourceHealthIndicator which is overriding this validation scheme.

Let's consider providing an option to use isValid() in the health indicator. That should satisfy both use cases.

[ckoutsouridis]

Thank you @wilkinsona. i would suggest to use the hikariCP approach.

In hikariCP if we don't specify a connection-test-query, the validation will be done against the connection.isValid() of the jdbc driver .

in the same way, if someone is not specifying a custom query for the healthIndicator, maybe it's better to just use the isValid().

Besides, if it's not a custom query the isValid and select 1 from * are kind of equivalent.

We should also keep in mind that if we configure the datasource with something like
spring.datasource.hikari.connection-test-query=test_my_db_query

Apart from some minor timing windows, this query will get executed by a simple datasource.getConnection()

[wilkinsona]

Apart from some minor timing windows, this query will get executed by a simple datasource.getConnection()

We cannot be certain that is the case if the DataSource has been proxied. It's possible for a Connection returned from DataSource.getConnection() to be lazy and only actually retrieve a Connection from the pool when it's used.

[ckoutsouridis]

interesting, well, i am not sure how all datasource pools work, but at lest for hikaricp, the pool will not release a connection unless it passes the validation stage (according to it's ALIVE_BYPASS_WINDOW_MS).

So with a default setup, if a connection is not used within 500ms, the connection validation will run just by a datasource.getConnection()

in fact, after debugging this, the pool validation query is executed by the healthIndicator, even before the check query, it is executed during this block:

	private String getProduct(Connection connection) throws SQLException {
		return connection.getMetaData().getDatabaseProductName();
	}

In any case, any improvement in this, would be welcome.

[ttddyy]

+1 for using isValid() on DataSourceHealthIndicator.

Currently, in our tracing, there are bunch of SELECT 1 traces.
So, we need to exclude SELECT 1 on tracing configuration for jdbc instrumentation.
Such traces affect rate limit/capacity for the tracing service.

I like the idea that if test query is specified, then use the query; otherwise use isValid.
Though, it wouldn't really match with current design which retrieves default validation query from DatabaseDriver#validationQuery when query is not specified.

According to hikari documentation:

connectionTestQuery
If your driver supports JDBC4 we strongly recommend not setting this property. This is for "legacy" drivers that do not support the JDBC4 Connection.isValid() API. ...

We introduced similar in R2DBC-SPI, Connection#validate. This is because driver knows better in current condition of the connection, and it may have internal optimal way to check the status of the connection rather than performing validation query.

[snicoll]

I've started to hack something and, given the different in behaviour, I am wondering if that wouldn't be better to have a completely separate implementation (see 89c73bc). Using Connection#isValid is very simple, does not require spring-jdbc on the classpath (as there is no need for JdbcTemplate) and mixing the two modes in the same implementation feels a bit confusing to me.