xiangfu0 commented on code in PR #18855:
URL: https://github.com/apache/pinot/pull/18855#discussion_r3481273050
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentmetadata/SegmentZkMetadataFetcher.java:
##########
@@ -82,7 +84,7 @@ public void init(IdealState idealState, ExternalView
externalView, Set<String> o
listener.init(idealState, externalView, segments, znRecords);
}
for (int i = 0; i < numSegments; i++) {
- if (znRecords.get(i) != null) {
+ if (!isConsumingInExternalView(externalView, segments.get(i))) {
Review Comment:
This init-time cache update no longer checks whether the ZK fetch actually
returned metadata. If an ONLINE segment comes back with a null ZNRecord here,
`TimeSegmentPruner.init()` records `DEFAULT_INTERVAL` and this branch still
marks the segment as cached, so later assignment changes will never retry the
fetch. That regresses the old behavior and can leave the broker permanently
unable to time-prune that segment until restart/refresh; please keep the
original `znRecords.get(i) != null` guard in addition to the `CONSUMING` check
before caching.
--
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]
