eclipse / microprofile-health

microprofile-health
Apache License 2.0
108 stars 61 forks source link

HealthCheck not supported async mode [enhancement] #211

Open deripas opened 5 years ago

deripas commented 5 years ago

Already using jboss-servlet-api_4. 0 which supports AsyncContext (since 3.0). I want to be able to implement asynchronous HealthCheck, for example:

public class SimpleHealthCheck  implements HealthCheckAsync {
    @Override
    public CompletionStage<HealthCheckResponse> call() {
        return CompletableFuture.supplyAsync(() -> {
            return HealthCheckResponse.builder().up().name("redis/mongo/eq").build();
        });
    }
}

or RxJava

public class SimpleHealthCheck  implements HealthCheckReactive {
    @Override
    public Single<HealthCheckResponse> call() {
        return Single.fromCallable(() -> {
            return HealthCheckResponse.builder().up().name("redis/mongo/eq").build();
        });
    }
}
kenfinnigan commented 5 years ago

Can you elaborate on the benefits of supporting asynchronous return types given the health check call doesn't actually happen until someone requests it?

deripas commented 5 years ago

Many libraries already support the asynchronous api, such as: nosql drivers (mongodb-driver-reactivestreams, Cassandra), Redis client (Lettuce ), etc.

doesn't actually happen until someone requests it

Yes, but this is not a one-time request, but a regular load, especially in the context of microservices. Microservices are periodically polled Liveness/Readiness probs and at the same time have limited resources (cpu, threads, ...). I'm a little confused that there are ways to do asynchronous rest-controller, but health-check endpoint only blocking mode. It forces me to abandon the use of health-api (HealthCheck) and make my own health async rest-controller.

olotenko commented 4 years ago

I agree with the need to introduce async/rx API. People will poll other services to determine readiness and or liveness, even if you tell them not to. In the absence of asynchronous API this may consume threads unnecessarily. (Compare to JAX-RS AsyncResponse and support for CompletionStage<Response> as the return type)

xstefank commented 4 years ago

We took a stance to not introduce new features into the specification which may be still unstable. So we introduced async health checks in the SmallRye Health implementation -> https://github.com/smallrye/smallrye-health/blob/master/implementation/src/main/java/io/smallrye/health/SmallRyeHealthReporter.java#L208 that utilize Mutiny as a reactive framework. We also would like to encourage different implementations to also introduce some form of their async API so we can catch most of the errors before we move the API to the spec which would result in a smaller need for breaking changes in the future.

If you can, please give a try and report any problems/enhancements that you find.