cpoerschke commented on a change in pull request #677: SOLR-13257: support for
stable replica routing preferences
URL: https://github.com/apache/lucene-solr/pull/677#discussion_r312612736
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
##########
@@ -230,7 +288,23 @@ public void init(PluginInfo info) {
this.accessPolicy = getParameter(args, INIT_FAIRNESS_POLICY,
accessPolicy,sb);
this.whitelistHostChecker = new WhitelistHostChecker(args == null? null:
(String) args.get(INIT_SHARDS_WHITELIST), !getDisableShardsWhitelist());
log.info("Host whitelist initialized: {}", this.whitelistHostChecker);
-
+
+ this.httpListenerFactory = new
InstrumentedHttpListenerFactory(this.metricNameStrategy);
+ int connectionTimeout = getParameter(args,
HttpClientUtil.PROP_CONNECTION_TIMEOUT,
+ HttpClientUtil.DEFAULT_CONNECT_TIMEOUT, sb);
+ int maxConnectionsPerHost = getParameter(args,
HttpClientUtil.PROP_MAX_CONNECTIONS_PER_HOST,
+ HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST, sb);
+ int soTimeout = getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT,
+ HttpClientUtil.DEFAULT_SO_TIMEOUT, sb);
+
+ this.defaultClient = new Http2SolrClient.Builder()
+ .connectionTimeout(connectionTimeout)
+ .idleTimeout(soTimeout)
+ .maxConnectionsPerHost(maxConnectionsPerHost).build();
+ this.defaultClient.addListenerFactory(this.httpListenerFactory);
+ this.loadbalancer = new LBHttp2SolrClient(defaultClient);
+ initReplicaListTransformers(getParameter(args, "replicaRouting", null,
sb));
+
log.debug("created with {}",sb);
Review comment:
You clarify in the
https://github.com/apache/lucene-solr/pull/677/commits/d82dc54fd625762008a5f8b7c69bcc4ee4f57203
commit message that the re-ordering here is because `sb` is being logged here
but then it's also subsequently still used.
I wonder if an alternative change could be to 'relocate' the logging
statement to the end of the `init` method? The logging of a 'created' message
in the middle of the init seems surprising from a code comprehension
perspective but also just after the `log.debug` there is the `r.setSeed` call
and (theoretically at least) moving code from 'after' to 'before' that might
make a difference.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
