kfaraz commented on code in PR #16020:
URL: https://github.com/apache/druid/pull/16020#discussion_r1512568367


##########
server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java:
##########
@@ -242,7 +242,7 @@ public Object2IntMap<String> 
getDatasourceToUnavailableSegmentCount()
     final Iterable<DataSegment> dataSegments = 
metadataManager.segments().iterateAllUsedSegments();
     for (DataSegment segment : dataSegments) {
       SegmentReplicaCount replicaCount = 
segmentReplicationStatus.getReplicaCountsInCluster(segment.getId());
-      if (replicaCount != null && replicaCount.totalLoaded() > 0) {
+      if (replicaCount != null && (replicaCount.totalLoaded() > 0 || 
replicaCount.required() == 0)) {

Review Comment:
   Yeah, I did have similar thoughts.
   
   Following the approach used in this PR, no segment would ever truly be 
considered unavailable since it can always be queried using 
MSQ-query-from-deep-storage.
   
   The source of the confusion is the definition of `unavailable/count` itself.
   Does `unavailable` denote a segment
   1. that should be loaded but is not?
   OR
   2. that should be queryable but is not?
   
   The metric `underReplicated/count` corresponds with option 1 and this PR 
makes `unavailable/count` use option 1 too.
   
   The docs currently include both the flavors in the definition of this metric 
(which is correct for the native SQL engine):
   > Number of unique segments __left to load__ until all used segments are 
__available for queries__.
   
   @georgew5656 , to eliminate the confusion, as @cryptoe suggests, I think we 
should:
   - keep this metric unchanged (in case there are downstream apps relying on 
its values being reported correctly for the native engine)
   - add a new metric for MSQ cases
   - update the new info in the docs



-- 
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