Skip to content

Disable Reactor Netty auto-retry on HttpClient to prevent duplicate P… #488

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.

Sign up for GitHub

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

Open
dxichen wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −3
Open

Disable Reactor Netty auto-retry on HttpClient to prevent duplicate P… #488

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions client/secureclient/src/main/java/com/linkedin/openhouse/client/ssl/WebClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ private HttpClient createHttpClient(String baseUrl) {
HttpClient client = null;
if (HttpConnectionStrategy.NEW.equals(strategy)) {
log.info("Using new connection strategy");
client = HttpClient.newConnection();
client = HttpClient.newConnection().disableRetry(true);
} else {
log.info("Using connection pool strategy");
client = HttpClient.create(getCustomConnectionProvider());
client = HttpClient.create(getCustomConnectionProvider()).disableRetry(true);
Copy link
Member

@abhisheknath2011 abhisheknath2011 Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This config is for pooled connection and not at the request level. Normally Reactor Netty may retry once automatically when:

  • A pooled connection becomes stale
  • Connection closed before request write
  • TCP reset during acquisition
  • Connection closed by server before request starts.

Ideally on 200 or 409 Netty should not retry. In which scenario you are observing the issue?

Copy link
Member

@abhisheknath2011 abhisheknath2011 Mar 6, 2026

Choose a reason for hiding this comment

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

As this is common client side http connection pool config, changing default netty behaviour might have some consequences. Disabling netty retry can increase:

  • 503 errors.
  • connection reset
  • PrematureCloseException
  • ClosedChannelException

Copy link
Member

@abhisheknath2011 abhisheknath2011 Mar 6, 2026

Choose a reason for hiding this comment

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

This code is common and not only used across openhouse codebase, but also in client side.

}
return client;
}
Expand All @@ -144,11 +144,15 @@ private HttpClient createSecureHttpClient(String truststoreLocation) {
HttpClient client = null;
if (HttpConnectionStrategy.NEW.equals(strategy)) {
log.info("Using new connection strategy");
client = HttpClient.newConnection().secure(t -> t.sslContext(createSslContext(truststore)));
client =
HttpClient.newConnection()
.disableRetry(true)
.secure(t -> t.sslContext(createSslContext(truststore)));
} else {
log.info("Using connection pool strategy");
client =
HttpClient.create(getCustomConnectionProvider())
.disableRetry(true)
.secure(t -> t.sslContext(createSslContext(truststore)));
}

Expand Down