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

RestTemplateEurekaHttpClient not closing HttpClient on shutdown #4103

Open
BenEfrati opened this issue Jun 15, 2022 · 15 comments · Fixed by #4258
Open

RestTemplateEurekaHttpClient not closing HttpClient on shutdown #4103

BenEfrati opened this issue Jun 15, 2022 · 15 comments · Fixed by #4258
Assignees
Labels

Comments

@BenEfrati
Copy link

BenEfrati commented Jun 15, 2022

Spring Cloud 2021.0.0
Spring Boot 2.6.8

Bug Description:
RestTemplateEurekaHttpClient is not closing HttpClient on shutdown. This leads to TCP CLOSE_WAIT connections to eureka server.

EurekaClient will shutdown when an exception occurs on an http request, but not shutdown HttpClient.

This bug is related to

In case of exception here:
https://github.com/Netflix/eureka/blob/ed0da19ca1c049c87e3dbf75b6015c1861d5c2d0/eureka-client/src/main/java/com/netflix/discovery/shared/transport/decorator/RedirectingEurekaHttpClient.java#L96
new HttpClient will be created without closing the existing one - this causes CLOSE_WAIT connections

This supplier creates new CloseableHttpClient for every call to

public class DefaultEurekaClientHttpRequestFactorySupplier implements EurekaClientHttpRequestFactorySupplier {

	@Override
	public ClientHttpRequestFactory get(SSLContext sslContext, @Nullable HostnameVerifier hostnameVerifier) {
		HttpClientBuilder httpClientBuilder = HttpClients.custom();
		if (sslContext != null) {
			httpClientBuilder = httpClientBuilder.setSSLContext(sslContext);
		}
		if (hostnameVerifier != null) {
			httpClientBuilder = httpClientBuilder.setSSLHostnameVerifier(hostnameVerifier);
		}
		CloseableHttpClient httpClient = httpClientBuilder.build();
		HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory();
		requestFactory.setHttpClient(httpClient);
		return requestFactory;
	}

}

so in case of shutdown, currentEurekaClient shutdown don't closes connections:

possible solution will be trying to close the HttpClient:
shutdown could be

public void shutdown() {
        Optional.of(unwrapRequestFactoryIfNecessary(restTemplate.getRequestFactory()))
                .filter(HttpComponentsClientHttpRequestFactory.class::isInstance)
                .map(HttpComponentsClientHttpRequestFactory.class::cast)
                .ifPresent(requestFactory-> {
                    try {
                        requestFactory.destroy();
                    } catch (Exception e) {
                    }
                });
    }

unwrapRequestFactoryIfNecessary
https://github.com/spring-projects/spring-boot/blob/47516b50c39bd6ea924a1f6720ce6d4a71088651/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java#L746

spring-projects/spring-boot#31075

As you can see process open files increases over time until GC occurred
image

image

We can also create EurekaClientHttpRequestFactorySupplier which return the same ClientHttpRequestFactory, but it not a stable solution since we can't control eureka client code, maybe in case of exceptions connections not closes (hence not returns to PoolingHttpClientConnectionManager) - this can lead to no available connections in pool. In that case, restart is required

Original Issue:
#4062

@BenEfrati
Copy link
Author

spring-projects/spring-boot#31075 (comment)

the piece code instantiating the request factory should be in charge of closing resources properly

@OlgaMaciaszek
Copy link
Collaborator

Hello, @BenEfrati , thanks for reporting it. Looks like a bug.

@sdzx3783
Copy link

sdzx3783 commented Oct 28, 2022

Recently, I encountered this problem in the production environment. I found that the HttpClient was created many times. The HttpClient connection pool did not enable the thread for clearing expired connections. The problem was solved by sharing one HttpClient and enabling the IdleConnectionEvaluator thread. The code is as follows:

@Configuration
public class EurekaConfig {

    private final AtomicReference<CloseableHttpClient> ref = new AtomicReference<>();

    @Bean
    public EurekaClientHttpRequestFactorySupplier defaultEurekaClientHttpRequestFactorySupplier() {
        return (sslContext, hostnameVerifier) -> {
            HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory();
            HttpClientBuilder httpClientBuilder = HttpClients
                    .custom()
                    .evictExpiredConnections()
                    .evictIdleConnections(30L, TimeUnit.SECONDS);
            if (sslContext != null) {
                httpClientBuilder = httpClientBuilder.setSSLContext(sslContext);
            }
            if (hostnameVerifier != null) {
                httpClientBuilder = httpClientBuilder.setSSLHostnameVerifier(hostnameVerifier);
            }
            if (ref.get() == null) {
                ref.compareAndSet(null, httpClientBuilder.build());
            }
            requestFactory.setHttpClient(ref.get());
            return requestFactory;
        };
    }
}
`

@OlgaMaciaszek
Copy link
Collaborator

Hello @sdzx3783 - thanks for your comment. Would you like to submit a PR with a fix?

@sdzx3783
Copy link

sdzx3783 commented Nov 3, 2022

I provide a temporary solution without changing the source code

@dangkhoaphung
Copy link

Hi @OlgaMaciaszek , may I know which version will include the fix?

@OlgaMaciaszek
Copy link
Collaborator

@dangkhoaphung if the issue is added to an active backlog, it will be assigned a milestone. We're now prioritising tasks for the upcoming release and will circle back to non-blocking issues later on. However, feel free to create a PR, and we will definitely review it.

@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2022.0.5 Dec 18, 2023
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.1 Dec 18, 2023
@OlgaMaciaszek OlgaMaciaszek added this to the 4.0.4 milestone Dec 18, 2023
@spencergibb spencergibb modified the milestones: 4.0.4, 4.1.1 Feb 7, 2024
@spencergibb spencergibb removed this from 2022.0.5 Feb 7, 2024
ZIRAKrezovic added a commit to ZIRAKrezovic/spring-cloud-netflix that referenced this issue Feb 19, 2024
ZIRAKrezovic added a commit to ZIRAKrezovic/spring-cloud-netflix that referenced this issue Mar 26, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2023.0.1 Mar 26, 2024
OlgaMaciaszek added a commit that referenced this issue Apr 15, 2024
@OlgaMaciaszek OlgaMaciaszek reopened this Apr 15, 2024
@OlgaMaciaszek
Copy link
Collaborator

Initial fix reverted due to #4275. Reopening.

@OlgaMaciaszek OlgaMaciaszek reopened this Apr 15, 2024
@OlgaMaciaszek OlgaMaciaszek removed this from the 4.1.1 milestone Apr 15, 2024
@impactCn
Copy link

Spring Cloud eureka client 4.1.3
Spring Boot 3.3.1

hi, I upgraded to 4.1.3 and still have a lot of close_wait problems.
@OlgaMaciaszek

Image

@impactCn
Copy link

Spring Cloud eureka client 4.1.3 Spring Boot 3.3.1

hi, I upgraded to 4.1.3 and still have a lot of close_wait problems. @OlgaMaciaszek

Image

Is there any plan to fix this issue in which version?

@OlgaMaciaszek
Copy link
Collaborator

OlgaMaciaszek commented Jan 16, 2025

Yes, @impactCn. the initial fix had to be reverted due to regression. We'll need to work on another fix, however it's not yet assigned to a new backlog. We some higher priority task right now, but I will try to look into it next month.

@impactCn
Copy link

@OlgaMaciaszek I have a temporary solution to solve this problem, using webflux's webclient as the caller. I don't seem to have this problem in the test environment. I'll test it in the production environment tomorrow and take a look.

@OlgaMaciaszek
Copy link
Collaborator

Sounds good @impactCn. Please take a look at that regression linked above as well to see if your solution is not doing the same. If it works fine, please feel free to create a PR or share the workaround.

@impactCn
Copy link

eureka:
  client:
    webclient:
      enabled: true

@OlgaMaciaszek
I used the above configuration and just observed it in the production environment for 30 minutes. There is no close_wait problem.
And, I'll put it to tomorrow and see if there will be any problems with close_wait.

@OlgaMaciaszek
Copy link
Collaborator

Ah, ok - that's a different thing - it's just using WebClient instead of HttpClient, but please verify if all your use-cases work, as the scope of integration (i.e. TransportFactories implementation) was not full for WebClient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants