diff --git a/changelog/unreleased/SOLR-18114.yml b/changelog/unreleased/SOLR-18114.yml new file mode 100644 index 00000000000..cd0e66a96cb --- /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 1a41044b793..277029deaf5 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; } } @@ -1809,6 +1812,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 +1838,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 242bfd4261e..a2693167833 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,17 +18,22 @@ import java.io.IOException; import java.util.ArrayList; +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; 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; 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; @@ -130,4 +135,58 @@ 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"; + + final String docId = String.valueOf(random().nextInt(1000)); + final String routeValue = TestUtil.randomRealisticUnicodeString(random()); + + 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", docId, "router_field_s", routeValue); + addRequest.process(client); + client.commit(); + + assertEquals(1, 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(); + } + } + } }