This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new b868e030986 SOLR-17869: Avoid creating grouping shard requests when
timeAllowed is exhausted (cherry-pick from main). (#3698)
b868e030986 is described below
commit b868e03098660c9cd3adbd8eb3e34d55d297eb52
Author: Andrzej Białecki <[email protected]>
AuthorDate: Thu Sep 25 16:32:13 2025 +0200
SOLR-17869: Avoid creating grouping shard requests when timeAllowed is
exhausted (cherry-pick from main). (#3698)
---
solr/CHANGES.txt | 2 +
.../TopGroupsShardRequestFactory.java | 22 ++--
.../org/apache/solr/TestDistributedGrouping.java | 111 +++++++++++++++++----
3 files changed, 112 insertions(+), 23 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index f8fc9da446f..8af764058bf 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -42,6 +42,8 @@ Bug Fixes
* SOLR-17837: PULL replica nodes could be marked as "preferredLeader" by
BALANCESHARDUNIQUE despite never being able to be elected leader (Kevin Liang
via Houston Putman)
+* SOLR-17869: Avoid creating grouping shard requests when timeAllowed has
already run out. (Andrzej Bialecki, hossman)
+
Dependency Upgrades
---------------------
(No changes)
diff --git
a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/TopGroupsShardRequestFactory.java
b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/TopGroupsShardRequestFactory.java
index 22127f76dfc..fb795d21d62 100644
---
a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/TopGroupsShardRequestFactory.java
+++
b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/TopGroupsShardRequestFactory.java
@@ -83,6 +83,22 @@ public class TopGroupsShardRequestFactory implements
ShardRequestFactory {
sreq.purpose = ShardRequest.PURPOSE_GET_TOP_IDS;
sreq.params = new ModifiableSolrParams(rb.req.getParams());
+ // TODO: should eventually all components that send sub-requests be
subject to similar
+ // logic if we are running with timeAllowed?
+ int origTimeAllowed = sreq.params.getInt(CommonParams.TIME_ALLOWED, -1);
+ if (origTimeAllowed > 0) {
+ int remainingTimeAllowed = origTimeAllowed - rb.firstPhaseElapsedTime;
+ // at least 1 ms to be useful
+ if (remainingTimeAllowed <= 1) {
+ // We've already used up our time budget
+ rb.rsp.setPartialResults(rb.req);
+ rb.rsp.addPartialResponseDetail(
+ getClass().getSimpleName() + ": timeAllowed exhausted in first
phase");
+ return new ShardRequest[0];
+ }
+ sreq.params.set(CommonParams.TIME_ALLOWED, remainingTimeAllowed);
+ }
+
// If group.format=simple group.offset doesn't make sense
Grouping.Format responseFormat = rb.getGroupingSpec().getResponseFormat();
if (responseFormat == Grouping.Format.simple ||
rb.getGroupingSpec().isMain()) {
@@ -132,12 +148,6 @@ public class TopGroupsShardRequestFactory implements
ShardRequestFactory {
sreq.params.set(CommonParams.FL, schema.getUniqueKeyField().getName());
}
- int origTimeAllowed = sreq.params.getInt(CommonParams.TIME_ALLOWED, -1);
- if (origTimeAllowed > 0) {
- sreq.params.set(
- CommonParams.TIME_ALLOWED, Math.max(1, origTimeAllowed -
rb.firstPhaseElapsedTime));
- }
-
return new ShardRequest[] {sreq};
}
}
diff --git a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
index ae55c613c26..c6ec79d0c0d 100644
--- a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
+++ b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
@@ -18,11 +18,9 @@ package org.apache.solr;
import static org.hamcrest.CoreMatchers.containsString;
-import java.io.IOException;
import java.util.List;
import org.apache.solr.SolrTestCaseJ4.SuppressPointFields;
import org.apache.solr.client.solrj.SolrClient;
-import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
@@ -30,6 +28,13 @@ import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.component.QueryComponent;
+import org.apache.solr.handler.component.ResponseBuilder;
+import org.apache.solr.handler.component.ShardRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import
org.apache.solr.search.grouping.distributed.requestfactory.TopGroupsShardRequestFactory;
+import org.junit.BeforeClass;
import org.junit.Test;
/**
@@ -41,6 +46,11 @@ import org.junit.Test;
@SuppressPointFields(bugUrl =
"https://issues.apache.org/jira/browse/SOLR-10844";)
public class TestDistributedGrouping extends BaseDistributedSearchTestCase {
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ initCore("solrconfig-delaying-component.xml", "schema.xml");
+ }
+
public TestDistributedGrouping() {
// SOLR-10844: Even with points suppressed, this test breaks if we
(randomize) docvalues="true"
// on trie fields?!?!?!!?
@@ -1253,19 +1263,48 @@ public class TestDistributedGrouping extends
BaseDistributedSearchTestCase {
for (String rows : new String[] {"10", "0"}) {
simpleQuery(
"q", "*:*", "group", "true", "group.field", i1, "group.ngroups",
ngroups, "rows", rows);
- simpleQuery(
- "q",
- "*:*",
- "group",
- "true",
- "group.field",
- i1,
- "group.ngroups",
- ngroups,
- "rows",
- rows,
- "timeAllowed",
- "123456");
+ // delaying component introduces a delay longer than timeAllowed
+ QueryResponse rsp =
+ simpleQuery(
+ "q",
+ t1 + ":eggs",
+ "group",
+ "true",
+ "group.field",
+ i1,
+ "group.ngroups",
+ ngroups,
+ "rows",
+ rows,
+ "cache",
+ "false",
+ "timeAllowed",
+ "200",
+ "sleep",
+ "300");
+ assertTrue(
+ "header: " + rsp.getHeader(),
SolrQueryResponse.isPartialResults(rsp.getHeader()));
+ //
+ rsp =
+ simpleQuery(
+ "q",
+ t1 + ":eggs",
+ "group",
+ "true",
+ "group.field",
+ i1,
+ "group.ngroups",
+ ngroups,
+ "rows",
+ rows,
+ "cache",
+ "false",
+ "timeAllowed",
+ "200",
+ "sleep",
+ "10");
+ assertFalse(
+ "header: " + rsp.getHeader(),
SolrQueryResponse.isPartialResults(rsp.getHeader()));
}
}
@@ -1652,13 +1691,51 @@ public class TestDistributedGrouping extends
BaseDistributedSearchTestCase {
"true");
}
- private void simpleQuery(Object... queryParams) throws SolrServerException,
IOException {
+ @Test
+ public void testShardRequestFactory() throws Exception {
+ SolrQueryRequest req =
+ req(
+ "q",
+ "*:*",
+ "rows",
+ "100",
+ "fl",
+ "id," + i1,
+ "group",
+ "true",
+ "group.field",
+ i1,
+ "group.limit",
+ "-1",
+ "sort",
+ i1 + " asc, id asc",
+ "timeAllowed",
+ "200");
+ try {
+ SolrQueryResponse rsp = new SolrQueryResponse();
+ ResponseBuilder rb = new ResponseBuilder(req, rsp, List.of());
+ new QueryComponent().prepare(rb);
+ rb.setNeedDocSet(true);
+
+ TopGroupsShardRequestFactory f = new TopGroupsShardRequestFactory();
+ ShardRequest[] sreq = f.constructRequest(rb);
+ assertTrue(sreq.length > 0);
+
+ rb.firstPhaseElapsedTime = 200; // simulate timeout
+ sreq = f.constructRequest(rb);
+ assertEquals(0, sreq.length);
+ } finally {
+ req.close();
+ }
+ }
+
+ private QueryResponse simpleQuery(Object... queryParams) throws Exception {
ModifiableSolrParams params = new ModifiableSolrParams();
for (int i = 0; i < queryParams.length; i += 2) {
params.add(queryParams[i].toString(), queryParams[i + 1].toString());
}
params.set("shards", shards);
- queryServer(params);
+ return queryServer(params);
}
/**
