xiangfu0 commented on code in PR #18137:
URL: https://github.com/apache/pinot/pull/18137#discussion_r3057458410
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -979,6 +985,7 @@ public void commitSegmentMetadataToDone(String
realtimeTableName,
FlushThresholdUpdater flushThresholdUpdater =
_flushThresholdUpdateManager.getFlushThresholdUpdater(streamConfig);
flushThresholdUpdater.onSegmentCommit(streamConfig,
committingSegmentDescriptor, committingSegmentZKMetadata);
+ updateCommittingSegmentSizeGauge(streamConfig,
committingSegmentDescriptor.getSegmentSizeBytes());
Review Comment:
I do not think this is fully orthogonal to this PR.
`createNewSegmentMetadata()` already resolves the correct `StreamConfig` from
the committing segment partition id
(`getStreamConfigFromPinotPartitionId(...)`), but this new metric path switches
back to `getFirstStreamConfig(tableConfig)`. On pauseless tables configured
with multiple streams, `COMMITTING_SEGMENT_SIZE_WITH_TOPIC` will now be emitted
under the first topic even when the committed segment belongs to a later
stream. Since the user-visible change here is to always emit these gauges, that
means the new per-topic metric is incorrect for multi-stream tables unless we
derive the stream config from `segmentName` here as well.
--
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]
