[SOLR-13605] HttpSolrClient.Builder.withHttpClient() is useless for the purpose of setting client scoped so/connect timeouts

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: main (10.0), 9.3
• Assignee: Eric Pugh
• Reporter: Chris M. Hostetter
• Created: 03/Jul/19 21:38
• Updated: 21/Jul/23 20:40
• Resolved: 03/May/23 10:28

Description

TL;DR: trying to use HttpSolrClient.Builder.withHttpClient is useless for the the purpose of specifying an HttpClient with the default "timeouts" you want to use on all requests, because of how HttpSolrClient.Builder and HttpClientUtil.createDefaultRequestConfigBuilder() hardcode values thta get set on every HttpRequest.

This internally affects code that uses things like UpdateShardHandler.getDefaultHttpClient(), UpdateShardHandler.getUpdateOnlyHttpClient() UpdateShardHandler.getRecoveryOnlyHttpClient(), etc...

---

While looking into the patch in SOLR-13532, I realized that the way HttpSolrClient.Builder and it's super class SolrClientBuilder work, the following code doesn't do what a reasonable person would expect...

SolrParams clientParams = params(HttpClientUtil.PROP_SO_TIMEOUT, 12345,
                                 HttpClientUtil.PROP_CONNECTION_TIMEOUT, 67890);
HttpClient httpClient = HttpClientUtil.createClient(clientParams);
HttpSolrClient solrClient = new HttpSolrClient.Builder(ANY_BASE_SOLR_URL)
        .withHttpClient(httpClient)
        .build();

When solrClient is used to execute a request, neither of the properties passed to HttpClientUtil.createClient(...) will matter - the HttpSolrClient.Builder (via inheritence from SolrClientBuilder has the following hardcoded values...

  // SolrClientBuilder
  protected Integer connectionTimeoutMillis = 15000;
  protected Integer socketTimeoutMillis = 120000;

...which unless overridden by calls to withConnectionTimeout() and withSocketTimeout() will get set on the HttpSolrClient object, and used on every request...

    // protected HttpSolrClient constructor
    this.connectionTimeout = builder.connectionTimeoutMillis;
    this.soTimeout = builder.socketTimeoutMillis;

It would be tempting to try and do something like this to work around the problem...

SolrParams clientParams = params(HttpClientUtil.PROP_SO_TIMEOUT, 12345,
                                 HttpClientUtil.PROP_CONNECTION_TIMEOUT, 67890);
HttpClient httpClient = HttpClientUtil.createClient(clientParams);
HttpSolrClient solrClient = new HttpSolrClient.Builder(ANY_BASE_SOLR_URL)
        .withHttpClient(httpClient)
        .withSocketTimeout(null)
        .withConnectionTimeout(null)
        .build();

...except for 2 problems:

1. In HttpSolrClient.executeMethod, if the values of this.connectionTimeout or this.soTimeout are null, then the values from HttpClientUtil.createDefaultRequestConfigBuilder(); get used, which has it's own hardcoded defaults.
2. withSocketTimeout and withConnectionTimeout take an int, not a (nullable) Integer.

So then maybe something like this would work? - particularly since at the HttpClient / HttpRequest / RequestConfig level, a "-1" set on the HttpRequest's RequestConfig is suppose to mean "use the (client) default" ...

SolrParams clientParams = params(HttpClientUtil.PROP_SO_TIMEOUT, 12345,
                                 HttpClientUtil.PROP_CONNECTION_TIMEOUT, 67890);
HttpClient httpClient = HttpClientUtil.createClient(clientParams);
HttpSolrClient client = new HttpSolrClient.Builder(ANY_BASE_SOLR_URL)
        .withHttpClient(httpClient)
        .withSocketTimeout(-1)
        .withConnectionTimeout(-1)
        .build();

...except that if we do that we get an IllegalArgumentException...

  // SolrClientBuilder
  public B withConnectionTimeout(int connectionTimeoutMillis) {
    if (connectionTimeoutMillis < 0) {
      throw new IllegalArgumentException("connectionTimeoutMillis must be a non-negative integer.");
    }

This is madness, and eliminates most/all of the known value of using .withHttpClient

Issue Links

• is related to: SOLR-12550 ConcurrentUpdateSolrClient doesn't respect timeouts for commits and optimize
• is related to: SOLR-13457 Managing Timeout values in Solr
• relates to: SOLR-13570 Unable to control timeout for requests from Overseer to other nodes
• links to: GitHub Pull Request #1565
• links to: GitHub Pull Request #1619