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

["Eric Pugh (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-13605?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Eric Pugh resolved SOLR-13605.
------------------------------
    Fix Version/s: main (10.0)
                   9.3
         Assignee: Eric Pugh
       Resolution: Fixed

> HttpSolrClient.Builder.withHttpClient() is useless for the purpose of setting client scoped so/connect timeouts
> ---------------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-13605
>                 URL: https://issues.apache.org/jira/browse/SOLR-13605
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Chris M. Hostetter
>            Assignee: Eric Pugh
>            Priority: Major
>             Fix For: main (10.0), 9.3
>
>          Time Spent: 2.5h
>  Remaining Estimate: 0h
>
> 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...
> {code:java}
> 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();
> {code}
> 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...
> {code:java}
>   // SolrClientBuilder
>   protected Integer connectionTimeoutMillis = 15000;
>   protected Integer socketTimeoutMillis = 120000;
> {code}
> ...which unless overridden by calls to {{withConnectionTimeout()}} and {{withSocketTimeout()}} will get set on the {{HttpSolrClient}} object, and used on every request...
> {code:java}
>     // protected HttpSolrClient constructor
>     this.connectionTimeout = builder.connectionTimeoutMillis;
>     this.soTimeout = builder.socketTimeoutMillis;
> {code}
> It would be tempting to try and do something like this to work around the problem...
> {code:java}
> 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();
> {code}
> ...except for 2 problems:
>  # 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.
>  # {{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" ...
> {code:java}
> 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();
> {code}
> ...except that if we do *that* we get an IllegalArgumentException...
> {code:java}
>   // SolrClientBuilder
>   public B withConnectionTimeout(int connectionTimeoutMillis) {
>     if (connectionTimeoutMillis < 0) {
>       throw new IllegalArgumentException("connectionTimeoutMillis must be a non-negative integer.");
>     }
> {code}
> This is madness, and eliminates most/all of the known value of using {{.withHttpClient}}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org