You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2019/08/14 23:55:17 UTC

[GitHub] [incubator-pinot] snleee commented on a change in pull request #4516: [Instance Assignment] Plug in instance assignment

snleee commented on a change in pull request #4516: [Instance Assignment] Plug in instance assignment
URL: https://github.com/apache/incubator-pinot/pull/4516#discussion_r314123808
 
 

 ##########
 File path: pinot-common/src/main/java/org/apache/pinot/common/config/instance/InstanceAssignmentConfigUtils.java
 ##########
 @@ -33,58 +33,74 @@
   private InstanceAssignmentConfigUtils() {
   }
 
+  /**
+   * Returns whether the instance assignment is allowed for the given table config.
+   */
+  public static boolean allowInstanceAssignment(TableConfig tableConfig,
+      InstancePartitionsType instancePartitionsType) {
+    TableType tableType = tableConfig.getTableType();
+    Map<InstancePartitionsType, InstanceAssignmentConfig> instanceAssignmentConfigMap =
+        tableConfig.getInstanceAssignmentConfigMap();
+    switch (instancePartitionsType) {
+      // Allow OFFLINE instance assignment if the OFFLINE table has it configured or (for backward-compatibility) is
+      // using replica-group segment assignment
+      case OFFLINE:
+        return tableType == TableType.OFFLINE && ((instanceAssignmentConfigMap != null && instanceAssignmentConfigMap
+            .containsKey(InstancePartitionsType.OFFLINE))
+            || AssignmentStrategy.REPLICA_GROUP_SEGMENT_ASSIGNMENT_STRATEGY
+            .equalsIgnoreCase(tableConfig.getValidationConfig().getSegmentAssignmentStrategy()));
+      // Allow CONSUMING/COMPLETED instance assignment if the REALTIME table has it configured
+      case CONSUMING:
+      case COMPLETED:
+        return tableType == TableType.REALTIME && (instanceAssignmentConfigMap != null && instanceAssignmentConfigMap
+            .containsKey(instancePartitionsType));
+      default:
+        throw new IllegalStateException();
+    }
+  }
+
+  /**
+   * Extracts or generates default instance assignment config from the given table config.
+   */
   public static InstanceAssignmentConfig getInstanceAssignmentConfig(TableConfig tableConfig,
       InstancePartitionsType instancePartitionsType) {
+    Preconditions.checkState(allowInstanceAssignment(tableConfig, instancePartitionsType),
+        "Instance assignment is not allowed for the given table config");
 
 Review comment:
   Can you add `tableName` and `instancePartitionType` to the log? 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org