shauryachats commented on code in PR #18855:
URL: https://github.com/apache/pinot/pull/18855#discussion_r3771232309
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/TimeSegmentPruner.java:
##########
@@ -107,12 +107,16 @@ private Interval
extractIntervalFromSegmentZKMetaZNRecord(String segment, @Nulla
@Override
public synchronized void onAssignmentChange(IdealState idealState,
ExternalView externalView,
Set<String> onlineSegments, List<String> pulledSegments, List<ZNRecord>
znRecords) {
- // NOTE: We don't update all the segment ZK metadata for every external
view change, but only the new added/removed
- // ones. The refreshed segment ZK metadata change won't be picked up.
for (int idx = 0; idx < pulledSegments.size(); idx++) {
String segment = pulledSegments.get(idx);
ZNRecord zNrecord = znRecords.get(idx);
- _intervalMap.computeIfAbsent(segment, k ->
extractIntervalFromSegmentZKMetaZNRecord(k, zNrecord));
+ // Always update segments that have DEFAULT_INTERVAL, which covers two
cases:
+ // 1. New segments not yet in the map
+ // 2. Segments that transitioned from CONSUMING (DEFAULT_INTERVAL) to
COMMITTED (valid time range)
+ Interval existing = _intervalMap.get(segment);
Review Comment:
Switched to Map.compute in TimeSegmentPruner.onAssignmentChange to fold the
get+put into a single map lookup.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/TimeSegmentPruner.java:
##########
@@ -107,12 +107,16 @@ private Interval
extractIntervalFromSegmentZKMetaZNRecord(String segment, @Nulla
@Override
public synchronized void onAssignmentChange(IdealState idealState,
ExternalView externalView,
Set<String> onlineSegments, List<String> pulledSegments, List<ZNRecord>
znRecords) {
- // NOTE: We don't update all the segment ZK metadata for every external
view change, but only the new added/removed
- // ones. The refreshed segment ZK metadata change won't be picked up.
for (int idx = 0; idx < pulledSegments.size(); idx++) {
String segment = pulledSegments.get(idx);
ZNRecord zNrecord = znRecords.get(idx);
- _intervalMap.computeIfAbsent(segment, k ->
extractIntervalFromSegmentZKMetaZNRecord(k, zNrecord));
+ // Always update segments that have DEFAULT_INTERVAL, which covers two
cases:
+ // 1. New segments not yet in the map
+ // 2. Segments that transitioned from CONSUMING (DEFAULT_INTERVAL) to
COMMITTED (valid time range)
+ Interval existing = _intervalMap.get(segment);
Review Comment:
Switched to Map.compute in `TimeSegmentPruner.onAssignmentChange` to fold
the get+put into a single map lookup.
--
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]
