[
https://issues.apache.org/jira/browse/SOLR-13605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17712525#comment-17712525
]
Eric Pugh commented on SOLR-13605:
----------------------------------
Adding a unit test that demonstrates the problem... I did notice that part
of the Http2SolrClient is that it never exposes the internal HttpClient, so it
doesn't have the same withHttpClient issue...
Thoughts on just marking this issue "won't fix" and moving on?
> 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
> Priority: Major
> Time Spent: 10m
> 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: [email protected]
For additional commands, e-mail: [email protected]
