SOLR-18413: Handle deleted MIGRATE routing targets

changelog/unreleased/fix-dangling-routing-rule-target.yml

@@ -0,0 +1,8 @@
+title: Prevent partial writes when a collection referenced by a MIGRATE routing rule has been deleted
+type: fixed
+authors:
+  - name: ZhenyuLi
+    nick: JHSUYU
+links:
+  - name: SOLR-18413
+    url: https://issues.apache.org/jira/browse/SOLR-18413

solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java

@@ -1011,7 +1011,17 @@ protected List<SolrCmdDistributor.Node> getNodesByRoutingRules(
                 int hash = compositeIdRouter.sliceHash(id, doc, null, coll);
                 for (DocRouter.Range range : ranges) {
                   if (range.includes(hash)) {
-                    DocCollection targetColl = cstate.getCollection(rule.getTargetCollectionName());
+                    DocCollection targetColl =
+                        cstate.getCollectionOrNull(rule.getTargetCollectionName());
+                    if (targetColl == null) {
+                      if (log.isInfoEnabled()) {
+                        log.info(
+                            "Removing shard update routing rule because the target collection {} doesn't exist",
+                            rule.getTargetCollectionName());
+                      }
+                      removeRoutingRule(myShardId, routeKey);
+                      break;
+                    }
                     Collection<Slice> activeSlices =
                         targetColl.getRouter().getSearchSlicesSingle(id, null, targetColl);
                     if (activeSlices == null || activeSlices.isEmpty()) {
@@ -1031,46 +1041,8 @@ protected List<SolrCmdDistributor.Node> getNodesByRoutingRules(
                 }
               }
             } else {
-              ReentrantLock ruleExpiryLock = req.getCore().getRuleExpiryLock();
-              if (!ruleExpiryLock.isLocked()) {
-                try {
-                  if (ruleExpiryLock.tryLock(10, TimeUnit.MILLISECONDS)) {
-                    log.info("Going to expire routing rule");
-                    try {
-                      Map<String, Object> map =
-                          Map.of(
-                              Overseer.QUEUE_OPERATION,
-                              OverseerAction.REMOVEROUTINGRULE.toLower(),
-                              ZkStateReader.COLLECTION_PROP,
-                              collection,
-                              ZkStateReader.SHARD_ID_PROP,
-                              myShardId,
-                              "routeKey",
-                              routeKey + "!");
-                      if (distributedClusterStateUpdater.isDistributedStateUpdate()) {
-                        ZkNodeProps message = new ZkNodeProps(map);
-                        distributedClusterStateUpdater.doSingleStateUpdate(
-                            DistributedClusterStateUpdater.MutatingCommand.SliceRemoveRoutingRule,
-                            message,
-                            zkController.getOverseer().getSolrCloudManager(),
-                            zkController.getOverseer().getZkStateReader());
-                      } else {
-                        zkController.getOverseer().offerStateUpdate(Utils.toJSON(map));
-                      }
-                    } catch (KeeperException e) {
-                      log.warn(
-                          "Exception while removing routing rule for route key: {}", routeKey, e);
-                    } catch (Exception e) {
-                      log.error(
-                          "Exception while removing routing rule for route key: {}", routeKey, e);
-                    } finally {
-                      ruleExpiryLock.unlock();
-                    }
-                  }
-                } catch (InterruptedException e) {
-                  Thread.currentThread().interrupt();
-                }
-              }
+              log.info("Removing shard update routing rule because it has expired");
+              removeRoutingRule(myShardId, routeKey);
             }
           }
         }
@@ -1079,6 +1051,47 @@ protected List<SolrCmdDistributor.Node> getNodesByRoutingRules(
     return nodes;
   }
 
+  private void removeRoutingRule(String shardId, String routeKey) {
+    ReentrantLock ruleExpiryLock = req.getCore().getRuleExpiryLock();
+    if (ruleExpiryLock.isLocked()) {
+      return;
+    }
+    try {
+      if (ruleExpiryLock.tryLock(10, TimeUnit.MILLISECONDS)) {
+        try {
+          Map<String, Object> map =
+              Map.of(
+                  Overseer.QUEUE_OPERATION,
+                  OverseerAction.REMOVEROUTINGRULE.toLower(),
+                  ZkStateReader.COLLECTION_PROP,
+                  collection,
+                  ZkStateReader.SHARD_ID_PROP,
+                  shardId,
+                  "routeKey",
+                  routeKey + "!");
+          if (distributedClusterStateUpdater.isDistributedStateUpdate()) {
+            ZkNodeProps message = new ZkNodeProps(map);
+            distributedClusterStateUpdater.doSingleStateUpdate(
+                DistributedClusterStateUpdater.MutatingCommand.SliceRemoveRoutingRule,
+                message,
+                zkController.getOverseer().getSolrCloudManager(),
+                zkController.getOverseer().getZkStateReader());
+          } else {
+            zkController.getOverseer().offerStateUpdate(Utils.toJSON(map));
+          }
+        } catch (KeeperException e) {
+          log.warn("Exception while removing routing rule for route key: {}", routeKey, e);
+        } catch (Exception e) {
+          log.error("Exception while removing routing rule for route key: {}", routeKey, e);
+        } finally {
+          ruleExpiryLock.unlock();
+        }
+      }
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+    }
+  }
+
   private void doDefensiveChecks(DistribPhase phase, UpdateCommand updateCommand) {
     boolean isReplayOrPeersync =
         (updateCommand.getFlags() & (UpdateCommand.REPLAY | UpdateCommand.PEER_SYNC)) != 0;

solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java

@@ -100,6 +100,59 @@ public void testMissingSplitKey() throws Exception {
     assertTrue(remoteSolrException.getMessage().contains("split.key cannot be null or empty"));
   }
 
+  @Test
+  public void updateSucceedsAfterMigrateTargetIsDeleted() throws Exception {
+    String sourceCollection = "deletedMigrateTarget-source";
+    CollectionAdminRequest.createCollection(sourceCollection, "conf", 1, 2)
+        .process(cluster.getSolrClient());
+    String targetCollection = "deletedMigrateTarget-target";
+    CollectionAdminRequest.createCollection(targetCollection, "conf", 1, 1)
+        .process(cluster.getSolrClient());
+
+    cluster.getSolrClient().add(sourceCollection, new SolrInputDocument("id", "a!1"));
+    cluster.getSolrClient().commit(sourceCollection);
+
+    invokeCollectionMigration(
+        CollectionAdminRequest.migrateData(sourceCollection, targetCollection, "a!")
+            .setForwardTimeout(45));
+    waitForState(
+        "Expected to find routing rule for split key a",
+        sourceCollection,
+        c -> {
+          if (c == null) return false;
+          Map<String, RoutingRule> routingRules = c.getSlice("shard1").getRoutingRules();
+          return routingRules != null && routingRules.containsKey("a!");
+        });
+
+    CollectionAdminRequest.deleteCollection(targetCollection).process(cluster.getSolrClient());
+    waitForState("Expected target collection deletion", targetCollection, c -> c == null);
+
+    cluster.getSolrClient().add(sourceCollection, new SolrInputDocument("id", "a!2"));
+    cluster.getSolrClient().commit(sourceCollection);
+
+    DocCollection sourceState = getCollectionState(sourceCollection);
+    assertEquals(2, sourceState.getSlice("shard1").getReplicas().size());
+    for (Replica replica : sourceState.getSlice("shard1")) {
+      try (SolrClient replicaClient = getHttpSolrClient(replica)) {
+        SolrQuery query = new SolrQuery("id:\"a!2\"");
+        query.set("distrib", false);
+        assertEquals(
+            "Document missing from replica " + replica.getName(),
+            1,
+            replicaClient.query(query).getResults().getNumFound());
+      }
+    }
+
+    waitForState(
+        "Expected dangling routing rule removal",
+        sourceCollection,
+        c -> {
+          if (c == null) return false;
+          Map<String, RoutingRule> routingRules = c.getSlice("shard1").getRoutingRules();
+          return routingRules == null || !routingRules.containsKey("a!");
+        });
+  }
+
   @Test
   public void multipleShardMigrateTest() throws Exception {