From 54dca800a9432a72b93723d40fa4abc9a8e11f14 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Fri, 15 May 2020 00:31:35 -0400 Subject: [PATCH] SOLR-14471: Fix last-place replica after shards.preference rules (#1507) Properly apply base replica ordering to last-place shards.preference matches --- .../RequestReplicaListTransformerGenerator.java | 3 ++- ...uestReplicaListTransformerGeneratorTest.java | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java b/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java index 2f417687a4c..4853787783c 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java @@ -158,7 +158,7 @@ public class RequestReplicaListTransformerGenerator { Object current; int idx = 1; int boundaryCount = 0; - int[] boundaries = new int[choices.size() - 1]; + int[] boundaries = new int[choices.size()]; do { current = iter.next(); if (replicaComp.compare(prev, current) != 0) { @@ -167,6 +167,7 @@ public class RequestReplicaListTransformerGenerator { prev = current; idx++; } while (iter.hasNext()); + boundaries[boundaryCount++] = idx; // Finally inspect boundaries to apply base transformation, where necessary (separate phase to avoid ConcurrentModificationException) int startIdx = 0; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGeneratorTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGeneratorTest.java index 5b64fde54ba..c0ebad379d6 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGeneratorTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGeneratorTest.java @@ -88,6 +88,19 @@ public class RequestReplicaListTransformerGeneratorTest extends SolrTestCaseJ4 { ) ); + // Add a PULL replica so that there's a tie for "last place" + replicas.add( + new Replica( + "node5", + map( + ZkStateReader.BASE_URL_PROP, "http://host2_2:8983/solr", + ZkStateReader.NODE_NAME_PROP, "node5", + ZkStateReader.CORE_NAME_PROP, "collection1", + ZkStateReader.REPLICA_TYPE, "PULL" + ), "c1","s1" + ) + ); + // replicaType and replicaBase combined rule param String rulesParam = ShardParams.SHARDS_PREFERENCE_REPLICA_TYPE + ":NRT," + ShardParams.SHARDS_PREFERENCE_REPLICA_TYPE + ":TLOG," + @@ -101,6 +114,7 @@ public class RequestReplicaListTransformerGeneratorTest extends SolrTestCaseJ4 { assertEquals("node2", replicas.get(1).getNodeName()); assertEquals("node4", replicas.get(2).getNodeName()); assertEquals("node3", replicas.get(3).getNodeName()); + assertEquals("node5", replicas.get(4).getNodeName()); params.set("routingPreference", "1"); rlt = generator.getReplicaListTransformer(params); @@ -108,7 +122,8 @@ public class RequestReplicaListTransformerGeneratorTest extends SolrTestCaseJ4 { assertEquals("node1", replicas.get(0).getNodeName()); assertEquals("node4", replicas.get(1).getNodeName()); assertEquals("node2", replicas.get(2).getNodeName()); - assertEquals("node3", replicas.get(3).getNodeName()); + assertEquals("node5", replicas.get(3).getNodeName()); + assertEquals("node3", replicas.get(4).getNodeName()); } @SuppressWarnings("unchecked")