From f3532eab6935a7d4c22522ebc353b16afe2f91e4 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 16 Mar 2026 16:35:52 -0400 Subject: [PATCH 1/4] Check deletes for routing info --- .../client/solrj/impl/CloudSolrClient.java | 14 +++++++ .../impl/CloudSolrClientRoutingTest.java | 38 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 1a41044b7939..0cc2537d0100 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -1809,6 +1809,10 @@ public Map getShardReplicationFactor(String collection, NamedLi return results; } + /** + * Determines whether an UpdateRequest contains sufficient routing information to identify shard + * leaders for direct updates when directUpdatesToLeadersOnly is enabled. + */ private static boolean hasInfoToFindLeaders(UpdateRequest updateRequest, String idField) { final Map> documents = updateRequest.getDocumentsMap(); final Map> deleteById = updateRequest.getDeleteByIdMap(); @@ -1831,6 +1835,16 @@ private static boolean hasInfoToFindLeaders(UpdateRequest updateRequest, String } } + if (deleteById != null) { + for (final Map.Entry> entry : deleteById.entrySet()) { + final Map params = entry.getValue(); + if (params == null || params.get(ShardParams._ROUTE_) == null) { + // deleteById entry lacks explicit route parameter, can't find leader for it + return false; + } + } + } + return true; } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java index 242bfd4261ea..f3c3a3e37c87 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java @@ -18,11 +18,14 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Optional; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.SolrQuery; +import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; @@ -130,4 +133,39 @@ public void routeParamHandling() throws IOException, SolrServerException { assertEquals(0, numForwardedWithRoute); assertEquals(100, numForwardedWithoutRoute); } + + @Test + public void testDeleteWithoutRouteWithDirectUpdatesToLeadersOnly() throws Exception { + final String collectionName = "delete_without_route_collection"; + + CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1) + .setRouterField("router_field_s") + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(collectionName, 2, 2); + + try (CloudSolrClient client = + new CloudSolrClient.Builder( + Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) + .withDefaultCollection(collectionName) + .sendUpdatesOnlyToShardLeaders() + .sendDirectUpdatesToShardLeadersOnly() + .build()) { + + UpdateRequest addRequest = new UpdateRequest(); + addRequest.add("id", "doc1", "router_field_s", "testRoute"); + addRequest.process(client); + client.commit(); + + assertEquals(1, client.query(new SolrQuery("id:doc1")).getResults().getNumFound()); + + // Delete by ID without providing explicit route + // Should still delete via sending to all shards + UpdateRequest deleteRequest = new UpdateRequest(); + deleteRequest.deleteById("doc1"); + deleteRequest.process(client); + client.commit(); + + assertEquals(0, client.query(new SolrQuery("id:doc1")).getResults().getNumFound()); + } + } } From 60dff97f68a0118d831f9ffb34a1d77d6374412d Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 19 Mar 2026 11:01:08 -0400 Subject: [PATCH 2/4] Address PR comments --- changelog/unreleased/SOLR-18114.yml | 8 ++++++++ .../solr/client/solrj/impl/CloudSolrClient.java | 3 +++ .../solrj/impl/CloudSolrClientRoutingTest.java | 12 ++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/SOLR-18114.yml diff --git a/changelog/unreleased/SOLR-18114.yml b/changelog/unreleased/SOLR-18114.yml new file mode 100644 index 000000000000..cd0e66a96cb7 --- /dev/null +++ b/changelog/unreleased/SOLR-18114.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Fixed CloudSolrClient deleteById failure when routing info is not passed with compositeId router, router.field, and directUpdatesToLeadersOnly enabled +type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Matthew Biscocho +links: + - name: SOLR-18114 + url: https://issues.apache.org/jira/browse/SOLR-18114 diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 0cc2537d0100..277029deaf54 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -772,6 +772,9 @@ private NamedList directUpdate(UpdateRequest request, String collection) "directUpdatesToLeadersOnly==true but could not find leader(s)"); } else { // we could not find a leader or routes yet - use unoptimized general path + log.warn( + "No routing info found for update to collection '{}', broadcasting to all shards.", + collection); return null; } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java index f3c3a3e37c87..a45fca67cd29 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -138,6 +139,9 @@ public void routeParamHandling() throws IOException, SolrServerException { public void testDeleteWithoutRouteWithDirectUpdatesToLeadersOnly() throws Exception { final String collectionName = "delete_without_route_collection"; + final String docId = TestUtil.randomRealisticUnicodeString(random()); + final String routeValue = TestUtil.randomRealisticUnicodeString(random()); + CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1) .setRouterField("router_field_s") .process(cluster.getSolrClient()); @@ -152,20 +156,20 @@ public void testDeleteWithoutRouteWithDirectUpdatesToLeadersOnly() throws Except .build()) { UpdateRequest addRequest = new UpdateRequest(); - addRequest.add("id", "doc1", "router_field_s", "testRoute"); + addRequest.add("id", docId, "router_field_s", routeValue); addRequest.process(client); client.commit(); - assertEquals(1, client.query(new SolrQuery("id:doc1")).getResults().getNumFound()); + assertEquals(1, client.query(new SolrQuery("id:" + docId)).getResults().getNumFound()); // Delete by ID without providing explicit route // Should still delete via sending to all shards UpdateRequest deleteRequest = new UpdateRequest(); - deleteRequest.deleteById("doc1"); + deleteRequest.deleteById(docId); deleteRequest.process(client); client.commit(); - assertEquals(0, client.query(new SolrQuery("id:doc1")).getResults().getNumFound()); + assertEquals(0, client.query(new SolrQuery("id:" + docId)).getResults().getNumFound()); } } } From d9f753a5f8919a569e09a96c0c4afc0e8b6b6a85 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 19 Mar 2026 17:26:15 -0400 Subject: [PATCH 3/4] Add logListener --- .../impl/CloudSolrClientRoutingTest.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java index a45fca67cd29..070e4ce04c06 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java @@ -33,6 +33,7 @@ import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.UpdateParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.util.LogListener; import org.junit.BeforeClass; import org.junit.Test; @@ -162,14 +163,30 @@ public void testDeleteWithoutRouteWithDirectUpdatesToLeadersOnly() throws Except assertEquals(1, client.query(new SolrQuery("id:" + docId)).getResults().getNumFound()); - // Delete by ID without providing explicit route - // Should still delete via sending to all shards - UpdateRequest deleteRequest = new UpdateRequest(); - deleteRequest.deleteById(docId); - deleteRequest.process(client); - client.commit(); - - assertEquals(0, client.query(new SolrQuery("id:" + docId)).getResults().getNumFound()); + try (LogListener warnLog = + LogListener.warn(CloudSolrClient.class) + .substring( + "No routing info found for update to collection '" + + collectionName + + "', broadcasting to all shards.")) { + + // Delete by ID without providing explicit route + // Should still delete via sending to all shards and log a warning + UpdateRequest deleteRequest = new UpdateRequest(); + deleteRequest.deleteById(docId); + deleteRequest.process(client); + client.commit(); + + assertEquals(0, client.query(new SolrQuery("id:" + docId)).getResults().getNumFound()); + + // Verify the warning was logged when routing info was missing + assertNotNull( + "Expected at least one warning about missing routing info for deleteById", + warnLog.pollMessage()); + + // Clear any remaining messages + warnLog.clearQueue(); + } } } } From 364d6cd000c125aad457954d41a681cc75852ece Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 20 Mar 2026 13:37:10 -0400 Subject: [PATCH 4/4] Use ints for docId --- .../solr/client/solrj/impl/CloudSolrClientRoutingTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java index 070e4ce04c06..a26931678337 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java @@ -140,7 +140,7 @@ public void routeParamHandling() throws IOException, SolrServerException { public void testDeleteWithoutRouteWithDirectUpdatesToLeadersOnly() throws Exception { final String collectionName = "delete_without_route_collection"; - final String docId = TestUtil.randomRealisticUnicodeString(random()); + final String docId = String.valueOf(random().nextInt(1000)); final String routeValue = TestUtil.randomRealisticUnicodeString(random()); CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)