lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tomás Fernández Löbbe <tflo...@apache.org>
Subject Re: Review Request 65942: SOLR-11982: Add support for shards.sort parameter
Date Thu, 08 Mar 2018 20:54:28 GMT

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


Mime
View raw message