Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebReactiveOptions support for maxIdleTime and maxLifeTime #559

Closed
mnbattaglia opened this issue Mar 29, 2023 · 6 comments
Closed

WebReactiveOptions support for maxIdleTime and maxLifeTime #559

mnbattaglia opened this issue Mar 29, 2023 · 6 comments

Comments

@mnbattaglia
Copy link

mnbattaglia commented Mar 29, 2023

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
Copy link
Author

Please, also consider adding the metrics attribute as well

@jchrobert
Copy link

Might relate to this reactor/reactor-netty#1774

@mnbattaglia
Copy link
Author

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

kptfh pushed a commit that referenced this issue Apr 7, 2023
WebReactiveOptions support for maxIdleTime and maxLifeTime
@kptfh kptfh closed this as completed in 8417772 Apr 7, 2023
@mnbattaglia
Copy link
Author

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

kptfh pushed a commit that referenced this issue Apr 7, 2023
WebReactiveOptions support metrics
kptfh pushed a commit that referenced this issue Apr 7, 2023
WebReactiveOptions support metrics
kptfh pushed a commit that referenced this issue Apr 7, 2023
WebReactiveOptions support metrics
@kptfh
Copy link
Collaborator

kptfh commented Apr 7, 2023

next week

@kptfh
Copy link
Collaborator

kptfh commented Apr 15, 2023

please check 3.2.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants