Skip to content

Add config for http client connect timeout#16831

Merged
abhishekagarwal87 merged 2 commits intoapache:masterfrom
a2l007:clientconnect
Aug 7, 2024
Merged

Add config for http client connect timeout#16831
abhishekagarwal87 merged 2 commits intoapache:masterfrom
a2l007:clientconnect

Conversation

@a2l007
Copy link
Copy Markdown
Contributor

@a2l007 a2l007 commented Aug 1, 2024

Description

Adds a configuration clientConnectTimeout to our http client config which controls the connection timeout for our http client requests.

It was observed that on busy K8S clusters, the default connect timeout of 500ms is sometimes not enough time to complete syn/acks for a request and in these cases, the requests timeout with the error:
exceptionType=java.net.SocketTimeoutException, exceptionMessage=Connect Timeout
This behavior was mostly observed on the router while forwarding queries to the broker.
Having a slightly higher connect timeout helped resolve these issues.

####Release Note:
Adds http client config clientConnectTimeout to configure connection timeouts for Druid http client requests.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • been tested in a test Druid cluster.

httpClient.setMaxConnectionsPerDestination(config.getNumConnections());
httpClient.setMaxRequestsQueuedPerDestination(config.getNumRequestsQueued());
httpClient.setConnectTimeout(CLIENT_CONNECT_TIMEOUT_MILLIS);
httpClient.setConnectTimeout(config.getClientConnectTimeout());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to remove CLIENT_CONNECT_TIMEOUT_MILLIS variable as well

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@abhishekagarwal87 abhishekagarwal87 merged commit 76ad17f into apache:master Aug 7, 2024
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants