Copilot commented on code in PR #15831:
URL: https://github.com/apache/pinot/pull/15831#discussion_r2094186198
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -87,18 +93,35 @@
public class IngestionDelayTracker {
private static class IngestionInfo {
- final long _ingestionTimeMs;
- final long _firstStreamIngestionTimeMs;
- final StreamPartitionMsgOffset _currentOffset;
- final StreamPartitionMsgOffset _latestOffset;
-
- IngestionInfo(long ingestionTimeMs, long firstStreamIngestionTimeMs,
- @Nullable StreamPartitionMsgOffset currentOffset, @Nullable
StreamPartitionMsgOffset latestOffset) {
+ final StreamMetadataProvider _streamMetadataProvider;
+ volatile long _ingestionTimeMs;
+ volatile long _firstStreamIngestionTimeMs;
+ volatile StreamPartitionMsgOffset _currentOffset;
+ volatile StreamPartitionMsgOffset _latestOffset;
+ volatile Long _offsetLag;
+
+ IngestionInfo(long ingestionTimeMs, long firstStreamIngestionTimeMs,
StreamPartitionMsgOffset currentOffset,
+ Long offsetLag, StreamMetadataProvider streamMetadataProvider) {
Review Comment:
[nitpick] Using the `Long` wrapper introduces unnecessary boxing and allows
null values; consider using the primitive `long` with a default of 0 for better
performance and null-safety.
```suggestion
volatile long _offsetLag;
IngestionInfo(long ingestionTimeMs, long firstStreamIngestionTimeMs,
StreamPartitionMsgOffset currentOffset,
long offsetLag, StreamMetadataProvider streamMetadataProvider) {
```
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -287,11 +356,22 @@ public void updateIngestionMetrics(String segmentName,
int partitionId, long ing
ServerGauge.END_TO_END_REALTIME_INGESTION_DELAY_MS,
() -> getPartitionEndToEndIngestionDelayMs(partitionId));
}
- if (currentOffset != null && latestOffset != null) {
+ if (currentOffset != null) {
Review Comment:
These gauge updates are only inside the branch that creates a new
IngestionInfo (v == null), so on subsequent updates metrics like offset lag,
consuming offset, and upstream offset won't be refreshed. Move the
setOrUpdatePartitionGauge calls outside of the v==null block so metrics are
updated on every invocation.
--
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]
