Add separate thread pool for health checks

[evyrosseels]

When the TOMCAT_MAX_THREADS limit is reached and a http health check is performed, the check will fail because there are no more available connections/threads to handle the request in time. For a Kubernetes liveness probe this will eventually cause the Alfresco container to be killed.

This PR adds an additional connector with a limited number of threads so health probes can use this instead of the main thread pool.

[Brobrechts]

When the TOMCAT_MAX_THREADS limit is reached and a http health check is performed, the check will fail because there are no more available connections/threads to handle the request in time. For a Kubernetes liveness probe this will eventually cause the Alfresco container to be killed.

This PR adds an additional connector with a limited number of threads so health probes can use this instead of the main thread pool.

Dumb question, but is this what is needed?
The health check will show that the system no longer responds because all threads are tied up doing work.
(yes, the system is doing work, all 200 threads) but now we have a pod reporting healthy while it cannot server response.

Also, there is a 200 thread limit, but not necessarily connection limit, I might be wrong but I think the healthcheck would go into a queue. So unless the 200 threads are all working on long running processes, I would imagine it to just be slow?

I don't have the context, so you can potentially disregard if the topic was already checked, but it feels like tweaking the health check could be a better route, but I imagine things are not always so simple

[gert-glassee]

The issue is that the liveness probe does not get a timely reply and therefore thinks that it must kill the container.
If the threadpool is fully busy the prometheus metrics are also not delivered.
Tweaking the liveness probe a bit could indeed also bring some solace but would not solve the prometheus issue.

When (in case of OUP) a large load is send to the ACS pool this situation could go on for more than an hour. If the readiness probe would remove the IP from the service, there would not be any containers attached to the service and the requests would fail with connection errors.
If required, you could map the readiness probe to the existing pool and the metrics/liveness to the new pool.

[Brobrechts]

The issue is that the liveness probe does not get a timely reply and therefore thinks that it must kill the container. If the threadpool is fully busy the prometheus metrics are also not delivered. Tweaking the liveness probe a bit could indeed also bring some solace but would not solve the prometheus issue.

When (in case of OUP) a large load is send to the ACS pool this situation could go on for more than an hour. If the readiness probe would remove the IP from the service, there would not be any containers attached to the service and the requests would fail with connection errors. If required, you could map the readiness probe to the existing pool and the metrics/liveness to the new pool.

Ok...I known the prometheus call has a rather...stringent timeout by default (3s).

Is there not a risk if the threadpool is completely blocked (deadlock.) the container won't fix itself by restarting because the healthcheck is on another pool?

I just don't know if this is caused by latency or connection errors.

This is also not opt-in right. Does it imply that all helm based deployments need to point to the new endpoint? (after building with a new alfresco image, or just rebuilding in general.)
ALFRESCO_DEFAULT_LIVE_PROBE (runs on 8090 now?)

[evyrosseels]

This is also not opt-in right. Does it imply that all helm based deployments need to point to the new endpoint? (after building with a new alfresco image, or just rebuilding in general.) ALFRESCO_DEFAULT_LIVE_PROBE (runs on 8090 now?)

The HealthCheck with the ALFRESCO_DEFAULT_LIVE_PROBE will be used by the tomcat image's default docker HEALTHCHECK instruction. For helm deployments this will not be used as such, but I was planning on also changing the default liveness check in our alfresco helm chart to the new endpoint. That default is currently an http check on the 8080 port for /alfresco/api/-default-/public/alfresco/versions/1/probes/-live-.

[Brobrechts]

Problem is disparity between xenit-docker images and building containers,

merging to master implies we are overriding tags,

And most if not all our images rely on this (not ideal, I know).
baseImage = "docker.io/xenit/alfresco-repository-skeleton:${alfrescoVersionShort}"

alfresco-repository-skeleton is the one pushed by this repo I think,

I don't want to be that guy but this has the potential to break things, or I'm overlooking the fix in the default case.

Or let me rephrase, of someone rebuilds an image tomorrow, will they have to change something in their healthchecks?

[evyrosseels]

@Brobrechts Not at all, these are all valid points.
I hadn't considered a deadlock situation. I'm not sure what would happen in that case.
Besides that, I don't see this change breaking anything because the probe endpoint will still work whether you call it via the old port or the new one.
To be safe I can switch the default healthcheck port back, and the separate thread pool will just be available but unused by default?