[
https://issues.apache.org/jira/browse/SOLR-13605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17953861#comment-17953861
]
David Smiley commented on SOLR-13605:
-------------------------------------
It appears the issue thread here wasn't updated to clarify what happened. The
last meaningful comment said basically we're not going to actually fix
anything. But the PR seemed to fix stuff.
> 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: 3h 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: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
