HoustonPutman commented on code in PR #4697:
URL: https://github.com/apache/solr/pull/4697#discussion_r3707012433


##########
solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java:
##########
@@ -171,10 +171,8 @@ private List<String> findReplicas(
               .filter(replica -> replica.isActive(clusterState.getLiveNodes()))
               .filter(
                   replica ->
-                      !builder.onlyNrt
-                          || (replica.getType() == Replica.Type.NRT
-                              || (replica.getType() == Replica.Type.TLOG

Review Comment:
   This change looks good to me too. Would be cool to have a `isRealTime()` 
utility.



##########
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##########
@@ -574,21 +571,24 @@ private static String createSliceShardsStr(final 
List<String> 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 
<collection>_<slice> format
         && cloudDescriptor.getLastPublished() == Replica.State.ACTIVE
-        && (!onlyNrtReplicas || cloudDescriptor.getReplicaType() == 
Replica.Type.NRT)) {
+        && (!onlyRtgReplicas
+            || cloudDescriptor.getReplicaType() == Replica.Type.NRT
+            || cloudDescriptor.isLeader())) {

Review Comment:
   This change looks good to me. Though maybe we should have a utility method? 
Maybe if its gets used somewhere else. (Since this is a CloudDescriptor and the 
CloudReplicaSource change is using a Replica object, it can't be shared I guess)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to