KKcorps commented on code in PR #14074:
URL: https://github.com/apache/pinot/pull/14074#discussion_r1776905802


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -83,22 +82,35 @@
  *
  * TODO: handle bug situations like the one where a partition is not allocated 
to a given server due to a bug.
  */
-
 public class IngestionDelayTracker {
 
   private static class IngestionInfo {
-    final long _ingestionTimeMs;
-    final long _firstStreamIngestionTimeMs;
-    final StreamPartitionMsgOffset _currentOffset;
-    final StreamPartitionMsgOffset _latestOffset;
+    volatile long _ingestionTimeMs;
+    volatile long _firstStreamIngestionTimeMs;
+    volatile StreamPartitionMsgOffset _currentOffset;
+    volatile StreamPartitionMsgOffset _latestOffset;

Review Comment:
   Basically instead of creating new objects, we simply keep on updating the 
value of current offset and latest offset in this object
   
   We the prometheus actually calls from the metric, we simply pull the last 
updated values from this object and return it to prom
   
   tbh, this logic has not been changed, it's just that I have marked them as 
volatile so that the updates to these variables are reflected correctly. These 
fields should always have been marked as volatile but we don't do that in a lot 
in our code :P 



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -83,22 +82,35 @@
  *
  * TODO: handle bug situations like the one where a partition is not allocated 
to a given server due to a bug.
  */
-
 public class IngestionDelayTracker {
 
   private static class IngestionInfo {
-    final long _ingestionTimeMs;
-    final long _firstStreamIngestionTimeMs;
-    final StreamPartitionMsgOffset _currentOffset;
-    final StreamPartitionMsgOffset _latestOffset;
+    volatile long _ingestionTimeMs;
+    volatile long _firstStreamIngestionTimeMs;
+    volatile StreamPartitionMsgOffset _currentOffset;
+    volatile StreamPartitionMsgOffset _latestOffset;

Review Comment:
   Basically instead of creating new objects, we simply keep on updating the 
value of current offset and latest offset in this object
   
   We the prometheus actually calls from the metric, we simply pull the last 
updated values from this object and return it to prom
   
   tbh, this logic has not been changed, it's just that I have marked them as 
volatile so that the updates to these variables are reflected correctly. These 
fields should always have been marked as volatile but we don't do that in a lot 
in our code 



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