magibney 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_r302352238
########## File path: solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java ########## @@ -449,9 +556,83 @@ private static boolean hasReplicaType(Object o, String preferred) { } } + private final ReplicaListTransformerFactory randomRltFactory = (String configSpec, SolrQueryRequest request, + ReplicaListTransformerFactory fallback) -> shufflingReplicaListTransformer; + private ReplicaListTransformerFactory stableRltFactory; + private ReplicaListTransformerFactory defaultRltFactory; + + /** + * Private class responsible for applying pairwise sort based on inherent replica attributes, + * and subsequently reordering any equivalent replica sets according to behavior specified + * by the baseReplicaListTransformer. + */ + private static final class TopLevelReplicaListTransformer implements ReplicaListTransformer { + + private final NodePreferenceRulesComparator replicaComp; + private final ReplicaListTransformer baseReplicaListTransformer; + + public TopLevelReplicaListTransformer(NodePreferenceRulesComparator replicaComp, ReplicaListTransformer baseReplicaListTransformer) { + this.replicaComp = replicaComp; + this.baseReplicaListTransformer = baseReplicaListTransformer; + } + + @Override + public void transform(List<?> choices) { + if (choices.size() > 1) { + if (log.isDebugEnabled()) { + log.debug("Applying the following sorting preferences to replicas: {}", + Arrays.toString(replicaComp.preferenceRules.toArray())); + } + + // First, sort according to comparator rules. + try { + choices.sort(replicaComp); + } catch (IllegalArgumentException iae) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + iae.getMessage() Review comment: This sounds reasonable to me, but this specific section of code is actually directly carried over from the existing code. I'm only mentioning this because I'm inclined to proceed with more caution here, in case this was a considered choice (for instance, if iae.getMessage() is expected to provide all the necessary information?). I notice that the IAEs that actually get thrown in NodePreferenceRulesComparator.compare() have pretty specific messages, so perhaps a the full exception/stack trace was deemed to be unnecessary? ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org