jtuglu-netflix commented on code in PR #17847:
URL: https://github.com/apache/druid/pull/17847#discussion_r2094610674


##########
server/src/main/java/org/apache/druid/segment/realtime/SegmentGenerationMetrics.java:
##########
@@ -105,14 +164,23 @@ public void setSinkCount(long sinkCount)
     this.sinkCount.set(sinkCount);
   }
 
+  public void reportMessageGap(final long messageGap)
+  {
+    messageGapStats.add(messageGap);
+  }
+
   public void reportMessageMaxTimestamp(long messageMaxTimestamp)
   {
-    this.messageMaxTimestamp.set(Math.max(messageMaxTimestamp, 
this.messageMaxTimestamp.get()));
+    if (this.messageMaxTimestamp.get() < messageMaxTimestamp) {
+      this.messageMaxTimestamp.getAndAccumulate(messageMaxTimestamp, 
Math::max);
+    }
   }
 
   public void reportMaxSegmentHandoffTime(long maxSegmentHandoffTime)
   {
-    this.maxSegmentHandoffTime.set(Math.max(maxSegmentHandoffTime, 
this.maxSegmentHandoffTime.get()));
+    if (this.maxSegmentHandoffTime.get() < maxSegmentHandoffTime) {
+      this.maxSegmentHandoffTime.getAndAccumulate(maxSegmentHandoffTime, 
Math::max);

Review Comment:
   Not sure I understand. This codepath does already have multi-threaded write 
access (for `maxSegmentHandoffTime`). See `reset()`. 
   1. Not only is it making the code faster, it's also making removing an 
incorrect, racy pattern.
   2. See below benchmarks that it also provides a performance boost:
   
   5 warmup/5 measurement on 10k calls
   Old:
   ```
   Benchmark                                                                    
   Mode  Cnt      Score    Error  Units
   
SegmentGenerationMetricsBenchmark.benchmarkMultipleReportMaxSegmentHandoffTime  
avgt    5  22755.902 ± 80.865  ns/op
   SegmentGenerationMetricsBenchmark.benchmarkMultipleReportMessageMaxTimestamp 
   avgt    5  22768.625 ± 63.275  ns/op
   ```
   
   New:
   ```
   Benchmark                                                                    
   Mode  Cnt      Score     Error  Units
   
SegmentGenerationMetricsBenchmark.benchmarkMultipleReportMaxSegmentHandoffTime  
avgt    5   6254.388 ± 319.197  ns/op
   SegmentGenerationMetricsBenchmark.benchmarkMultipleReportMessageGap          
   avgt    5  32911.301 ±  21.004  ns/op
   SegmentGenerationMetricsBenchmark.benchmarkMultipleReportMessageMaxTimestamp 
   avgt    5   6252.912 ± 222.810  ns/op
   ```



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