----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65942/#review198909 -----------------------------------------------------------
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java Lines 309 (patched) <https://reviews.apache.org/r/65942/#comment279172> I'm debating with myself on whether we should allow both params to be specified or not. I think it would be better to throw an exception in case of both parameters specified (since preferLocalShards would now be deprecated) solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java Line 364 (original), 325 (patched) <https://reviews.apache.org/r/65942/#comment279171> I think we should surround this with a try/catch and throw a SolrException(SolrError.BAD_REQUEST) in case of an IllegalArgumentException solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java Lines 379 (patched) <https://reviews.apache.org/r/65942/#comment279170> This should be an inner class or have it's own file solr/core/src/test/org/apache/solr/handler/component/HttpShardHandlerFactoryTest.java Lines 31 (patched) <https://reviews.apache.org/r/65942/#comment279173> Note that there is a `TestHttpShardHandlerFactory`, did you intentionally not use that one? - Tomás Fernández Löbbe On March 8, 2018, 5:38 p.m., Tomás Fernández Löbbe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65942/ > ----------------------------------------------------------- > > (Updated March 8, 2018, 5:38 p.m.) > > > Review request for lucene. > > > Repository: lucene-solr > > > Description > ------- > > Creating this Review request on Ere Maijala's patch. See SOLR-11982 for > previous discussion. > It would be nice to have the possibility to easily sort the shards in the > preferred order e.g. by replica type. The attached patch adds support for > shards.sort parameter that allows one to sort e.g. PULL and TLOG replicas > first with ``shards.sor=replicaType:PULL|TLOG``(which would mean that NRT > replicas wouldn't be hit with queries unless they're the only ones available) > and/or to sort by replica location (like preferLocalShards=true but more > versatile). > > > Diffs > ----- > > solr/CHANGES.txt c1b5161c9c > > solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java > 6bfd36af94 > > solr/core/src/test/org/apache/solr/handler/component/HttpShardHandlerFactoryTest.java > PRE-CREATION > solr/solr-ref-guide/src/distributed-requests.adoc f5aaff469e > solr/solrj/src/java/org/apache/solr/common/params/ShardParams.java > cbc33f41f4 > > solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java > 9539846f88 > > > Diff: https://reviews.apache.org/r/65942/diff/2/ > > > Testing > ------- > > > Thanks, > > Tomás Fernández Löbbe > >
