From 566608604ad0f56b257743351ff67c7882dbc431 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 3 Aug 2026 14:22:19 -0400 Subject: [PATCH] SOLR-18330: RTG requests to TLOG can short-circuit Optimization: RTG requests to a TLOG leader replica shouldn't use an extra local HTTP hop. --- .../SOLR-18330-TLOG-RTG-shortCircuit.yml | 8 ++++ .../solr/handler/RealTimeGetHandler.java | 5 ++- .../handler/component/CloudReplicaSource.java | 13 +++--- .../handler/component/HttpShardHandler.java | 26 ++++++------ .../component/CloudReplicaSourceTest.java | 14 +++---- ...testRealTimeGetTlogLeaderShortCircuit.json | 19 +++++++++ .../opentelemetry/TestDistributedTracing.java | 40 +++++++++++++++++++ 7 files changed, 96 insertions(+), 29 deletions(-) create mode 100644 changelog/unreleased/SOLR-18330-TLOG-RTG-shortCircuit.yml create mode 100644 solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testRealTimeGetTlogLeaderShortCircuit.json diff --git a/changelog/unreleased/SOLR-18330-TLOG-RTG-shortCircuit.yml b/changelog/unreleased/SOLR-18330-TLOG-RTG-shortCircuit.yml new file mode 100644 index 000000000000..65b400b2a883 --- /dev/null +++ b/changelog/unreleased/SOLR-18330-TLOG-RTG-shortCircuit.yml @@ -0,0 +1,8 @@ +title: > + Optimization: RTG requests to a TLOG leader replica shouldn't use an extra local HTTP hop. +type: changed +authors: + - name: David Smiley +links: + - name: SOLR-18330 + url: https://issues.apache.org/jira/browse/SOLR-18330 diff --git a/solr/core/src/java/org/apache/solr/handler/RealTimeGetHandler.java b/solr/core/src/java/org/apache/solr/handler/RealTimeGetHandler.java index 5f1d04600903..15be93e15134 100644 --- a/solr/core/src/java/org/apache/solr/handler/RealTimeGetHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/RealTimeGetHandler.java @@ -38,8 +38,9 @@ protected List getDefaultComponents() { @Override public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - // Tell HttpShardHandlerthat this request should only be distributed to NRT replicas - req.getContext().put(HttpShardHandler.ONLY_NRT_REPLICAS, Boolean.TRUE); + // Tell HttpShardHandler that this request should only be distributed to replicas that serve the + // most up-to-date view of the data + req.getContext().put(HttpShardHandler.ONLY_RTG_REPLICAS, Boolean.TRUE); super.handleRequestBody(req, rsp); } diff --git a/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java b/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java index 5315d413f421..7932358f9de1 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java +++ b/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java @@ -171,10 +171,8 @@ private List findReplicas( .filter(replica -> replica.isActive(clusterState.getLiveNodes())) .filter( replica -> - !builder.onlyNrt - || (replica.getType() == Replica.Type.NRT - || (replica.getType() == Replica.Type.TLOG - && isShardLeader.test(replica)))) + !builder.onlyRtg + || (replica.getType() == Replica.Type.NRT || isShardLeader.test(replica))) .collect(Collectors.toList()); builder.replicaListTransformer.transform(list); List coreUrls = list.stream().map(Replica::getCoreUrl).collect(Collectors.toList()); @@ -275,7 +273,7 @@ static class Builder { private String collection; private ZkStateReader zkStateReader; private SolrParams params; - private boolean onlyNrt; + private boolean onlyRtg; private ReplicaListTransformer replicaListTransformer; private AllowListUrlChecker urlChecker; @@ -294,8 +292,9 @@ public Builder params(SolrParams params) { return this; } - public Builder onlyNrt(boolean onlyNrt) { - this.onlyNrt = onlyNrt; + /** Replica can serve the most recent data (RealTimeGet capable). */ + public Builder onlyRtg(boolean onlyRtg) { + this.onlyRtg = onlyRtg; return this; } 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 da2847210809..39f47e3d1198 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 @@ -75,13 +75,10 @@ public class HttpShardHandler extends ShardHandler { /** * If the request context map has an entry with this key and Boolean.TRUE as value, {@link - * #prepDistributed(ResponseBuilder)} will only include {@link - * org.apache.solr.common.cloud.Replica.Type#NRT} replicas as possible destination of the - * distributed request (or a leader replica of type {@link - * org.apache.solr.common.cloud.Replica.Type#TLOG}). This is used by the RealtimeGet handler, - * since other types of replicas shouldn't respond to RTG requests + * #prepDistributed(ResponseBuilder)} will only include replicas that serve the latest data. This + * is used by the {@link org.apache.solr.handler.RealTimeGetHandler} (RTG). */ - public static final String ONLY_NRT_REPLICAS = "distribOnlyRealtime"; + public static final String ONLY_RTG_REPLICAS = "distribOnlyRealtime"; /** * This is a fake ShardResponse used internally to trigger the {@link #take(boolean)} method to @@ -496,7 +493,7 @@ public void prepDistributed(ResponseBuilder rb) { ReplicaSource replicaSource; if (zkController != null) { - boolean onlyNrt = Boolean.TRUE.equals(req.getContext().get(ONLY_NRT_REPLICAS)); + boolean onlyRtg = Boolean.TRUE.equals(req.getContext().get(ONLY_RTG_REPLICAS)); replicaSource = new CloudReplicaSource.Builder() @@ -505,11 +502,11 @@ public void prepDistributed(ResponseBuilder rb) { .allowListUrlChecker(urlChecker) .replicaListTransformer(replicaListTransformer) .collection(cloudDescriptor.getCollectionName()) - .onlyNrt(onlyNrt) + .onlyRtg(onlyRtg) .build(); rb.slices = replicaSource.getSliceNames().toArray(new String[replicaSource.getSliceCount()]); - if (!rb.isForcedDistrib() && canShortCircuit(rb.slices, onlyNrt, params, cloudDescriptor)) { + if (!rb.isForcedDistrib() && canShortCircuit(rb.slices, onlyRtg, params, cloudDescriptor)) { rb.isDistrib = false; rb.shortCircuitedURL = ZkCoreNodeProps.getCoreUrl(zkController.getBaseUrl(), coreDescriptor.getName()); @@ -528,7 +525,7 @@ public void prepDistributed(ResponseBuilder rb) { .allowListUrlChecker(AllowListUrlChecker.ALLOW_ALL) .replicaListTransformer(NoOpReplicaListTransformer.INSTANCE) .collection(cloudDescriptor.getCollectionName()) - .onlyNrt(false) + .onlyRtg(false) .build(); final String adjective = (allActiveReplicaSource.getReplicasBySlice(i).isEmpty() ? "active" : "eligible"); @@ -574,21 +571,24 @@ private static String createSliceShardsStr(final List shardUrls) { /** Can we avoid distributed search / coordinator? */ private boolean canShortCircuit( String[] slices, - boolean onlyNrtReplicas, + boolean onlyRtgReplicas, SolrParams params, CloudDescriptor cloudDescriptor) { // Are we hosting the shard that this request is for, and are we active? If so, then handle it // ourselves and make it a non-distributed request. String ourSlice = cloudDescriptor.getShardId(); String ourCollection = cloudDescriptor.getCollectionName(); - // Some requests may only be fulfilled by replicas of type Replica.Type.NRT + // Real-time requests may only be fulfilled by an NRT replica or the shard leader (e.g. a TLOG + // leader), matching the replica selection in CloudReplicaSource. if (slices.length == 1 && slices[0] != null && (slices[0].equals(ourSlice) || slices[0].equals( ourCollection + "_" + ourSlice)) // handle the _ format && cloudDescriptor.getLastPublished() == Replica.State.ACTIVE - && (!onlyNrtReplicas || cloudDescriptor.getReplicaType() == Replica.Type.NRT)) { + && (!onlyRtgReplicas + || cloudDescriptor.getReplicaType() == Replica.Type.NRT + || cloudDescriptor.isLeader())) { // currently just a debugging parameter to check distrib search on a single node boolean shortCircuit = params.getBool("shortCircuit", true); diff --git a/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java index 5af923867120..124df5eaf42b 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java @@ -48,7 +48,7 @@ public void testSimple_ShardsParam() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(false) + .onlyRtg(false) .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -79,7 +79,7 @@ public void testShardsParam_DeadNode() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(false) + .onlyRtg(false) .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -108,7 +108,7 @@ public void testShardsParam_DownReplica() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(false) + .onlyRtg(false) .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -139,7 +139,7 @@ public void testMultipleCollections() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(false) + .onlyRtg(false) .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -186,7 +186,7 @@ public void testSimple_UsingClusterState() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(false) + .onlyRtg(false) .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -228,7 +228,7 @@ public void testSimple_OnlyNrt() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(true) // enable only nrt mode + .onlyRtg(true) // enable only nrt mode .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) @@ -273,7 +273,7 @@ public void testMultipleCollections_OnlyNrt() { CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder() .collection("collection1") - .onlyNrt(true) // enable only nrt mode + .onlyRtg(true) // enable only nrt mode .zkStateReader(zkStateReader) .replicaListTransformer(replicaListTransformer) .allowListUrlChecker(checker) diff --git a/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testRealTimeGetTlogLeaderShortCircuit.json b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testRealTimeGetTlogLeaderShortCircuit.json new file mode 100644 index 000000000000..7b0f6d78db75 --- /dev/null +++ b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testRealTimeGetTlogLeaderShortCircuit.json @@ -0,0 +1,19 @@ +{ + "phases": [ + { + "description": "phase 0", + "spans": [ + { + "name": "get:/{collection}/get", + "kind": "SERVER", + "db.instance": "tlogCollection", + "db.type": "solr", + "http.request.method": "GET", + "http.response.status_code": 200, + "http.url": "http://NORMALIZED/solr/tlogCollection/get", + "http.params": "ids=1&wt=javabin" + } + ] + } + ] +} diff --git a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java index f73d47060032..69181cb99903 100644 --- a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java +++ b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java @@ -32,14 +32,17 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.MetricsRequest; +import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.request.SolrQuery; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.client.solrj.request.V2Request; import org.apache.solr.client.solrj.response.CollectionAdminResponse; import org.apache.solr.client.solrj.response.InputStreamResponseParser; +import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.V2Response; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.RetryUtil; import org.apache.solr.util.stats.MetricUtils; @@ -53,6 +56,7 @@ public class TestDistributedTracing extends SolrCloudTestCase { private static final String COLLECTION = "collection1"; + private static final String TLOG_COLLECTION = "tlogCollection"; @BeforeClass public static void setupCluster() throws Exception { @@ -97,6 +101,18 @@ public static void setupCluster() throws Exception { .setNode(node3) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION, 2, 4); + + // A single-shard, single-TLOG-replica collection for testing real-time GET routing. A lone + // replica is the leader, so there's no follower replication polling to add nondeterministic + // spans. Its leader lives on node0. + CollectionAdminRequest.createCollection(TLOG_COLLECTION, "config", 1, 0, 1, 0) + .setCreateNodeSet("EMPTY") + .process(cluster.getSolrClient()); + CollectionAdminRequest.addReplicaToShard(TLOG_COLLECTION, "shard1") + .setType(Replica.Type.TLOG) + .setNode(node0) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(TLOG_COLLECTION, 1, 1); } @AfterClass @@ -129,6 +145,30 @@ public void test() throws Exception { verifier.done(); } + /** + * A real-time GET against a TLOG collection must be served by the shard leader. When the request + * lands on the leader (as here), it is served locally with no self-hop to itself; the gold file + * therefore has a single span. Were the leader not short-circuited, a child {@code /{core}/get} + * span would appear. + */ + @Test + public void testRealTimeGetTlogLeaderShortCircuit() throws Exception { + var verifier = new GoldFileTraceVerifier(getClass(), "testRealTimeGetTlogLeaderShortCircuit"); + var leaderClient = cluster.getJettySolrRunner(0).getSolrClient(); + + new UpdateRequest().add(sdoc("id", "1")).commit(leaderClient, TLOG_COLLECTION); + getAndClearSpans(); // ignore indexing spans + + var req = new QueryRequest(params("ids", "1")); + req.setPath("/get"); + QueryResponse rsp = req.process(leaderClient, TLOG_COLLECTION); + assertEquals(1, rsp.getResults().size()); + assertEquals("1", rsp.getResults().get(0).getFieldValue("id")); + + verifier.verifyPhase(); + verifier.done(); + } + @Test public void testAdminApi() throws Exception { var verifier = new GoldFileTraceVerifier(getClass(), "testAdminApi");