DataSourceHealthIndicator need timeout

[bohrqiu]

for the business function，we will set database timeout(get connent form pool time out or exec query wait time out ) very long ,but It's not appropriate for HealthIndicator. in HealthIndicator，We just check whether the connection is available. so we should add timeout for HealthIndicator,as DataSourceHealthIndicator an example

https://gist.github.com/bohrqiu/094e6e22a56b684fcec8

[bohrqiu]

We'd better add timeout on org.springframework.boot.actuate.health.AbstractHealthIndicator#health method，all HealthIndicator can timeout

[philwebb]

We're cleaning out the issue tracker and closing issues that we've not seen much demand to fix. Feel free to comment with additional justifications if you feel that this one should not have been closed.

[philwebb]

It's also worth noting that as of 2.0 there is caching with health endpoints and the option of a custom TTL

[kostellodon]

Sorry, don't know the etiquette, if leaving a comment here vs creating a new issue is preferred. This is something that we need. We have a spring boot app deployed to a container (cloud foundry) that gets intermittent health check failures due to the endpoint not responding. We see no exceptions in our logs, so I'm assuming one of the health check components is taking too long to respond, but we have no way of knowing which one. Having a timeout on the health check indicators would help us track down what is causing our intermittent failures.

[philwebb]

@kostellodon Timeout support is going to be quite hard for us to implement, but your use-case makes me think that we should try to improve our logging. I've opened #31231 to track that.

[yassenb]

@philwebb , I would like to point out that logging the timed out health checks is an improvement but still does not resolve the original issue. Imagine a Kubernetes deployment. From Kubernet's standpoint the liveness probe still just timed out, there is no additional information. One has to go and dig through the logs for warnings and attempt to correlate the timing with the time of the failed liveness probe. If a proper timeout is implemented in DataSourceHealthIndicator Kubernetes would be able to store and display the result of the HTTP request which would contain information about the failed health checks

[philwebb]

Understood, but we still don't have an easy to implement timeout support I'm afraid.

[yassenb]

I'm not familiar enough with Spring internals to open a PR but where is the complication in implementing it for DataSourceHealthIndicator. If I'm not missing anything in the method

private Boolean isConnectionValid(Connection connection) throws SQLException {
	return connection.isValid(0);
}

all that's needed is to replace the 0 with something configurable, something in the lines of management.endpoint.health.db.timeout for the configuration path

[philwebb]

Perhaps it's not as hard as I feared. Some vendors seem to ignore the timeout, but it looks like MySQL and Postgres support it. We'll discuss this on a team call and decide what to do.

[philwebb]

@yassenb We talks about this on our team call and other members of the team reminded me why we didn't go down the connection.isValid(timeout) route. The problem is that we also support the use of a validation query and the JDBC API doesn't offer a timeout for calling that.

That leaves us back with the idea of running the health indicator in a background thread so we can offer a timeout. It seems like there is demand for that, and it's a more general problem than just datasource health indicators. I've opened #43449 to see what we can do.

[wilkinsona]

I missed yesterday's meeting, but I wonder if configuring the query timeout on the JdbcTemplate would help here? At the moment, I think we're relying on the JDBC driver's default.

[philwebb]

Interesting, it looks like I missed that java.sql.Statement also supports a timeout (although only in seconds). The code in DataSourceUtils.applyTimeout(...) is used by JdbcTemplate.