zeronerdzerogeekzerocool commented on PR #14643: URL: https://github.com/apache/pinot/pull/14643#issuecomment-5897849486
@Jackie-Jiang good question — the ReentrantLock swap alone doesn’t actually fix that. It just changes which mutex blocks callers: a refreshControllerLeaderMap() stuck on a slow/unresponsive ZK still serializes every getControllerLeader()/invalidateCachedControllerLeader() call behind it, same as the original synchronized methods did. The latest commits change the approach. Rather than trying to make refreshControllerLeaderMap() itself faster (it still calls out to ZK/Helix and can still be slow), they stop that latency from blocking anyone except the one thread already doing the refresh: getControllerLeader()’s cache-hit path never takes a lock at all — _cachedControllerLeaderMap is a ConcurrentHashMap. On a cache miss, refreshControllerLeaderMap() is guarded by ReentrantLock#tryLock() instead of lock(). If a refresh is already in flight (e.g. stuck on ZK), every other caller falls through immediately instead of queueing, and just returns the current (possibly stale/null) cached value — consistent with this method’s existing “retry on next request” contract. invalidateCachedControllerLeader() never talks to ZK, so it no longer shares any lock with the refresh path at all; its rate-limited check-and-set is now lock-free via AtomicLong#getAndUpdate(). So the fix isn’t “make ZK faster” — it’s “don’t let other threads pay for ZK being slow.” A stuck ZK call now only ever stalls the single thread performing that refresh. -- 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]
