Jackie-Jiang commented on a change in pull request #6483:
URL: https://github.com/apache/incubator-pinot/pull/6483#discussion_r564128367
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -793,6 +793,27 @@ void
updateInstanceStatesForNewConsumingSegment(Map<String, Map<String, String>>
LOGGER.info("Updating segment: {} to ONLINE state",
committingSegmentName);
}
+ // There used to be a race condition in pinot (caused by heavy GC on the
controller during segment commit)
Review comment:
I think it can only happen in the following orders:
- The current leader controller start committing a segment
- The current leader controller runs into GC and not responding, leadership
changes to another controller
- The new leader controller committed the segment
- The previous leader controller wakes up and commit the segment again
If this is the order, the fix won't work because the `instanceStatesMap` is
not up-to-date if it is already calculated before the controller runs into GC.
The right fix might be checking the leadership again before actually committing
the segment.
@jackjlli What was the fix?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
