> On Nov. 25, 2014, 12:55 a.m., Gregory Chanan wrote: > > It seems strange to me that the configuration here is all done via SolrXml; > > getting rid of the stale performance check and closing idle connections > > doesn't seem much different than configuring say, SO_TIMEOUT, which is all > > in HttpClientUtil. Also, if this configuration is so great, wouldn't I > > want it on my own HttpSolrServers, or at least the ability to configure it? > > Of course the parameters could still be set in SolrConfigXml for the > > HttpShardhandler and UpdateShardHandler, but the meat of it seems like it > > could live in HttpClientUtil. > > > > One complication with that approach is how to close the pool. Perhaps, > > given we are about to do a new major release, it's a good time to update > > our interfaces to return CloseableHttpClients rather than HttpClients. > > Just a thought. > > Mark Miller wrote: > SO_TIMEOUT and the connection timeout for distrib updates and queries is > defined in solr.xml actually. This matches the current pattern. > > HttpSolrServer takes an httpclient instance if you want to rig this > yourself. I was going to set up CloudSolrServer to do this by default, but > I'm less concerned about HttpSolrServer.
I don't think we are disagreeing here. PROP_SO_TIMEOUT is a property of HttpClientUtil, but there is a corresponding property in ConfigSolr for setting that property in the UpdateShardHandler (getDistributedSocketTimeout). I'm just suggesting we take the same approach here. This allows normal clients to configure it easily if they want, and the server processes behave the exact same way (configured via ConfigSolr). > On Nov. 25, 2014, 12:55 a.m., Gregory Chanan wrote: > > trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java, line 288 > > <https://reviews.apache.org/r/28393/diff/1/?file=774306#file774306line288> > > > > is there anywhere that says what the unit of time this actually is? > > What about just renaming TIME -> SECS or something? > > Mark Miller wrote: > Since we don't do that with others, I won't in this issue, but it should > be in milliseconds as al the other time settings are. Doesn't look like milliseconds: ses.scheduleWithFixedDelay(sweeper, threadPoolSweeperPollTime, threadPoolSweeperPollTime, TimeUnit.SECONDS); - Gregory ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28393/#review62902 ----------------------------------------------------------- On Nov. 24, 2014, 3:42 p.m., Mark Miller wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28393/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2014, 3:42 p.m.) > > > Review request for lucene. > > > Repository: lucene > > > Description > ------- > > https://issues.apache.org/jira/browse/SOLR-4509 > > > Diffs > ----- > > trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java 1641405 > trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java 1641405 > trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java 1641405 > trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java 1641405 > trunk/solr/core/src/java/org/apache/solr/core/PluginInfo.java 1641405 > > trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java > 1641405 > trunk/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java > 1641405 > > trunk/solr/core/src/java/org/apache/solr/util/IdleConnectionMonitorRunnable.java > PRE-CREATION > trunk/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java 1641405 > > trunk/solr/core/src/test/org/apache/solr/cloud/TestLeaderElectionZkExpiry.java > 1641405 > trunk/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java 1641405 > trunk/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java > 1641405 > trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java > 1641405 > > trunk/solr/core/src/test/org/apache/solr/core/TestImplicitCoreProperties.java > 1641405 > trunk/solr/server/etc/jetty.xml 1641405 > > trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java > 1641405 > > Diff: https://reviews.apache.org/r/28393/diff/ > > > Testing > ------- > > > Thanks, > > Mark Miller > >
