This is an automated email from the ASF dual-hosted git repository.
hossman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 5c3465e 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.
5c3465e is described below
commit 5c3465eb498551c53dc310e44ae82cb89890ffc6
Author: Chris Hostetter <[email protected]>
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.
---
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 01c697b..7261e75 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -420,6 +420,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-15566: Clarify ref guide documentation about SQL queries with `SELECT
*` requiring a `LIMIT` clause (Timothy Potter)
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 fd475ac..e031cf7 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
@@ -575,7 +575,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 381cce8..65937d7 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 8147cc2..8bcb1d7 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
@@ -162,10 +162,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 39c0d07..20bae0e 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
@@ -305,10 +305,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;
}
@@ -320,11 +322,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);
