Hoss Man created SOLR-13605:
-------------------------------

             Summary: 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
      Security Level: Public (Default Security Level. Issues are Public)
            Reporter: Hoss Man


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
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to