cryptoe commented on code in PR #16020:
URL: https://github.com/apache/druid/pull/16020#discussion_r1512532737
##########
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:
Should we also document this change here :
`/docs/operations/metrics.md#L333`. Thinking more about it on how the doc
would look, should we add a new metric type if replicaCount.required==0 ?
The reason I am saying this is that users on the regular sql/native endpoint
will never see the data from used segment in deep storage ie`required==0` .
They might get thrown off because of this.
I feel the current doc change for the metric fails to capture this nuance.
If we try to explain this nuance, it becomes complicated.
A new metric `segment/deepStorage/count` would probably make more sense
no ?
Wdyt ?
--
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]
