SOLR-17433: Set default http request timeout to infinite

[VishnuPriyaChandraSekar]

Description

In earlier versions of SolrJ 9.4, the legacy HttpSolrClient was able to support long running streaming operation (> 10 mins). However, in SolrJ 9.4+, the http client throws exception exactly after 10 mins.

https://issues.apache.org/jira/browse/SOLR-17433

Solution

In case of long streaming operation, the server sends the response in chunks and request timeout determines how long the client can wait to receive all the chunks. The default request timeout was set to 10 mins which caused the streaming operations to timeout abruptly.

In order to support long running streams, the request timeout must be infinite (as we don't know how long streams will take to complete). Jetty excepts either 0 or -1 however JDK expects no value to represent the infinite timeout. The following changes were made to implement the behavior

• HttpSolrClient: Set the default request timeout to infinite by setting it to zero. This helps streams to continue streaming until they exhaust the content.
• HttpJdkSolrClient: Prevented passing over the request timeout to JDK's HttpClient if it is zero.

Tests

• Updated ConcurrentUpdateSolrClientTestBase.testSocketTimeoutOnCommit to explicitly set a request timeout. The test previously relied on the default 1 ms request timeout (from idle timeout) to trigger an HTTP client timeout. After the default timeout was changed to infinite, the client no longer timed out, causing the test's 10-second timeout to fail first. This change restores the intended test behavior by overriding the request timeout explicitly.
• Updated HttpJdkSolrClientTest.testTimeout to explicitly set a request timeout. Previously, request timeout was based on idle timeout. After the infinite request timeout, the client no longer timed out. Fixed the test by overriding the request timeout explicitly
• Updated LB2SolrClientTest.testTimeoutExceptionMarksServerAsZombie to explicitly set a request timeout as the default value causes the client to wait infinitely.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• [X ] I have added tests for my changes.
• I have added documentation for the Reference Guide
• I have added a changelog entry for my change

[dsmiley]

use fewer words please (one line)

[dsmiley]

Also I'm confused. You're implying that the idle timeout isn't honored if the request timeout is infinite?

[VishnuPriyaChandraSekar]

The idle timeout provided in the tests was ineffective for JDK and Jetty clients.
In case of Jetty, the idle timeout is hardcoded as infinite (not sure whether this is a bug or intended behavior). While in JDK, we are not even passing the override idle Time out to the JDK http client (looks like JDK does not support configuring idle timeout using builder).

[dsmiley]

You quote the Jetty timeout as infinite but if you see the comment there (I wrote that), the timeout is applied instead at a request level (instead of at the HttpClient level).

Oh right, JDK HttpClient doesn't support idle timeout. Therefore we can't have a test using the Jdk HttpClient that specifically relates to idle timeout testing (unsupported).

[VishnuPriyaChandraSekar]

You are right. Looks like Jetty timeout is set in the request. Let me fix the tests and submit a new revision

[dsmiley]

This should probably just be 0, infinite?

[dsmiley]

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]

idle refers to the longest period of not receiving any data at all from the server.

My understanding is that idle timeout kicks in after processing response from server and before initiating the next request. Thus idle timeout makes sense when the socket is reused for another HTTP request. While request timeout kicks in when client sends a request and waits for server to respond.

The test case that uses TimeoutZombieTestContext makes a single http request. Thus idle timeout never comes into the picture. Additionally, the server that is mocked for this test case, never responds to the request and keeps the client stuck infinitely. Thus, in this case, the request timeout should be > 0

public void testTimeoutExceptionMarksServerAsZombie() throws Exception {
    try (TimeoutZombieTestContext ctx = new TimeoutZombieTestContext()) {
      LBSolrClient.Req lbReq = ctx.createQueryRequest();

      try {
        ctx.lbClient.request(lbReq);
      } catch (Exception e) {
      }

      ctx.assertZombieState();
    }
  }

[dsmiley]

see this line ;-)

[dsmiley]

Strangely... it seems this PR/branch now has several individual commits copied from main branch. If you intended to resync this PR with main, you simply merge main into your feature branch. To remedy this, simply do that now.