> 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?

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.


> On Nov. 25, 2014, 12:55 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java, line 334
> > <https://reviews.apache.org/r/28393/diff/1/?file=774309#file774309line334>
> >
> >     why isn't this the same as above, i.e. 2 passed to the hashmap 
> > constructor.  Actually, could this whole thing be simplified, i.e.
> >     
> >     if (info == null || info.attributes == null || info.attributes.size==0) 
> > {
> >       attributes = new ...
> >     }
> >     else {
> >       attributes = info.attributes;
> >     }

Yeah, I started it thinking that name and class had to be in the init args and 
not attributes - still hadn't cleaned it up fully.


> On Nov. 25, 2014, 12:55 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java,
> >  line 177
> > <https://reviews.apache.org/r/28393/diff/1/?file=774311#file774311line177>
> >
> >     should these be configurable?  At least TCP_NODELAY seems like it 
> > should, it seems orthogonal to the rest of the patch.

I kept it in because it's in the original patch and it seems we should be using 
it internally: " When applications wish to decrease network latency and 
increase performance, they can disable Nagle's algorithm"


> On Nov. 25, 2014, 12:55 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java,
> >  line 185
> > <https://reviews.apache.org/r/28393/diff/1/?file=774311#file774311line185>
> >
> >     So we don't do anything if we don't use the default client?  This seems 
> > like a better fit for the HttpClientUtil.

Yeah, its up to the creator of the http client to decide what params to pass 
and whether or not they want to use the IdleConnectionMonitorRunnable. Good 
flag though, we should also do this for the http client used by peer sync. I 
was going to search for all our uses, but I'm mainly concerned about main query 
and update paths.


- Mark


-----------------------------------------------------------
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
> 
>

Reply via email to