ege-st commented on code in PR #14074:
URL: https://github.com/apache/pinot/pull/14074#discussion_r1775212737
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -150,6 +167,19 @@ public IngestionDelayTracker(ServerMetrics serverMetrics,
String tableNameWithTy
_realTimeTableDataManager = realtimeTableDataManager;
_clock = Clock.systemUTC();
_isServerReadyToServeQueries = isServerReadyToServeQueries;
+
+ if (realtimeTableDataManager.getInstanceDataManagerConfig() != null
+ && realtimeTableDataManager.getInstanceDataManagerConfig().getConfig()
!= null) {
+ PinotConfiguration pinotConfiguration =
realtimeTableDataManager.getInstanceDataManagerConfig().getConfig();
+ _enableOffsetLagMetric =
+ pinotConfiguration.getProperty("offset.lag.tracking.enable",
DEFAULT_ENABLE_OFFSET_LAG_METRIC);
+ _offsetLagUpdateIntervalMs =
+
pinotConfiguration.getProperty("offset.lag.tracking.update.interval",
DEFAULT_OFFSET_LAG_UPDATE_INTERVAL_MS);
Review Comment:
Shouldn't `"offset.lag.tracking.enable"` and
`"offset.lag.tracking.update.interval"` be named constants?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -150,6 +167,19 @@ public IngestionDelayTracker(ServerMetrics serverMetrics,
String tableNameWithTy
_realTimeTableDataManager = realtimeTableDataManager;
_clock = Clock.systemUTC();
_isServerReadyToServeQueries = isServerReadyToServeQueries;
+
+ if (realtimeTableDataManager.getInstanceDataManagerConfig() != null
+ && realtimeTableDataManager.getInstanceDataManagerConfig().getConfig()
!= null) {
+ PinotConfiguration pinotConfiguration =
realtimeTableDataManager.getInstanceDataManagerConfig().getConfig();
+ _enableOffsetLagMetric =
+ pinotConfiguration.getProperty("offset.lag.tracking.enable",
DEFAULT_ENABLE_OFFSET_LAG_METRIC);
+ _offsetLagUpdateIntervalMs =
Review Comment:
Should there be a minimum and maximum for this? For example: what happens
if someone puts 0ms or 1ms?
##########
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:
`volatile` is a bit of Java that I don't yet fully understand: would someone
explain what making these 4 fields `volatile` will do within this class?
--
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]
