You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2021/08/16 18:09:27 UTC
[lucene-solr] branch branch_8x updated: SOLR-8889: Fixed various
problems in Solr and SolrJ that could cause deleteById commands with
"_route_" information to processed by the wrong shard,
and/or fail when forwarded to replicas from the shard leader.
This is an automated email from the ASF dual-hosted git repository.
hossman pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 1d69e9c SOLR-8889: Fixed various problems in Solr and SolrJ that could cause deleteById commands with "_route_" information to processed by the wrong shard, and/or fail when forwarded to replicas from the shard leader.
1d69e9c is described below
commit 1d69e9c59e979e7c47d51a3cb7167df044e5db77
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Mon Aug 16 09:56:51 2021 -0700
SOLR-8889: Fixed various problems in Solr and SolrJ that could cause deleteById commands with "_route_" information to processed by the wrong shard, and/or fail when forwarded to replicas from the shard leader.
Portions of this bug and fixes were tracked in SOLR-12694.
(cherry picked from commit 5c3465eb498551c53dc310e44ae82cb89890ffc6)
---
solr/CHANGES.txt | 3 +
.../processor/DistributedZkUpdateProcessor.java | 2 +-
.../solr/cloud/FullSolrCloudDistribCmdsTest.java | 165 +++++++++++-
.../solr/update/DeleteByIdWithRouterFieldTest.java | 291 +++++++++++++++++++++
.../solrj/request/JavaBinUpdateRequestCodec.java | 9 +-
.../solr/client/solrj/request/UpdateRequest.java | 8 +-
6 files changed, 469 insertions(+), 9 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fb257ef..81fa79a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -73,6 +73,9 @@ Bug Fixes
* SOLR-15579: Allow for many values in a SQL IN clause; previously using more than 19 would result in
the IN clause being ignored, now users are only limited by the `maxBooleanClauses` configured for a collection (Timothy Potter)
+* SOLR-8889: Fixed various problems in Solr and SolrJ that could cause deleteById commands with "_route_" information to processed
+ by the wrong shard, and/or fail when forwarded to replicas from the shard leader. (Dan Fox, Yuki Yano, hossman)
+
Other Changes
---------------------
* SOLR-15355: Upgrade Hadoop from 3.2.0 to 3.2.2. (Antoine Gruzelle via David Smiley)
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 d364a11..fca6b77 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
@@ -585,7 +585,7 @@ public class DistributedZkUpdateProcessor extends DistributedUpdateProcessor {
nodes = setupRequest(acmd.getIndexedIdStr(), acmd.getSolrInputDocument());
} else if (cmd instanceof DeleteUpdateCommand) {
DeleteUpdateCommand dcmd = (DeleteUpdateCommand)cmd;
- nodes = setupRequest(dcmd.getId(), null);
+ nodes = setupRequest(dcmd.getId(), null, (null != dcmd.getRoute() ? dcmd.getRoute() : req.getParams().get(ShardParams._ROUTE_)) );
}
}
diff --git a/solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java b/solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java
index 0b15fe0..5238704 100644
--- a/solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java
@@ -18,6 +18,7 @@ package org.apache.solr.cloud;
import java.lang.invoke.MethodHandles;
+import java.util.Arrays;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -27,9 +28,10 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
-
+
import org.apache.lucene.util.TestUtil;
import org.apache.lucene.util.LuceneTestCase.Slow;
+import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.cloud.SocketProxy;
import org.apache.solr.client.solrj.embedded.JettySolrRunner;
@@ -137,6 +139,167 @@ public class FullSolrCloudDistribCmdsTest extends SolrCloudTestCase {
}
+ public void testDeleteByIdImplicitRouter() throws Exception {
+ final CloudSolrClient cloudClient = cluster.getSolrClient();
+ final String name = "implicit_collection_without_routerfield_" + NAME_COUNTER.getAndIncrement();
+ assertEquals(RequestStatusState.COMPLETED,
+ CollectionAdminRequest.createCollectionWithImplicitRouter(name, "_default", "shard1,shard2", 2)
+ .processAndWait(cloudClient, DEFAULT_TIMEOUT));
+ cloudClient.waitForState(name, DEFAULT_TIMEOUT, TimeUnit.SECONDS,
+ (n, c) -> DocCollection.isFullyActive(n, c, 2, 2));
+ cloudClient.setDefaultCollection(name);
+
+ final DocCollection docCol = cloudClient.getZkStateReader().getClusterState().getCollection(name);
+ try (SolrClient shard1 = getHttpSolrClient(docCol.getSlice("shard1").getLeader().getCoreUrl());
+ SolrClient shard2 = getHttpSolrClient(docCol.getSlice("shard2").getLeader().getCoreUrl())) {
+
+ // Add three documents to shard1
+ shard1.add(sdoc("id", "1", "title", "s1 one"));
+ shard1.add(sdoc("id", "2", "title", "s1 two"));
+ shard1.add(sdoc("id", "3", "title", "s1 three"));
+ shard1.commit();
+ final AtomicInteger docCounts1 = new AtomicInteger(3);
+
+ // Add two documents to shard2
+ shard2.add(sdoc("id", "4", "title", "s2 four"));
+ shard2.add(sdoc("id", "5", "title", "s2 five"));
+ shard2.commit();
+ final AtomicInteger docCounts2 = new AtomicInteger(2);
+
+ // A re-usable helper to verify that the expected number of documents can be found on each shard...
+ Runnable checkShardCounts = () -> {
+ try {
+ // including cloudClient helps us test view from other nodes that aren't the leaders...
+ for (SolrClient c : Arrays.asList(cloudClient, shard1, shard2)) {
+ assertEquals(docCounts1.get() + docCounts2.get(), c.query(params("q", "*:*")).getResults().getNumFound());
+
+ assertEquals(docCounts1.get(), c.query(params("q", "*:*", "shards", "shard1")).getResults().getNumFound());
+ assertEquals(docCounts2.get(), c.query(params("q", "*:*", "shards", "shard2")).getResults().getNumFound());
+
+ assertEquals(docCounts1.get() + docCounts2.get(), c.query(params("q", "*:*", "shards", "shard2,shard1")).getResults().getNumFound());
+ }
+
+ assertEquals(docCounts1.get(), shard1.query(params("q", "*:*", "distrib", "false")).getResults().getNumFound());
+ assertEquals(docCounts2.get(), shard2.query(params("q", "*:*", "distrib", "false")).getResults().getNumFound());
+
+ } catch (Exception sse) {
+ throw new RuntimeException(sse);
+ }
+ };
+ checkShardCounts.run();
+
+ { // Send a delete request for a doc on shard1 to core hosting shard1 with NO routing info
+ // Should delete (implicitly) since doc is (implicitly) located on this shard
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("1");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ docCounts1.decrementAndGet();
+ }
+ checkShardCounts.run();
+
+ { // Send a delete request to core hosting shard1 with a route param for a document that is actually in shard2
+ // Should delete.
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("4").withRoute("shard2");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ docCounts2.decrementAndGet();
+ }
+ checkShardCounts.run();
+
+ { // Send a delete request to core hosting shard1 with NO route param for a document that is actually in shard2
+ // Shouldn't delete, since deleteById requests are not broadcast to all shard leaders.
+ // (This is effictively a request to delete "5" if an only if it is on shard1)
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("5");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ }
+ checkShardCounts.run();
+
+ { // Multiple deleteById commands for different shards in a single request
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("2", "shard1");
+ deleteRequest.deleteById("5", "shard2");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ docCounts1.decrementAndGet();
+ docCounts2.decrementAndGet();
+ }
+ checkShardCounts.run();
+ }
+
+ }
+
+ public void testDeleteByIdCompositeRouterWithRouterField() throws Exception {
+ final CloudSolrClient cloudClient = cluster.getSolrClient();
+ final String name = "composite_collection_with_routerfield_" + NAME_COUNTER.getAndIncrement();
+ assertEquals(RequestStatusState.COMPLETED,
+ CollectionAdminRequest.createCollection(name, "_default", 2, 2)
+ .setRouterName("compositeId")
+ .setRouterField("routefield_s")
+ .setShards("shard1,shard2")
+ .processAndWait(cloudClient, DEFAULT_TIMEOUT));
+ cloudClient.waitForState(name, DEFAULT_TIMEOUT, TimeUnit.SECONDS,
+ (n, c) -> DocCollection.isFullyActive(n, c, 2, 2));
+ cloudClient.setDefaultCollection(name);
+
+ final DocCollection docCol = cloudClient.getZkStateReader().getClusterState().getCollection(name);
+ try (SolrClient shard1 = getHttpSolrClient(docCol.getSlice("shard1").getLeader().getCoreUrl());
+ SolrClient shard2 = getHttpSolrClient(docCol.getSlice("shard2").getLeader().getCoreUrl())) {
+
+ // Add three documents w/diff routes (all sent to shard1 leader's core)
+ shard1.add(sdoc("id", "1", "routefield_s", "europe"));
+ shard1.add(sdoc("id", "3", "routefield_s", "europe"));
+ shard1.add(sdoc("id", "5", "routefield_s", "africa"));
+ shard1.commit();
+
+ // Add two documents w/diff routes (all sent to shard2 leader's core)
+ shard2.add(sdoc("id", "4", "routefield_s", "africa"));
+ shard2.add(sdoc("id", "2", "routefield_s", "europe"));
+ shard2.commit();
+
+ final AtomicInteger docCountsEurope = new AtomicInteger(3);
+ final AtomicInteger docCountsAfrica = new AtomicInteger(2);
+
+ // A re-usable helper to verify that the expected number of documents can be found based on _route_ key...
+ Runnable checkShardCounts = () -> {
+ try {
+ // including cloudClient helps us test view from other nodes that aren't the leaders...
+ for (SolrClient c : Arrays.asList(cloudClient, shard1, shard2)) {
+ assertEquals(docCountsEurope.get() + docCountsAfrica.get(), c.query(params("q", "*:*")).getResults().getNumFound());
+
+ assertEquals(docCountsEurope.get(), c.query(params("q", "*:*", "_route_", "europe")).getResults().getNumFound());
+ assertEquals(docCountsAfrica.get(), c.query(params("q", "*:*", "_route_", "africa")).getResults().getNumFound());
+ }
+ } catch (Exception sse) {
+ throw new RuntimeException(sse);
+ }
+ };
+ checkShardCounts.run();
+
+ { // Send a delete request to core hosting shard1 with a route param for a document that was originally added via core on shard2
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("4", "africa");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ docCountsAfrica.decrementAndGet();
+ }
+ checkShardCounts.run();
+
+ { // Multiple deleteById commands with different routes in a single request
+ final UpdateRequest deleteRequest = new UpdateRequest();
+ deleteRequest.deleteById("2", "europe");
+ deleteRequest.deleteById("5", "africa");
+ shard1.request(deleteRequest);
+ shard1.commit();
+ docCountsEurope.decrementAndGet();
+ docCountsAfrica.decrementAndGet();
+ }
+ checkShardCounts.run();
+ }
+ }
public void testThatCantForwardToLeaderFails() throws Exception {
final CloudSolrClient cloudClient = cluster.getSolrClient();
diff --git a/solr/core/src/test/org/apache/solr/update/DeleteByIdWithRouterFieldTest.java b/solr/core/src/test/org/apache/solr/update/DeleteByIdWithRouterFieldTest.java
new file mode 100644
index 0000000..af16af4
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/update/DeleteByIdWithRouterFieldTest.java
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.update;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.TestUtil;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.impl.LBSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.cloud.CloudInspectUtil;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+public class DeleteByIdWithRouterFieldTest extends SolrCloudTestCase {
+
+ public static final String COLL = "test";
+ public static final String ROUTE_FIELD = "field_s";
+ public static final int NUM_SHARDS = 3;
+
+ private static final List<SolrClient> clients = new ArrayList<>(); // not CloudSolrClient
+
+ /**
+ * A randomized prefix to put on every route value.
+ * This helps ensure that documents wind up on diff shards between diff test runs
+ */
+ private static String RVAL_PRE = null;
+
+ @BeforeClass
+ public static void setupClusterAndCollection() throws Exception {
+ RVAL_PRE = TestUtil.randomRealisticUnicodeString(random());
+
+ // sometimes use 2 replicas of every shard so we hit more interesting update code paths
+ final int numReplicas = usually() ? 1 : 2;
+
+ configureCluster(1 + (NUM_SHARDS * numReplicas) ) // we'll have one node that doesn't host any replicas
+ .addConfig("conf", configset("cloud-minimal"))
+ .configure();
+
+ assertTrue(CollectionAdminRequest.createCollection(COLL, "conf", NUM_SHARDS, numReplicas)
+ .setRouterField(ROUTE_FIELD)
+ .process(cluster.getSolrClient())
+ .isSuccess());
+
+ cluster.getSolrClient().setDefaultCollection(COLL);
+
+ ClusterState clusterState = cluster.getSolrClient().getClusterStateProvider().getClusterState();
+ for (Replica replica : clusterState.getCollection(COLL).getReplicas()) {
+ clients.add(getHttpSolrClient(replica.getCoreUrl()));
+ }
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ IOUtils.close(clients);
+ clients.clear();
+ RVAL_PRE = null;
+ }
+
+ @After
+ public void checkShardConsistencyAndCleanUp() throws Exception {
+ checkShardsConsistentNumFound();
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteByQuery("*:*")
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(cluster.getSolrClient())
+ .getStatus());
+ }
+
+ private void checkShardsConsistentNumFound() throws Exception {
+ final SolrParams params = params("q", "*:*", "distrib", "false");
+ final DocCollection collection = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(COLL);
+ for (Map.Entry<String,Slice> entry : collection.getActiveSlicesMap().entrySet()) {
+ final String shardName = entry.getKey();
+ final Slice slice = entry.getValue();
+ final Replica leader = entry.getValue().getLeader();
+ try (SolrClient leaderClient = getHttpSolrClient(leader.getCoreUrl())) {
+ final SolrDocumentList leaderResults = leaderClient.query(params).getResults();
+ for (Replica replica : slice) {
+ try (SolrClient replicaClient = getHttpSolrClient(replica.getCoreUrl())) {
+ final SolrDocumentList replicaResults = replicaClient.query(params).getResults();
+ assertEquals("inconsistency w/leader: shard=" + shardName + "core=" + replica.getCoreName(),
+ Collections.emptySet(),
+ CloudInspectUtil.showDiff(leaderResults, replicaResults,
+ shardName + " leader: " + leader.getCoreUrl(),
+ shardName + ": " + replica.getCoreUrl()));
+ }
+ }
+ }
+ }
+ }
+
+ private SolrClient getRandomSolrClient() {
+ final int index = random().nextInt(clients.size() + 1);
+ return index == clients.size() ? cluster.getSolrClient() : clients.get(index);
+ }
+
+ /**
+ * 100 docs with 2 digit ids and a route field that matches the last digit
+ * @see #del100Docs
+ */
+ private UpdateRequest add100Docs() {
+ final UpdateRequest adds = new UpdateRequest();
+ for (int x = 0; x <= 9; x++) {
+ for (int y = 0; y <= 9; y++) {
+ adds.add("id", x+""+y, ROUTE_FIELD, RVAL_PRE+y);
+ }
+ }
+ return adds;
+ }
+
+ /**
+ * 100 doc deletions with 2 digit ids and a route field that matches the last digit
+ * @see #add100Docs
+ */
+ private UpdateRequest del100Docs() {
+ final UpdateRequest dels = new UpdateRequest();
+ for (int x = 0; x <= 9; x++) {
+ for (int y = 0; y <= 9; y++) {
+ dels.deleteById(x+""+y, RVAL_PRE+y);
+ }
+ }
+ return dels;
+ }
+
+ public void testBlocksOfDeletes() throws Exception {
+
+ assertEquals(0, add100Docs().setAction(UpdateRequest.ACTION.COMMIT, true, true).process(getRandomSolrClient()).getStatus());
+ assertEquals(100, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ // sanity check a "block" of 1 delete
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteById("06", RVAL_PRE+"6")
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(getRandomSolrClient())
+ .getStatus());
+ assertEquals(99, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+
+ // block of 2 deletes w/diff routes
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteById("17", RVAL_PRE+"7")
+ .deleteById("18", RVAL_PRE+"8")
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(getRandomSolrClient())
+ .getStatus());
+ assertEquals(97, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+
+ // block of 2 deletes using single 'withRoute()' for both
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteById("29")
+ .deleteById("39")
+ .withRoute(RVAL_PRE+"9")
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(getRandomSolrClient())
+ .getStatus());
+ assertEquals(95, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+
+ { // block of 2 deletes w/ diff routes that are conditional on optimistic concurrency
+ final Long v48 = (Long) getRandomSolrClient().query(params("q", "id:48", "fl", "_version_")).getResults().get(0).get("_version_");
+ final Long v49 = (Long) getRandomSolrClient().query(params("q", "id:49", "fl", "_version_")).getResults().get(0).get("_version_");
+
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteById("48", RVAL_PRE+"8", v48)
+ .deleteById("49", RVAL_PRE+"9", v49)
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(getRandomSolrClient())
+ .getStatus());
+ }
+ assertEquals(93, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+
+ { // block of 2 deletes using single 'withRoute()' for both that are conditional on optimistic concurrency
+ final Long v58 = (Long) getRandomSolrClient().query(params("q", "id:58", "fl", "_version_")).getResults().get(0).get("_version_");
+ final Long v68 = (Long) getRandomSolrClient().query(params("q", "id:68", "fl", "_version_")).getResults().get(0).get("_version_");
+
+ assertEquals(0,
+ new UpdateRequest()
+ .deleteById("58", v58)
+ .deleteById("68", v68)
+ .withRoute(RVAL_PRE+"8")
+ .setAction(UpdateRequest.ACTION.COMMIT, true, true)
+ .process(getRandomSolrClient())
+ .getStatus());
+ }
+ assertEquals(91, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+
+ // now delete all docs, including the ones we already deleted (shouldn't cause any problems)
+ assertEquals(0, del100Docs().setAction(UpdateRequest.ACTION.COMMIT, true, true).process(getRandomSolrClient()).getStatus());
+ assertEquals(0, getRandomSolrClient().query(params("q", "*:*")).getResults().getNumFound());
+
+ checkShardsConsistentNumFound();
+ }
+
+
+ /**
+ * Test that {@link UpdateRequest#getRoutesToCollection} correctly populates routes for all deletes
+ */
+ public void testGlassBoxUpdateRequestRoutesToShards() throws Exception {
+
+ final DocCollection docCol = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(COLL);
+ // we don't need "real" urls for all replicas, just something we can use as lookup keys for verification
+ // so we'll use the shard names as "leader urls"
+ final Map<String,List<String>> urlMap = docCol.getActiveSlices().stream().collect
+ (Collectors.toMap(s -> s.getName(), s -> Collections.singletonList(s.getName())));
+
+ // simplified rote info we'll build up with the shards for each delete (after sanity checking they have routing info at all)...
+ final Map<String,String> actualDelRoutes = new LinkedHashMap<>();
+
+ final Map<String,LBSolrClient.Req> rawDelRoutes = del100Docs().getRoutesToCollection(docCol.getRouter(), docCol, urlMap, params(), ROUTE_FIELD);
+ for (LBSolrClient.Req lbreq : rawDelRoutes.values()) {
+ assertTrue(lbreq.getRequest() instanceof UpdateRequest);
+ final String shard = lbreq.getServers().get(0);
+ final UpdateRequest req = (UpdateRequest) lbreq.getRequest();
+ for (Map.Entry<String,Map<String,Object>> entry : req.getDeleteByIdMap().entrySet()) {
+ final String id = entry.getKey();
+ // quick sanity checks...
+ assertNotNull(id + " has null values", entry.getValue());
+ final Object route = entry.getValue().get(ShardParams._ROUTE_);
+ assertNotNull(id + " has null route value", route);
+ assertEquals("route value is wrong for id: " + id, RVAL_PRE + id.substring(id.length() - 1), route.toString());
+
+ actualDelRoutes.put(id, shard);
+ }
+ }
+
+ // look at the routes computed from the "adds" as the expected value for the routes of each "del"
+ for (SolrInputDocument doc : add100Docs().getDocuments()) {
+ final String id = doc.getFieldValue("id").toString();
+ final String actualShard = actualDelRoutes.get(id);
+ assertNotNull(id + " delete is missing?", actualShard);
+ final Slice expectedShard = docCol.getRouter().getTargetSlice(id, doc, doc.getFieldValue(ROUTE_FIELD).toString(), params(), docCol);
+ assertNotNull(id + " add route is null?", expectedShard);
+ assertEquals("Wrong shard for delete of id: " + id,
+ expectedShard.getName(), actualShard);
+ }
+
+ // sanity check no one broke our test and made it a waste of time
+ assertEquals(100, add100Docs().getDocuments().size());
+ assertEquals(100, actualDelRoutes.entrySet().size());
+ }
+
+}
+
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java
index 6285ce6..cf8a473 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java
@@ -165,10 +165,11 @@ public class JavaBinUpdateRequestCodec {
Map<String,Object> params = entry.getValue();
if (params != null) {
Long version = (Long) params.get(UpdateRequest.VER);
- if (params.containsKey(ShardParams._ROUTE_))
- updateRequest.deleteById(entry.getKey(), (String) params.get(ShardParams._ROUTE_));
- else
- updateRequest.deleteById(entry.getKey(), version);
+ if (params.containsKey(ShardParams._ROUTE_)) {
+ updateRequest.deleteById(entry.getKey(), (String) params.get(ShardParams._ROUTE_), version);
+ } else {
+ updateRequest.deleteById(entry.getKey(), version);
+ }
} else {
updateRequest.deleteById(entry.getKey());
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java
index 4c86a13..5c07bce 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java
@@ -312,10 +312,12 @@ public class UpdateRequest extends AbstractUpdateRequest {
String deleteId = entry.getKey();
Map<String,Object> map = entry.getValue();
Long version = null;
+ String route = null;
if (map != null) {
version = (Long) map.get(VER);
+ route = (String) map.get(_ROUTE_);
}
- Slice slice = router.getTargetSlice(deleteId, null, null, null, col);
+ Slice slice = router.getTargetSlice(deleteId, null, route, null, col);
if (slice == null) {
return null;
}
@@ -327,11 +329,11 @@ public class UpdateRequest extends AbstractUpdateRequest {
T request = routes.get(leaderUrl);
if (request != null) {
UpdateRequest urequest = (UpdateRequest) request.getRequest();
- urequest.deleteById(deleteId, version);
+ urequest.deleteById(deleteId, route, version);
} else {
UpdateRequest urequest = new UpdateRequest();
urequest.setParams(params);
- urequest.deleteById(deleteId, version);
+ urequest.deleteById(deleteId, route, version);
urequest.setCommitWithin(getCommitWithin());
urequest.setBasicAuthCredentials(getBasicAuthUser(), getBasicAuthPassword());
request = reqSupplier.get(urequest, urls);