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


##########
server/src/main/java/org/apache/druid/segment/realtime/SegmentGenerationMetrics.java:
##########
@@ -297,7 +313,12 @@ public SegmentGenerationMetrics snapshot()
       messageGapSnapshot = System.currentTimeMillis() - maxTimestamp;
     }
     retVal.messageGap.set(messageGapSnapshot);
-    retVal.messageGapStats.set(messageGapStats.getAndSet(new 
MessageGapStats()));

Review Comment:
   Thanks for the clarification!
   
   I think a lot of problem is arising due to the design of the method 
`SegmentGenerationMetrics.snapshot`.
   Ideally, it should not return an instance of `SegmentGenerationMetrics`, 
rather an immutable snapshot.
   
   You could consider doing that in this PR, if you are interested.



##########
server/src/main/java/org/apache/druid/segment/realtime/SegmentGenerationMetrics.java:
##########
@@ -297,7 +313,12 @@ public SegmentGenerationMetrics snapshot()
       messageGapSnapshot = System.currentTimeMillis() - maxTimestamp;
     }
     retVal.messageGap.set(messageGapSnapshot);
-    retVal.messageGapStats.set(messageGapStats.getAndSet(new 
MessageGapStats()));

Review Comment:
   Thanks for the clarification!
   
   I think a lot of problem is arising due to the design of the method 
`SegmentGenerationMetrics.snapshot`.
   Ideally, it should not return an instance of `SegmentGenerationMetrics`, 
rather an immutable object.
   
   You could consider doing that in this PR, if you are interested.



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