Fix client thread leak after origin reload.

[mikkokar]

This pull request addresses two issues:

A thread leak associated with origin reloads.

The Styx proxy.clientWorkerThreadsCount setting was not honoured. Styx is supposed to have only one client thread pool that is shared among all backend services and proxy.clientWorkderThradsCount is the number of threads in this pool.

Due to an error in styx code, this pool was instantiated once per each Styx backend service, resulting in a high number of client threads. More than configuration allowed.

Secondly, this resulted a thread leak. When new backend services were added, additional thread pools were being created. Because the thread pool was supposedly shared, it was never closed when a backend service was removed.

This PR ensures that the client thread pool is shared, and not separately created for each new backend service. I am also adding an integration test to ensure this leak won't happen in future.

A "global" event loop for StyxHttpClient.

This PR also adds a "global" event loop for StyxHttpClient. It lives in a static context processes all IO events for StyxHttpClient. Benefit is that a client is always ready to be used and doesn't need to be closed afterward. For example:

val client = StyxHttpClient.Builder().build()

client.send(get("/")...build())
    .wait()
    .status() shouldBe OK

# When using the global event loop it is no longer necessary to close
# the client afterwards. Hence the following line can be removed:
client.close()

As styx health check functionality uses this client, the health check IO events also go on to this global event loop.

The global pool size is derived from the CPU core count: two threads per CPU core.

[dvlato]

I don't know if I understand this code correctly: if the eventLoopGroup has been injected externally (and thus could be potentially shared among several different connection factories) we close it here. That doesn't seem correct.
Also we are initializing a Default Global Event Loop that's not going to be used in most cases.

[mikkokar]

Your criticism is justified. Let me see if the event loop can be closed outside.

[mikkokar]

Reading your comment again, and looking at code, I notice that eventLoopGroup is not really injected. It is created with the factory that has been externally injected.

But with memoization it may have the effect you describe: The same eventLoopGroup could end up being shared with different connection factories, and one of the factories may end up closing it on behalf of others.

This feels an incorrect usage of NettyConnectionFactory rather than an error in the NettyConnectionFactory itself.

[dvlato]

Oh, sure, that's right. In this case we might be safe ourselves as we don't try to close the connectionFactory except for healthchecks - and the StyxHttpClient used in healthchecks is using a different eventLoopGroup than the forwarding path... But it seems it could lead to some issues.

[kvosper]

Perhaps we should add comments to the javadoc explaining what pitfalls one should avoid when using the class, so misuse can be prevented?

[mikkokar]

Please check this commit: a5d70cd.

It is a different approach and removes the close method.

Now the Netty eventLoopGroup and accompanying channel type are injected from outside. They are never closed. The client event loop group is created when styx starts and stays alive until shut.

The Styx health checks & Styx standalone client are now permanently fixed on "global" event loop group without option to inject its own. This can be improved if necessary.

[kvosper]

Perhaps we should add comments to the javadoc explaining what pitfalls one should avoid when using the class, so misuse can be prevented?