mcvsubbu commented on a change in pull request #6165: URL: https://github.com/apache/incubator-pinot/pull/6165#discussion_r510496545
########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java ########## @@ -55,6 +55,7 @@ private static final String ENABLE_COMPRESSIONS_KEY = "enableCompression"; private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 2.0f); + private static final RetryPolicy DEFAULT_TABLE_IDEALSTATES_UPDATE_RETRY_POLICY = RetryPolicies.fixedDelayRetryPolicy(20, 100L); Review comment: I would use a random delay policy instead of fixed. Otherwise, you can have the same controllers coming back to update the IS at the same time. I don't think we have the policy, so you can create one. ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java ########## @@ -1672,6 +1679,10 @@ public void addNewSegment(String tableName, SegmentMetadata segmentMetadata, Str } } + private Object getTableUpdaterLock(String offlineTableName) { + return _tableUpdaterLocks[offlineTableName.hashCode() % _tableUpdaterLocks.length]; Review comment: hash code can be negative. use `& INTEGER_MAX` to get a positive. I fell into the same trap in the realtime part :-) ---------------------------------------------------------------- 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