dsmiley commented on code in PR #2493:
URL: https://github.com/apache/solr/pull/2493#discussion_r1697681291
##########
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##########
@@ -382,4 +397,14 @@ private boolean canShortCircuit(
public ShardHandlerFactory getShardHandlerFactory() {
return httpShardHandlerFactory;
}
+
+ // test helper function
Review Comment:
see `com.google.common.annotations.VisibleForTesting` instead
##########
solr/core/src/test/org/apache/solr/handler/component/TestHttpShardHandlerFactory.java:
##########
@@ -154,4 +156,63 @@ public void testLiveNodesToHostUrl() {
assertThat(hostSet, hasItem("1.2.3.4:9000"));
assertThat(hostSet, hasItem("1.2.3.4:9001"));
}
+
+ public void testHttpShardHandlerWithResponse() {
+ HttpShardHandlerFactory httpShardHandlerFactory = new
HttpShardHandlerFactory();
+ HttpShardHandler shardHandler = (HttpShardHandler)
httpShardHandlerFactory.getShardHandler();
+
+ long timeAllowedInMillis = -1;
+ // setting one pending request.
+ shardHandler.setPendingRequest(1);
+
+ ShardResponse shardResponse = new ShardResponse();
+ shardResponse.setShard("shard_1");
+ ShardRequest shardRequest = new ShardRequest();
+ // one shard
+ shardRequest.actualShards = new String[] {"shard_1"};
+ shardResponse.setShardRequest(shardRequest);
+
+ ExecutorService exec =
ExecutorUtil.newMDCAwareCachedThreadPool("timeAllowedTest");
+ try {
+ // generating shardresponse for one shard
+ exec.submit(() -> shardHandler.setResponse(shardResponse));
+ } finally {
+ ExecutorUtil.shutdownAndAwaitTermination(exec);
+ }
+ ShardResponse gotResponse =
+
shardHandler.takeCompletedIncludingErrorsWithTimeout(timeAllowedInMillis);
+
+ assertEquals(shardResponse, gotResponse);
+ }
+
+ @Test
+ public void testHttpShardHandlerWithPartialResponse() {
Review Comment:
These tests might be flaky on slower build machines.
One trick is to put `{!func v=sleep(200,1)}` for "q" param, which means to
sleep 200 milliseconds at the time the underlying query is parsed, and score as
"1". Basically, you can intentionally make the query take longer at the
shard(s) it's executed. For this PR, it'd be nice if we could conditionally
have some shards do this but not others, but this trick can't do that. If it
slept at each matching doc, then it could... hmmmm.
##########
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java:
##########
@@ -492,6 +492,7 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
} else {
// a distributed request
+ long maxTimeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED,
24 * 60 * 60 * 1000);
Review Comment:
if there is a default time allowed, as you are trying to do here, use a
constant field initialized with EnvUtils so that it's changeable when starting
Solr (perhaps `solr.search.distrib.timeAllowedDefault`. And comment the
default value so we readily see it in human terms.
##########
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##########
@@ -211,23 +216,33 @@ public ShardResponse takeCompletedIncludingErrors() {
*/
@Override
public ShardResponse takeCompletedOrError() {
- return take(true);
+ return take(true, -1);
}
- private ShardResponse take(boolean bailOnError) {
+ private ShardResponse take(boolean bailOnError, long maxAllowedTimeInMillis)
{
try {
- while (pending.get() > 0) {
- ShardResponse rsp = responses.take();
- responseFutureMap.remove(rsp);
+ long deadline = System.nanoTime();
+ if (maxAllowedTimeInMillis > 0) {
+ deadline += TimeUnit.MILLISECONDS.toNanos(maxAllowedTimeInMillis);
+ } else {
+ deadline = System.nanoTime() + TimeUnit.DAYS.toNanos(1);
+ }
+ ShardResponse previousResponse = null;
+ while (pending.get() > 0) {
+ long waitTime = deadline - System.nanoTime();
+ ShardResponse rsp = responses.poll(waitTime, TimeUnit.NANOSECONDS);
pending.decrementAndGet();
+ if (rsp == null) return previousResponse;
Review Comment:
definitely for another JIRA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
