SOLR-18335 : SPLITSHARD fails to migrate documents when using a numeric PointField as router.field

changelog/unreleased/SOLR-18335-splitshard-router-field-point-field.yml

@@ -0,0 +1,10 @@
+title: >
+  SPLITSHARD has expanded support for collection router.field, not only supporting indexed fields but also docValues.
+  Note: "Point" numeric fields only work in this case with docValues.
+type: added
+authors:
+  - name: Olivier Boudet
+  - name: David Smiley
+links:
+  - name: SOLR-18335
+    url: https://issues.apache.org/jira/browse/SOLR-18335

solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java

@@ -41,6 +41,8 @@
 import org.apache.lucene.index.Terms;
 import org.apache.lucene.index.TermsEnum;
 import org.apache.lucene.misc.store.HardlinkCopyDirectoryWrapper;
+import org.apache.lucene.queries.function.FunctionValues;
+import org.apache.lucene.queries.function.ValueSource;
 import org.apache.lucene.search.ConstantScoreScorer;
 import org.apache.lucene.search.ConstantScoreWeight;
 import org.apache.lucene.search.DocIdSetIterator;
@@ -679,7 +681,21 @@ static FixedBitSet[] split(
 
     Terms terms = reader.terms(field.getName());
     TermsEnum termsEnum = terms == null ? null : terms.iterator();
-    if (termsEnum == null) return docSets;
+    if (termsEnum == null) {
+      // No term dictionary for this field (e.g. a PointField, which has no Terms; or a
+      // docValues-only field that isn't indexed). Derive route values from docValues instead.
+      return splitUsingDocValues(
+          readerContext,
+          numPieces,
+          field,
+          rangesArr,
+          splitKey,
+          hashRouter,
+          delete,
+          docSets,
+          liveDocs,
+          currentPartition);
+    }
 
     BytesRef term = null;
     PostingsEnum postingsEnum = null;
@@ -746,38 +762,124 @@ static FixedBitSet[] split(
     }
 
     if (docsMatchingRanges != null) {
-      for (int ii = 0; ii < docsMatchingRanges.length; ii++) {
-        if (0 == docsMatchingRanges[ii]) continue;
-        switch (ii) {
-          case 0:
-            // document loss
-            log.error(
-                "Splitting {}: {} documents belong to no shards and will be dropped",
-                reader,
-                docsMatchingRanges[ii]);
-            break;
-          case 1:
-            // normal case, each document moves to one of the sub-shards
-            log.info(
-                "Splitting {}: {} documents will move into a sub-shard",
-                reader,
-                docsMatchingRanges[ii]);
-            break;
-          default:
-            // document duplication
-            log.error(
-                "Splitting {}: {} documents will be moved to multiple ({}) sub-shards",
-                reader,
-                docsMatchingRanges[ii],
-                ii);
-            break;
+      logDocsMatchingRanges(reader, docsMatchingRanges);
+    }
+
+    return docSets;
+  }
+
+  private static FixedBitSet[] splitUsingDocValues(
+      LeafReaderContext readerContext,
+      int numPieces,
+      SchemaField field,
+      DocRouter.Range[] rangesArr,
+      String splitKey,
+      HashBasedRouter hashRouter,
+      boolean delete,
+      FixedBitSet[] docSets,
+      Bits liveDocs,
+      AtomicInteger currentPartition)
+      throws IOException {
+    if (!field.hasDocValues()) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Route field '"
+              + field.getName()
+              + "' has no term index to split by and lacks docValues; unable to split shard.");
+    }
+    LeafReader reader = readerContext.reader();
+    ValueSource valueSource = field.getType().getValueSource(field, null);
+    FunctionValues routeFieldValues = valueSource.getValues(Map.of(), readerContext);
+
+    int[] docsMatchingRanges = null;
+    if (rangesArr != null) {
+      docsMatchingRanges = new int[rangesArr.length + 1];
+    }
+
+    for (int doc = 0; doc < reader.maxDoc(); doc++) {
+      if (liveDocs != null && !liveDocs.get(doc)) {
+        continue;
+      }
+
+      String routeValue = getRouteFieldValue(routeFieldValues, doc, field);
+      if (splitKey != null) {
+        String part1 = ((CompositeIdRouter) hashRouter).getRouteKeyNoSuffix(routeValue);
+        if (!splitKey.equals(part1)) {
+          continue;
         }
       }
+
+      if (rangesArr == null) {
+        if (delete) {
+          docSets[currentPartition.get()].clear(doc);
+        } else {
+          docSets[currentPartition.get()].set(doc);
+        }
+        currentPartition.set((currentPartition.get() + 1) % numPieces);
+      } else {
+        int hash = hashRouter.sliceHash(routeValue, null, null, null);
+        int matchingRangesCount = 0;
+        for (int i = 0; i < rangesArr.length; i++) {
+          if (rangesArr[i].includes(hash)) {
+            if (delete) {
+              docSets[i].clear(doc);
+            } else {
+              docSets[i].set(doc);
+            }
+            ++matchingRangesCount;
+          }
+        }
+        docsMatchingRanges[matchingRangesCount]++;
+      }
     }
 
+    if (docsMatchingRanges != null) {
+      logDocsMatchingRanges(reader, docsMatchingRanges);
+    }
     return docSets;
   }
 
+  private static String getRouteFieldValue(
+      FunctionValues routeFieldValues, int doc, SchemaField field) throws IOException {
+    if (routeFieldValues.exists(doc)) {
+      return routeFieldValues.strVal(doc);
+    }
+
+    throw new SolrException(
+        SolrException.ErrorCode.SERVER_ERROR,
+        "Unable to read route field '" + field.getName() + "' for shard splitting.");
+  }
+
+  private static void logDocsMatchingRanges(LeafReader reader, int[] docsMatchingRanges) {
+    for (int ii = 0; ii < docsMatchingRanges.length; ii++) {
+      if (0 == docsMatchingRanges[ii]) continue;
+      switch (ii) {
+        case 0:
+          // document loss
+          log.error(
+              "Splitting {}: {} documents belong to no shards and will be dropped",
+              reader,
+              docsMatchingRanges[ii]);
+          break;
+        case 1:
+          // normal case, each document moves to one of the sub-shards
+          log.info(
+              "Splitting {}: {} documents will move into a sub-shard",
+              reader,
+              docsMatchingRanges[ii]);
+          break;
+        default:
+          // document duplication
+          log.error(
+              "Splitting {}: {} documents will be moved to multiple ({}) sub-shards",
+              reader,
+              docsMatchingRanges[ii],
+              ii);
+          break;
+      }
+    }
+  }
+
   private static void checkRouterSupportsSplitKey(HashBasedRouter hashRouter, String splitKey) {
     if (splitKey != null && !(hashRouter instanceof CompositeIdRouter)) {
       throw new IllegalStateException(

solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java

@@ -107,6 +107,7 @@ public void test() throws Exception {
     incompleteOrOverlappingCustomRangeTest();
     splitByUniqueKeyTest();
     splitByRouteFieldTest();
+    splitByNumericRouteFieldTest();
     splitByRouteKeyTest();
 
     // todo can't call waitForThingsToLevelOut because it looks for jettys of all shards
@@ -1009,6 +1010,75 @@ public void splitByRouteFieldTest() throws Exception {
               .query(new SolrQuery("*:*").setParam("shards", "shard1_1"))
               .getResults()
               .getNumFound());
+      assertEquals(101, collectionClient.query(new SolrQuery("*:*")).getResults().getNumFound());
+    }
+  }
+
+  public void splitByNumericRouteFieldTest() throws Exception {
+    log.info("Starting splitByNumericRouteFieldTest");
+    String collectionName = "numericRouteFieldColl";
+    int numShards = 4;
+    int replicationFactor = 2;
+
+    HashMap<String, List<Integer>> collectionInfos = new HashMap<>();
+    String shardField = "shard_l";
+    try (CloudSolrClient client = createCloudClient(null)) {
+      Map<String, Object> props =
+          Map.of(
+              REPLICATION_FACTOR,
+              replicationFactor,
+              CollectionHandlingUtils.NUM_SLICES,
+              numShards,
+              "router.field",
+              shardField);
+
+      createCollection(collectionInfos, collectionName, props, client);
+    }
+
+    List<Integer> list = collectionInfos.get(collectionName);
+    checkForCollection(collectionName, list);
+
+    waitForRecoveriesToFinish(false);
+
+    getCommonCloudSolrClient();
+    String baseUrl = getBaseUrlFromZk(cloudClient.getClusterState(), collectionName);
+
+    try (SolrClient collectionClient = getHttpSolrClient(baseUrl, collectionName)) {
+      ClusterState clusterState = cloudClient.getClusterState();
+      final DocRouter router = clusterState.getCollection(collectionName).getRouter();
+      Slice shard1 = clusterState.getCollection(collectionName).getSlice(SHARD1);
+      DocRouter.Range shard1Range =
+          shard1.getRange() != null ? shard1.getRange() : router.fullRange();
+      final List<DocRouter.Range> ranges = router.partitionRange(2, shard1Range);
+      final int[] docCounts = new int[ranges.size()];
+
+      for (int i = 100; i <= 200; i++) {
+        collectionClient.add(getDoc(id, i, "n_ti", i, shardField, i));
+        int idx = getHashRangeIdx(router, ranges, Integer.toString(i));
+        if (idx != -1) {
+          docCounts[idx]++;
+        }
+      }
+
+      collectionClient.commit();
+
+      trySplit(collectionName, null, SHARD1, 3);
+
+      waitForRecoveriesToFinish(collectionName, false);
+
+      assertEquals(
+          docCounts[0],
+          collectionClient
+              .query(new SolrQuery("*:*").setParam("shards", "shard1_0"))
+              .getResults()
+              .getNumFound());
+      assertEquals(
+          docCounts[1],
+          collectionClient
+              .query(new SolrQuery("*:*").setParam("shards", "shard1_1"))
+              .getResults()
+              .getNumFound());
+      assertEquals(101, collectionClient.query(new SolrQuery("*:*")).getResults().getNumFound());
     }
   }
 

solr/core/src/test/org/apache/solr/update/SolrIndexSplitterTest.java

@@ -55,6 +55,10 @@ public static void beforeClass() throws Exception {
     // _version_
     System.setProperty("solr.directoryFactory", "solr.NRTCachingDirectoryFactory");
     System.setProperty("solr.tests.lockType", DirectoryFactory.LOCK_TYPE_SIMPLE);
+    // route_l needs docValues to be usable as a router.field when numerics are Point-based
+    if (Boolean.getBoolean(NUMERIC_POINTS_SYSPROP)) {
+      System.setProperty(NUMERIC_DOCVALUES_SYSPROP, "true");
+    }
 
     initCore("solrconfig.xml", "schema15.xml");
   }
@@ -413,6 +417,16 @@ public void testSplitByRouteKeyLink() throws Exception {
     doTestSplitByRouteKey(SolrIndexSplitter.SplitMethod.LINK);
   }
 
+  @Test
+  public void testSplitByNumericRouteField() throws Exception {
+    doTestSplitByNumericRouteField(SolrIndexSplitter.SplitMethod.REWRITE);
+  }
+
+  @Test
+  public void testSplitByNumericRouteFieldLink() throws Exception {
+    doTestSplitByNumericRouteField(SolrIndexSplitter.SplitMethod.LINK);
+  }
+
   private void doTestSplitByRouteKey(SolrIndexSplitter.SplitMethod splitMethod) throws Exception {
     Path indexDir = createTempDir();
 
@@ -480,6 +494,83 @@ private void doTestSplitByRouteKey(SolrIndexSplitter.SplitMethod splitMethod) th
     }
   }
 
+  private void doTestSplitByNumericRouteField(SolrIndexSplitter.SplitMethod splitMethod)
+      throws Exception {
+    CompositeIdRouter router = new CompositeIdRouter();
+    List<DocRouter.Range> ranges = router.partitionRange(2, router.fullRange());
+    int[] expectedDocCounts = new int[ranges.size()];
+
+    for (int i = 100; i < 140; i++) {
+      String routeValue = Integer.toString(i);
+      assertU(adoc("id", "doc-" + i, "route_l", routeValue));
+
+      int hash = router.sliceHash(routeValue, null, null, null);
+      for (int rangeIndex = 0; rangeIndex < ranges.size(); rangeIndex++) {
+        if (ranges.get(rangeIndex).includes(hash)) {
+          expectedDocCounts[rangeIndex]++;
+          break;
+        }
+      }
+    }
+
+    assertU(commit());
+    assertJQ(req("q", "*:*"), "/response/numFound==40");
+
+    SolrQueryRequestBase request = null;
+    Directory directory = null;
+    try {
+      request = lrf.makeRequest("q", "dummy");
+      SolrQueryResponse rsp = new SolrQueryResponse();
+      SplitIndexCommand command =
+          new SplitIndexCommand(
+              request,
+              rsp,
+              List.of(indexDir1.toString(), indexDir2.toString()),
+              null,
+              ranges,
+              router,
+              "route_l",
+              null,
+              splitMethod);
+      doSplit(command);
+
+      directory =
+          h.getCore()
+              .getDirectoryFactory()
+              .get(
+                  indexDir1.toString(),
+                  DirectoryFactory.DirContext.DEFAULT,
+                  h.getCore().getSolrConfig().indexConfig.lockType);
+      DirectoryReader reader = DirectoryReader.open(directory);
+      assertEquals(
+          "split index1 has wrong number of documents", expectedDocCounts[0], reader.numDocs());
+      reader.close();
+      h.getCore().getDirectoryFactory().release(directory);
+      directory = null;
+
+      directory =
+          h.getCore()
+              .getDirectoryFactory()
+              .get(
+                  indexDir2.toString(),
+                  DirectoryFactory.DirContext.DEFAULT,
+                  h.getCore().getSolrConfig().indexConfig.lockType);
+      reader = DirectoryReader.open(directory);
+      assertEquals(
+          "split index2 has wrong number of documents", expectedDocCounts[1], reader.numDocs());
+      reader.close();
+      h.getCore().getDirectoryFactory().release(directory);
+      directory = null;
+    } finally {
+      if (request != null) {
+        request.close();
+      }
+      if (directory != null) {
+        h.getCore().getDirectoryFactory().release(directory);
+      }
+    }
+  }
+
   @Test
   public void testSplitWithChildDocs() throws Exception {
     doTestSplitWithChildDocs(SolrIndexSplitter.SplitMethod.REWRITE);