mcvsubbu commented on a change in pull request #4553: Refactor ControllerLeaderLocator URL: https://github.com/apache/incubator-pinot/pull/4553#discussion_r324278317
########## File path: pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java ########## @@ -41,20 +43,22 @@ private static ControllerLeaderLocator _instance = null; public static final Logger LOGGER = LoggerFactory.getLogger(ControllerLeaderLocator.class); + // Minimum millis which must elapse between consecutive invalidation of cache + private static final long MILLIS_BETWEEN_INVALIDATE = 30_000; + private final HelixManager _helixManager; - // Co-ordinates of the last known controller leader. - private Pair<String, Integer> _controllerLeaderHostPort = null; + // Co-ordinates of the last known controller leader for every partitions, with partition name being the key and lead controller info being the value. + private final Map<Integer, Pair<String, Integer>> _cachedControllerLeaderMap; - // Indicates whether cached controller leader value is invalid + // Indicates whether cached controller leader value is invalid. If so, caches the host-port pair as the last known controller leader. private volatile boolean _cachedControllerLeaderInvalid = true; Review comment: While we are at it, can we change the name of this variable to be `_cachedControllerLeaderValid` and set it to false by default? A bit easier to read I think ---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
