WebReactiveOptions support for maxIdleTime and maxLifeTime

[mnbattaglia]

We recently found some issues where, after being idle for a while, our next request throws a WebClientRequestException: readAddress(..) failed: Connection reset by peer exception. It seems that the connection pool is trying to reuse a connection already closed by the server but the pool is not aware of it. There could be several reasons for it, but the suggestion everywhere is to set the maxIdleTime to the pool so those idle connections are discarded before being disconnected from the server.

This configuration happens at the ConnectionProvider. There are some netty system properties that can be set as default values, but we want to set this programatically. There is no way to configure this in Feign Reactive if we use the default implementation of the ClientHttpConnector created by NettyClientHttpConnectorBuilder. There are other parameters that can be set to the ConnectionProvider via WebReactiveOptions (eg: maxConnections, pendingAcquireMaxCount, etc.) but maxIdleTime and maxLifeTime are missing there.

As a workaround, we can provide our own implementation of a HttpClient with that configured and pass it to the WebReactiveFeign.Builder when creating our instance. So, we can just copy/paste the whole NettyClientHttpConnectorBuilder.buildNettyClientHttpConnector() logic (HttpClient instantiation part) and add the needed configuration when ConnectionProvider.Builder is created.

So, ideally, maxIdleTime and maxLifeTime should be added to WebReactiveOptions and properly set in the NettyClientHttpConnectorBuilder.buildNettyClientHttpConnector so we can configure these properties as there are lot of reports of this error and similar ones. In fact, I think the one reported in #552 can be fixed by setting the maxIdleTime variable.

Thanks and happy to collab!

[mnbattaglia]

Please, also consider adding the metrics attribute as well

[jchrobert]

Might relate to this reactor/reactor-netty#1774

[mnbattaglia]

@jchrobert it is related indeed. That's why we need to be able to set those props. via feign configuration

[mnbattaglia]

Thanks @kptfh to get this fixed so quickly. When is the release of next version including these changes planned?

[kptfh]

next week

[kptfh]

please check 3.2.10

[zBo1997]

@mnbattaglia Oh~ I'm sorry to raise the question again after a long time has passed regarding this matter，I have also discovered a similar issue . May I ask if you can provide logic of "NettyClientHttpConnectorBuilder.buildNettyClientHttpConnector() "
I will be extremely grateful

[kptfh]

Hi. Feel free to make PR.

…

On Fri, 13 Sep 2024, 14:30 zBo1997, ***@***.***> wrote: @mnbattaglia <https://github.com/mnbattaglia> Oh~ I'm sorry to raise the question again after a long time has passed regarding this matter，I have also discovered a similar issue . May I ask if you can provide logic of "NettyClientHttpConnectorBuilder.buildNettyClientHttpConnector() " I will be extremely grateful — Reply to this email directly, view it on GitHub <#559 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADYRISXQLNYHVWMAB6GYN43ZWLEDPAVCNFSM6AAAAABOFFWH4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBYG4YTIOJVGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>