Skip to content

SOLR-17433: Set default http request timeout to infinite #4626

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
VishnuPriyaChandraSekar wants to merge 10 commits into
base: main
Choose a base branch
from
Open

SOLR-17433: Set default http request timeout to infinite #4626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0a475cf
SOLR-17433: Set default http request timeout to infinite
VishnuPriyaChandraSekar Jul 7, 2026
08359ce
Remove ASF License headers when possible. (#4609)
dsmiley Jul 9, 2026
bca4e41
SOLR-18197: Add root document query shortcut support to NestPathField…
abumarjikar Jul 9, 2026
739f3e9
Removed the comments in the changelog
VishnuPriyaChandraSekar Jul 9, 2026
206d1df
Merge branch 'apache:main' into SOLR-1743-changed-default-request-tim…
VishnuPriyaChandraSekar Jul 9, 2026
898166d
Merge branch 'apache:main' into SOLR-1743-changed-default-request-tim…
VishnuPriyaChandraSekar Jul 10, 2026
52bad08
ConcurrentUpdateJettySolrClientTest: don't set request timeout
dsmiley Jul 11, 2026
e5c4d0b
HttpJdkSolrClientTest: remove testTimeout()
dsmiley Jul 11, 2026
99fe3d1
Merge branch 'apache:main' into SOLR-1743-changed-default-request-tim…
VishnuPriyaChandraSekar Jul 13, 2026
f91fc5e
Apply suggestion from @dsmiley
VishnuPriyaChandraSekar Jul 22, 2026
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
9 changes: 9 additions & 0 deletions changelog/unreleased/SOLR-17433-changed-default-request-timeout.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
title: >
SolrClients no longer have a default request timeout. It was problematic with streaming expressions.
The idle timeout remains, albeit the JDK HttpClient doesn't support that (has none).
type: changed
authors:
- name: Vishnu Priya Chandra Sekar
links:
- name: SOLR-17433
url: https://issues.apache.org/jira/browse/SOLR-17433
3 changes: 2 additions & 1 deletion ...etty/src/test/org/apache/solr/client/solrj/jetty/ConcurrentUpdateJettySolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public ConcurrentUpdateBaseSolrClient outcomeCountingConcurrentClient(
public HttpSolrClient solrClient(Integer overrideIdleTimeoutMs) {
var builder = new HttpJettySolrClient.Builder();
if (overrideIdleTimeoutMs != null) {
builder.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
builder
.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
}
return builder.build();
}
Expand Down
5 changes: 1 addition & 4 deletions solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,10 +671,7 @@ public void testBuilder() {
public void testIdleTimeoutWithHttpClient() throws Exception {
String url = solrTestRule.getBaseUrl() + SLOW_STREAM_SERVLET_PATH;
try (var oldClient =
new HttpJettySolrClient.Builder(url)
.withRequestTimeout(Long.MAX_VALUE, TimeUnit.MILLISECONDS)
.withIdleTimeout(100, TimeUnit.MILLISECONDS)
.build()) {
new HttpJettySolrClient.Builder(url).withIdleTimeout(100, TimeUnit.MILLISECONDS).build()) {

try (var onlyBaseUrlChangedClient =
new HttpJettySolrClient.Builder(url).withHttpClient(oldClient).build()) {
Expand Down
5 changes: 4 additions & 1 deletion solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,10 @@ private synchronized boolean maybeTryHeadRequestSync(URI uriNoQueryParams) {
}

private void decorateRequest(HttpRequest.Builder reqb, SolrRequest<?> solrRequest) {
reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS));
// JDK does not allow non-positive value for request timeout.
if (requestTimeoutMillis > 0) {
reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS));
}

reqb.header("User-Agent", USER_AGENT);
setBasicAuthHeader(solrRequest, reqb);
Expand Down
12 changes: 9 additions & 3 deletions solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,10 +580,16 @@ public B withRequestTimeout(long requestTimeout, TimeUnit unit) {
return (B) this;
}

/**
* Returns the request timeout in milliseconds.
*
* <p>If no request timeout is configured or if the configured value is non-positive, this
* method returns {@code 0}, indicating that requests will wait infinitely for a response.
*
* @return request timeout in milliseconds or {@code 0} if no valid timeout is configured.
*/
public long getRequestTimeoutMillis() {
return requestTimeoutMillis != null && requestTimeoutMillis > 0
? requestTimeoutMillis
: getIdleTimeoutMillis();
return requestTimeoutMillis != null && requestTimeoutMillis > 0 ? requestTimeoutMillis : 0;
}

/**
Expand Down
6 changes: 5 additions & 1 deletion solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateJdkSolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ public HttpSolrClient solrClient(Integer overrideIdleTimeoutMs) {
var builder =
new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT);
if (overrideIdleTimeoutMs != null) {
builder.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
builder
.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS)
// override the infinite request timeout with idle timeout to ensure idle requests times
// out.
Comment thread
VishnuPriyaChandraSekar marked this conversation as resolved.
.withRequestTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);

@dsmiley dsmiley Jul 9, 2026

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.

This should probably just be 0, infinite?

@VishnuPriyaChandraSekar VishnuPriyaChandraSekar Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By default the request timeout is infinite which causes the test to hang forever. Thus, I had to change it to a definite value.

}
return builder.build();
}
Expand Down
15 changes: 2 additions & 13 deletions solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ public void testRequestWithBaseUrl() throws Exception {
client.requestWithBaseUrl(intendedUrl, new QueryRequest(q, SolrRequest.METHOD.GET), null);
assertEquals(
client.getParser().getWriterType(), DebugServlet.parameters.get(CommonParams.WT)[0]);
// verify whether the default request timeout is infinite.
assertEquals(0, client.requestTimeoutMillis);
}
}

Expand Down Expand Up @@ -206,19 +208,6 @@ public void testAsyncException() throws Exception {
super.testAsyncExceptionBase();
}

@Test
public void testTimeout() throws Exception {
SolrQuery q = new SolrQuery("*:*");
try (HttpJdkSolrClient client =
(HttpJdkSolrClient)
builder(solrTestRule.getBaseUrl() + SLOW_SERVLET_PATH, 500, 500).build()) {
client.query(q, SolrRequest.METHOD.GET);
fail("No exception thrown.");
} catch (SolrServerException e) {
assertTrue(e.getMessage().contains("timeout") || e.getMessage().contains("Timeout"));
}
}

@Test
public void test0IdleTimeout() throws Exception {
SolrQuery q = new SolrQuery("*:*");
Expand Down
3 changes: 3 additions & 0 deletions solr/solrj/src/test/org/apache/solr/client/solrj/impl/LB2SolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ private class TimeoutZombieTestContext implements AutoCloseable {
new HttpJettySolrClient.Builder()
.withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
.withIdleTimeout(1, TimeUnit.MILLISECONDS)
// override the infinite request timeout with idle timeout to ensure idle requests
// times out.
Comment on lines +408 to +409

@dsmiley dsmiley Jul 9, 2026

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.

something seems suspicious... the idle timeout should work if the request timeout is infinite. They are different things; idle refers to the longest period of not receiving any data at all from the server. The request timeout is an overall timeout for the entire request.

@VishnuPriyaChandraSekar VishnuPriyaChandraSekar Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idle timeout is ineffective for this test. The http client (through Jetty) uses HTTP2 protocol to communicate with the server. After establishing connection with the server, it tries to send the request headers to the server as part of creating HTTP streams. However, the fake server fails to ack the headers.
The client is waiting infinitely for the server to ack and the connection never times out because of following reasons

  • The idle timeout in the request (1ms) is not applied as the Jetty applies it only after receiving ack from the server [1]. The default idle timeout (-1) set in the HttpJettySolrClient is applied [2]
  • The default request timeout was changed to -1

I was wondering how the test worked before. It looks like the test wanted to simulate socket timeout, however it was not happening. The HTTP request was timing out because of request timeout (previously it took idle timeout as default value.)

In order to fix the test, either the fake server must ack the header (Gemini says it is possible however I haven't tried first hand) or simulate request timeout instead. Let me know which one choose

Thanks to the Jetty logs [3] for helping me to understand this.

[1] https://github.com/jetty/jetty.project/blob/jetty-12.1.x/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpSenderOverHTTP2.java#L203
[2] https://github.com/apache/solr/blob/main/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java#L304
[3] Jetty logs: https://gist.github.com/VishnuPriyaChandraSekar/0e3d428cbfdd7025c18562356af7da4d

@dsmiley dsmiley Jul 18, 2026

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.

RE "this test": LB2SolrClientTest is a test suite with a number of tests, and this comment thread is on a general part of the test suite. Hopefully the "connect" timeout ones are valid; yes? So I suppose your input is that the "testTimeoutExceptionMarksServerAsZombie" test and Async one are flawed?

If the default idle timeout of -1 is actually used as such for the first response, even in the beginning... this is a problem IMO. If we have the intended/desired idle timeout then we can use that. If we don't, I suppose the connection timeout would be a reasonable substitute.
Obviously what we do will need some comments to help anyone studying the internals understand.

@VishnuPriyaChandraSekar VishnuPriyaChandraSekar Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"this test": LB2SolrClientTest is a test suite with a number of tests, and this comment thread is on a general part of the test suite. Hopefully the "connect" timeout ones are valid; yes? So I suppose your input is that the "testTimeoutExceptionMarksServerAsZombie" test and Async one are flawed?

@dsmiley The issue only affects testTimeoutExceptionMarksServerAsZombieAsyncRequest. Interestingly, the synchronous counterpart, testTimeoutExceptionMarksServerAsZombie, does throw a TimeoutException.

After digging into the code, it looks like the difference is that the synchronous API waits for the response headers using listener.get(idleTimeoutMillis, TimeUnit.MILLISECONDS), where idleTimeoutMillis comes from the request rather than the default infinite idle timeout set at transport layer [1]. As a result, even though the transport layer is configured with an infinite idle timeout, the application-layer timeout prevents the request from hanging indefinitely.

The async API, on the other hand, doesn't have an equivalent application-layer timeout and instead relies on the transport layer to trigger onHeaders() or onFailure() [2]. Since the transport idle timeout isn't applied at this stage, the future never completes and the test hangs.

[1] https://github.com/apache/solr/blob/main/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java#L484
[2] https://github.com/apache/solr/blob/main/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java#L402

If the default idle timeout of -1 is actually used as such for the first response, even in the beginning... this is a problem IMO. If we have the intended/desired idle timeout then we can use that. If we don't, I suppose the connection timeout would be a reasonable substitute.

Maybe we should a have timeout at the application layer like the sync call ? (Not sure whether it is the right PR to fix the async call as it seem to be a different problem)

Apologizes, if my findings weren't complete earlier. I found it hard to debug as the timeout was triggered at different layer based on the request and also tracing Java futures was hard. I hope now it make sense.

@VishnuPriyaChandraSekar VishnuPriyaChandraSekar Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think, it makes sense to set the idle timeout to a definite value. The problem will be solved without needing to add timeout to the futures that handles async request.

.withRequestTimeout(1, TimeUnit.MILLISECONDS)
.build();

lbClient = new LBJettySolrClient.Builder(delegateClient, nonRoutableEndpoint).build();
Expand Down
Loading