diff --git a/changelog/unreleased/fix-dangling-routing-rule-target.yml b/changelog/unreleased/fix-dangling-routing-rule-target.yml new file mode 100644 index 00000000000..a3ba20b78bc --- /dev/null +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java index 49c9fc717a7..fad45bb70cb 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java @@ -1011,7 +1011,17 @@ protected List 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 activeSlices = targetColl.getRouter().getSearchSlicesSingle(id, null, targetColl); if (activeSlices == null || activeSlices.isEmpty()) { @@ -1031,46 +1041,8 @@ protected List 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 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 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 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; diff --git a/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java b/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java index 723f9d9ba6a..09114574f2a 100644 --- a/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java +++ b/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 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 routingRules = c.getSlice("shard1").getRoutingRules(); + return routingRules == null || !routingRules.containsKey("a!"); + }); + } + @Test public void multipleShardMigrateTest() throws Exception {