diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 90c2ceb2931..990fcb0ea2a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -535,6 +535,8 @@ Other Changes * SOLR-8283: factor out StrParser from QueryParsing.StrParser and SortSpecParsing[Test] from QueryParsing[Test] (Christine Poerschke) +* SOLR-8298: small preferLocalShards implementation refactor (Christine Poerschke) + ================== 5.3.1 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java index 8f9c901c0be..05f54580e94 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java @@ -114,11 +114,13 @@ public class HttpShardHandler extends ShardHandler { // Not thread safe... don't use in Callable. // Don't modify the returned URL list. - private List getURLs(ShardRequest sreq, String shard) { + private List getURLs(String shard, String preferredHostAddress) { List urls = shardToURLs.get(shard); if (urls == null) { urls = httpShardHandlerFactory.makeURLList(shard); - preferCurrentHostForDistributedReq(sreq, urls); + if (preferredHostAddress != null && urls.size() > 1) { + preferCurrentHostForDistributedReq(preferredHostAddress, urls); + } shardToURLs.put(shard, urls); } return urls; @@ -131,27 +133,7 @@ public class HttpShardHandler extends ShardHandler { * If all nodes prefer local-cores then a bad/heavily-loaded node will receive less requests from healthy nodes. * This will help prevent a distributed deadlock or timeouts in all the healthy nodes due to one bad node. */ - private void preferCurrentHostForDistributedReq(final ShardRequest sreq, final List urls) { - if (sreq == null || sreq.rb == null || sreq.rb.req == null || urls == null || urls.size() <= 1) - return; - - SolrQueryRequest req = sreq.rb.req; - - // determine if we should apply the local preference - if (!req.getOriginalParams().getBool(CommonParams.PREFER_LOCAL_SHARDS, false)) - return; - - // Get this node's base URL from ZK - SolrCore core = req.getCore(); - ZkController zkController = (core != null) ? core.getCoreDescriptor().getCoreContainer().getZkController() : null; - String currentHostAddress = (zkController != null) ? zkController.getBaseUrl() : null; - if (currentHostAddress == null) { - log.debug("Couldn't determine current host address to prefer local shards " + - "because either core is null? {} or there is no ZkController? {}", - Boolean.valueOf(core == null), Boolean.valueOf(zkController == null)); - return; - } - + private void preferCurrentHostForDistributedReq(final String currentHostAddress, final List urls) { if (log.isDebugEnabled()) log.debug("Trying to prefer local shard on {} among the urls: {}", currentHostAddress, Arrays.toString(urls.toArray())); @@ -174,9 +156,9 @@ public class HttpShardHandler extends ShardHandler { } @Override - public void submit(final ShardRequest sreq, final String shard, final ModifiableSolrParams params) { + public void submit(final ShardRequest sreq, final String shard, final ModifiableSolrParams params, String preferredHostAddress) { // do this outside of the callable for thread safety reasons - final List urls = getURLs(sreq, shard); + final List urls = getURLs(shard, preferredHostAddress); Callable task = new Callable() { @Override @@ -335,6 +317,12 @@ public class HttpShardHandler extends ShardHandler { CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor(); ZkController zkController = coreDescriptor.getCoreContainer().getZkController(); + if (params.getBool(CommonParams.PREFER_LOCAL_SHARDS, false)) { + rb.preferredHostAddress = (zkController != null) ? zkController.getBaseUrl() : null; + if (rb.preferredHostAddress == null) { + log.warn("Couldn't determine current host address to prefer local shards"); + } + } if (shards != null) { List lst = StrUtils.splitSmart(shards, ",", true); diff --git a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java b/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java index 46f5718b945..48e767c3cdb 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java @@ -136,6 +136,7 @@ public class ResponseBuilder public int shards_start = -1; public List outgoing; // requests to be sent public List finished; // requests that have received responses from all shards + public String preferredHostAddress = null; public String shortCircuitedURL; public int getShardNum(String shard) { @@ -147,7 +148,6 @@ public class ResponseBuilder public void addRequest(SearchComponent me, ShardRequest sreq) { outgoing.add(sreq); - sreq.rb = this; if ((sreq.purpose & ShardRequest.PURPOSE_PRIVATE) == 0) { // if this isn't a private request, let other components modify it. for (SearchComponent component : components) { diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 3c01eb8bb3c..71f144ec9fe 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -370,7 +370,7 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware , params.remove(CommonParams.QT); } } - shardHandler1.submit(sreq, shard, params); + shardHandler1.submit(sreq, shard, params, rb.preferredHostAddress); } } diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardHandler.java b/solr/core/src/java/org/apache/solr/handler/component/ShardHandler.java index 22dee759726..e239e02bb22 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ShardHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ShardHandler.java @@ -21,7 +21,10 @@ import org.apache.solr.common.params.ModifiableSolrParams; public abstract class ShardHandler { public abstract void prepDistributed(ResponseBuilder rb); - public abstract void submit(ShardRequest sreq, String shard, ModifiableSolrParams params) ; + public void submit(ShardRequest sreq, String shard, ModifiableSolrParams params) { + submit(sreq, shard, params, null); + } + public abstract void submit(ShardRequest sreq, String shard, ModifiableSolrParams params, String preferredHostAddress); public abstract ShardResponse takeCompletedIncludingErrors(); public abstract ShardResponse takeCompletedOrError(); public abstract void cancelAll(); diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java b/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java index 0248190c04d..f7c05d236bf 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java @@ -49,7 +49,6 @@ public class ShardRequest { public ModifiableSolrParams params; - public ResponseBuilder rb; /** list of responses... filled out by framework */ public List responses = new ArrayList<>(); diff --git a/solr/core/src/test/org/apache/solr/core/MockShardHandlerFactory.java b/solr/core/src/test/org/apache/solr/core/MockShardHandlerFactory.java index 6b3a5480524..b867b85ccfb 100644 --- a/solr/core/src/test/org/apache/solr/core/MockShardHandlerFactory.java +++ b/solr/core/src/test/org/apache/solr/core/MockShardHandlerFactory.java @@ -43,7 +43,7 @@ public class MockShardHandlerFactory extends ShardHandlerFactory implements Plug @Override public void submit(ShardRequest sreq, String shard, - ModifiableSolrParams params) {} + ModifiableSolrParams params, String preferredHostAddress) {} @Override public ShardResponse takeCompletedIncludingErrors() { diff --git a/solr/test-framework/src/java/org/apache/solr/handler/component/TrackingShardHandlerFactory.java b/solr/test-framework/src/java/org/apache/solr/handler/component/TrackingShardHandlerFactory.java index 81708abeed8..f4cd9b2f9ab 100644 --- a/solr/test-framework/src/java/org/apache/solr/handler/component/TrackingShardHandlerFactory.java +++ b/solr/test-framework/src/java/org/apache/solr/handler/component/TrackingShardHandlerFactory.java @@ -92,7 +92,7 @@ public class TrackingShardHandlerFactory extends HttpShardHandlerFactory { } @Override - public void submit(ShardRequest sreq, String shard, ModifiableSolrParams params) { + public void submit(ShardRequest sreq, String shard, ModifiableSolrParams params, String preferredHostAddress) { synchronized (TrackingShardHandlerFactory.this) { if (isTracking()) { queue.offer(new ShardRequestAndParams(sreq, shard, params));