Jackie-Jiang commented on code in PR #8989:
URL: https://github.com/apache/pinot/pull/8989#discussion_r939461094
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1693,10 +1693,21 @@ private void assignInstances(TableConfig tableConfig,
boolean override) {
InstanceAssignmentDriver instanceAssignmentDriver = new
InstanceAssignmentDriver(tableConfig);
List<InstanceConfig> instanceConfigs = getAllHelixInstanceConfigs();
for (InstancePartitionsType instancePartitionsType :
instancePartitionsTypesToAssign) {
- InstancePartitions instancePartitions =
- instanceAssignmentDriver.assignInstances(instancePartitionsType,
instanceConfigs, null);
- LOGGER.info("Persisting instance partitions: {}", instancePartitions);
- InstancePartitionsUtils.persistInstancePartitions(_propertyStore,
instancePartitions);
+ boolean hasPreConfiguredInstancePartitions =
TableConfigUtils.hasPreConfiguredInstancePartitions(tableConfig,
+ instancePartitionsType);
+ InstancePartitions instancePartitions;
+ if (!hasPreConfiguredInstancePartitions) {
+ instancePartitions =
instanceAssignmentDriver.assignInstances(instancePartitionsType,
instanceConfigs, null);
+ LOGGER.info("Persisting instance partitions: {}",
instancePartitions);
+ InstancePartitionsUtils.persistInstancePartitions(_propertyStore,
instancePartitions);
+ } else {
+ instancePartitions =
InstancePartitionsUtils.fetchInstancePartitionsWithRename(_propertyStore,
+
tableConfig.getInstancePartitionsMap().get(instancePartitionsType),
+ instancePartitionsType.getInstancePartitionsName(rawTableName));
+ LOGGER.info("Persisting instance partitions: {} (referencing {})",
instancePartitions,
+
tableConfig.getInstancePartitionsMap().get(instancePartitionsType));
Review Comment:
(nit) Reuse
`tableConfig.getInstancePartitionsMap().get(instancePartitionsType)`
```suggestion
String referenceInstancePartitionsName =
tableConfig.getInstancePartitionsMap().get(instancePartitionsType);
instancePartitions =
InstancePartitionsUtils.fetchInstancePartitionsWithRename(_propertyStore,
referenceInstancePartitionsName,
instancePartitionsType.getInstancePartitionsName(rawTableName));
LOGGER.info("Persisting instance partitions: {} (referencing {})",
instancePartitions, referenceInstancePartitionsName);
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -374,6 +375,11 @@ public TableConfigBuilder
setTunerConfigList(List<TunerConfig> tunerConfigList)
return this;
}
+ public TableConfigBuilder
setInstancePartitionsMap(Map<InstancePartitionsType, String>
instancePartitionsMap) {
Review Comment:
(nit) Move this next to `setInstanceAssignmentConfigMap()` for readability
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -418,6 +420,21 @@ private InstancePartitions
getInstancePartitions(TableConfig tableConfig,
String tableNameWithType = tableConfig.getTableName();
if (InstanceAssignmentConfigUtils.allowInstanceAssignment(tableConfig,
instancePartitionsType)) {
if (reassignInstances) {
+ String rawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType);
+ boolean hasPreConfiguredInstancePartitions =
TableConfigUtils.hasPreConfiguredInstancePartitions(tableConfig,
+ instancePartitionsType);
+ if (hasPreConfiguredInstancePartitions) {
+ InstancePartitions instancePartitions =
InstancePartitionsUtils.fetchInstancePartitionsWithRename(
+ _helixManager.getHelixPropertyStore(),
tableConfig.getInstancePartitionsMap().get(instancePartitionsType),
Review Comment:
(nit) Same here, store
`tableConfig.getInstancePartitionsMap().get(instancePartitionsType)` in a local
var
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableConfig.java:
##########
@@ -101,6 +102,9 @@ public class TableConfig extends BaseJsonConfig {
@JsonPropertyDescription(value = "Configs for Table config tuner")
private List<TunerConfig> _tunerConfigList;
+ @JsonPropertyDescription(value = "Point to an existing instance partitions")
+ private Map<InstancePartitionsType, String> _instancePartitionsMap;
Review Comment:
(nit) Move this next to `_instanceAssignmentConfigMap`, same for other
changes in this file
--
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: commits-unsubscr...@pinot.apache.org
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
