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


##########
server/src/main/java/org/apache/druid/segment/realtime/appenderator/StreamAppenderatorDriver.java:
##########
@@ -359,6 +360,7 @@ public void onSuccess(Object result)
                       if (numRemainingHandoffSegments.decrementAndGet() == 0) {
                         List<DataSegment> segments = 
segmentsAndCommitMetadata.getSegments();
                         log.debug("Successfully handed off [%d] segments.", 
segments.size());
+                        
metrics.reportMaxSegmentHandoffTime(System.currentTimeMillis() - 
handoffStartTime);

Review Comment:
   That does make sense, it would be nice to have the logs to clearly point to 
the problematic segments.
   
   But I think the only option is logging this only when the 
`numRemainingHandoffSegments` is 0. We would also need to call out in the log 
that possibly only "some" of the segment ids (out of the ones in the commit 
metadata) are problematic ones. This should be okay as the commit metadata 
should not be likely to have too many segments in one batch.
   
   Tapping into the `Object result` passed to the `onSuccess` to decide exactly 
which segment comes after the threshold doesn't seem viable either as the 
Futures seem to return weird objects (I didn't dig very deep though).
   
   Another concern is the exact value of the threshold itself, which would have 
to be hard-coded.



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