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

Reply via email to