Support max lifetime on HTTP Client pool

[tsegismont]

Closes #5360

It would help in some cases:

• make sure a connection does not live longer than authorized by some firewall
• give an opportunity to the client to find new backend replicas started after the pool reached its maximum size

[tsegismont]

@vietj I would like to rebase this PR but can you tell me if you have any concern with it before?

[NilsRenaud]

Sorry I've missed to subscribe to this issue, so I'm a bit late on this one...

But I see an issue with the fix: when the maxLifetime threshold is reached the connection gets closed instead of shutdowned with a grace period.
Closing the connection is brutal and will break the requests/responses happening on it without any notice. It would have been cleaner to have an eviction period, waiting for requests to complete during X sec but not accepting new one in the meantime to reduce the risk of killing healthy HTTP traffic.

closing the connection was the right move when dealing with KeepAliveTimeout but not when dealing with the lifetime of a connection.

What do you think ?

[vietj]

@NilsRenaud sounds like a good idea

[NilsRenaud]

Thanks @vietj , what should we do next then ? re-open the issue ? Create another one as "improvement" ?

[vietj]

open an issue with a precise description

[tsegismont]

@NilsRenaud I'm not sure what you mean, HttpConnectionInternal::isValid is the predicate provided to io.vertx.core.internal.pool.ConnectionPool#evict(java.util.function.Predicate<C>).

The doc for this method reads:

Evict connections from the pool with a {@code predicate}, only unused connection can be evicted.

[NilsRenaud]

Good point ! I missed it, thanks for your answer @tsegismont

[NilsRenaud]

That being said, if a request on this connection hangs forever, the connection will never be closed, right ?

[tsegismont]

Yeah, if connections are constantly busy, the max lifetime might not be honored

[NilsRenaud]

I'm wondering whether this could be a problem 🤔
Could this potentially infinite connection lifetime be abused ?

[tsegismont]

I guess it could, but I'd rather not fabricate a use case. If somebody reports an actual problem, we'll look into it.