somandal commented on code in PR #15233:
URL: https://github.com/apache/pinot/pull/15233#discussion_r1989950403
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -105,37 +106,100 @@ private Boolean checkReloadNeededOnServers(String
rebalanceJobId, String tableNa
LOGGER.warn("Caught exception while trying to fetch reload status from
servers", e);
}
- return needsReload;
+ needsReloadMessage = needsReload == null ? "Could not determine needReload
status, run manually"
+ : !needsReload ? "No need to reload" : "Reload needed prior to running
rebalance";
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus = needsReload ==
null
+ ? RebalancePreCheckerResult.PreCheckStatus.ERROR
+ : !needsReload
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ return new RebalancePreCheckerResult(preCheckStatus, needsReloadMessage);
}
/**
* Checks if minimize data movement is set for the given table in the
TableConfig
*/
- private boolean checkIsMinimizeDataMovement(String rebalanceJobId, String
tableNameWithType,
+ private RebalancePreCheckerResult checkIsMinimizeDataMovement(String
rebalanceJobId, String tableNameWithType,
TableConfig tableConfig) {
LOGGER.info("Checking whether minimizeDataMovement is set for table: {}
with rebalanceJobId: {}", tableNameWithType,
rebalanceJobId);
try {
if (tableConfig.getTableType() == TableType.OFFLINE) {
- InstanceAssignmentConfig instanceAssignmentConfig =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
- return instanceAssignmentConfig.isMinimizeDataMovement();
+ boolean isInstanceAssignmentAllowed =
InstanceAssignmentConfigUtils.allowInstanceAssignment(tableConfig,
+ InstancePartitionsType.OFFLINE);
+ InstanceAssignmentConfig instanceAssignmentConfig;
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (isInstanceAssignmentAllowed) {
+ instanceAssignmentConfig =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
+ instanceAssignmentStatusMessage =
instanceAssignmentConfig.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement is not enabled but instance assignment
is allowed";
+ preCheckStatus = instanceAssignmentConfig.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
Review Comment:
same as above
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -105,37 +106,100 @@ private Boolean checkReloadNeededOnServers(String
rebalanceJobId, String tableNa
LOGGER.warn("Caught exception while trying to fetch reload status from
servers", e);
}
- return needsReload;
+ needsReloadMessage = needsReload == null ? "Could not determine needReload
status, run manually"
+ : !needsReload ? "No need to reload" : "Reload needed prior to running
rebalance";
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus = needsReload ==
null
+ ? RebalancePreCheckerResult.PreCheckStatus.ERROR
+ : !needsReload
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ return new RebalancePreCheckerResult(preCheckStatus, needsReloadMessage);
}
/**
* Checks if minimize data movement is set for the given table in the
TableConfig
*/
- private boolean checkIsMinimizeDataMovement(String rebalanceJobId, String
tableNameWithType,
+ private RebalancePreCheckerResult checkIsMinimizeDataMovement(String
rebalanceJobId, String tableNameWithType,
TableConfig tableConfig) {
LOGGER.info("Checking whether minimizeDataMovement is set for table: {}
with rebalanceJobId: {}", tableNameWithType,
rebalanceJobId);
try {
if (tableConfig.getTableType() == TableType.OFFLINE) {
- InstanceAssignmentConfig instanceAssignmentConfig =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
- return instanceAssignmentConfig.isMinimizeDataMovement();
+ boolean isInstanceAssignmentAllowed =
InstanceAssignmentConfigUtils.allowInstanceAssignment(tableConfig,
+ InstancePartitionsType.OFFLINE);
+ InstanceAssignmentConfig instanceAssignmentConfig;
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (isInstanceAssignmentAllowed) {
+ instanceAssignmentConfig =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
+ instanceAssignmentStatusMessage =
instanceAssignmentConfig.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement is not enabled but instance assignment
is allowed";
+ preCheckStatus = instanceAssignmentConfig.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ } else {
+ instanceAssignmentStatusMessage = "Instance assignment not allowed,
no need for minimizeDataMovement";
+ preCheckStatus = RebalancePreCheckerResult.PreCheckStatus.PASS;
+ }
+ return new RebalancePreCheckerResult(preCheckStatus,
instanceAssignmentStatusMessage);
} else {
- InstanceAssignmentConfig instanceAssignmentConfigConsuming =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.CONSUMING);
+ boolean isInstanceAssignmentAllowedConsuming =
InstanceAssignmentConfigUtils.allowInstanceAssignment(
+ tableConfig, InstancePartitionsType.CONSUMING);
+ InstanceAssignmentConfig instanceAssignmentConfigConsuming = null;
+ if (isInstanceAssignmentAllowedConsuming) {
+ instanceAssignmentConfigConsuming =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.CONSUMING);
+ }
// For REALTIME tables need to check for both CONSUMING and COMPLETED
segments if relocation is enabled
if
(!InstanceAssignmentConfigUtils.shouldRelocateCompletedSegments(tableConfig)) {
- return instanceAssignmentConfigConsuming.isMinimizeDataMovement();
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (isInstanceAssignmentAllowedConsuming) {
+ instanceAssignmentStatusMessage =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement is not enabled but instance assignment
is allowed";
+ preCheckStatus =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
Review Comment:
same as above
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -105,37 +106,100 @@ private Boolean checkReloadNeededOnServers(String
rebalanceJobId, String tableNa
LOGGER.warn("Caught exception while trying to fetch reload status from
servers", e);
}
- return needsReload;
+ needsReloadMessage = needsReload == null ? "Could not determine needReload
status, run manually"
+ : !needsReload ? "No need to reload" : "Reload needed prior to running
rebalance";
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus = needsReload ==
null
+ ? RebalancePreCheckerResult.PreCheckStatus.ERROR
+ : !needsReload
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ return new RebalancePreCheckerResult(preCheckStatus, needsReloadMessage);
}
/**
* Checks if minimize data movement is set for the given table in the
TableConfig
*/
- private boolean checkIsMinimizeDataMovement(String rebalanceJobId, String
tableNameWithType,
+ private RebalancePreCheckerResult checkIsMinimizeDataMovement(String
rebalanceJobId, String tableNameWithType,
TableConfig tableConfig) {
LOGGER.info("Checking whether minimizeDataMovement is set for table: {}
with rebalanceJobId: {}", tableNameWithType,
rebalanceJobId);
try {
if (tableConfig.getTableType() == TableType.OFFLINE) {
- InstanceAssignmentConfig instanceAssignmentConfig =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
- return instanceAssignmentConfig.isMinimizeDataMovement();
+ boolean isInstanceAssignmentAllowed =
InstanceAssignmentConfigUtils.allowInstanceAssignment(tableConfig,
+ InstancePartitionsType.OFFLINE);
+ InstanceAssignmentConfig instanceAssignmentConfig;
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (isInstanceAssignmentAllowed) {
+ instanceAssignmentConfig =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.OFFLINE);
+ instanceAssignmentStatusMessage =
instanceAssignmentConfig.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement is not enabled but instance assignment
is allowed";
+ preCheckStatus = instanceAssignmentConfig.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ } else {
+ instanceAssignmentStatusMessage = "Instance assignment not allowed,
no need for minimizeDataMovement";
+ preCheckStatus = RebalancePreCheckerResult.PreCheckStatus.PASS;
+ }
+ return new RebalancePreCheckerResult(preCheckStatus,
instanceAssignmentStatusMessage);
} else {
- InstanceAssignmentConfig instanceAssignmentConfigConsuming =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.CONSUMING);
+ boolean isInstanceAssignmentAllowedConsuming =
InstanceAssignmentConfigUtils.allowInstanceAssignment(
+ tableConfig, InstancePartitionsType.CONSUMING);
+ InstanceAssignmentConfig instanceAssignmentConfigConsuming = null;
+ if (isInstanceAssignmentAllowedConsuming) {
+ instanceAssignmentConfigConsuming =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.CONSUMING);
+ }
// For REALTIME tables need to check for both CONSUMING and COMPLETED
segments if relocation is enabled
if
(!InstanceAssignmentConfigUtils.shouldRelocateCompletedSegments(tableConfig)) {
- return instanceAssignmentConfigConsuming.isMinimizeDataMovement();
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (isInstanceAssignmentAllowedConsuming) {
+ instanceAssignmentStatusMessage =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement is not enabled but instance assignment
is allowed";
+ preCheckStatus =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
+ } else {
+ instanceAssignmentStatusMessage = "Instance assignment not
allowed, no need for minimizeDataMovement";
+ preCheckStatus = RebalancePreCheckerResult.PreCheckStatus.PASS;
+ }
+ return new RebalancePreCheckerResult(preCheckStatus,
instanceAssignmentStatusMessage);
}
- InstanceAssignmentConfig instanceAssignmentConfigCompleted =
-
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.COMPLETED);
- return instanceAssignmentConfigConsuming.isMinimizeDataMovement()
- && instanceAssignmentConfigCompleted.isMinimizeDataMovement();
+ boolean isInstanceAssignmentAllowedCompleted =
InstanceAssignmentConfigUtils.allowInstanceAssignment(
+ tableConfig, InstancePartitionsType.COMPLETED);
+ InstanceAssignmentConfig instanceAssignmentConfigCompleted = null;
+ if (isInstanceAssignmentAllowedCompleted) {
+ instanceAssignmentConfigCompleted =
+
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(tableConfig,
InstancePartitionsType.COMPLETED);
+ }
+
+ String instanceAssignmentStatusMessage;
+ RebalancePreCheckerResult.PreCheckStatus preCheckStatus;
+ if (!isInstanceAssignmentAllowedConsuming &&
!isInstanceAssignmentAllowedCompleted) {
+ instanceAssignmentStatusMessage = "Instance assignment not allowed,
no need for minimizeDataMovement";
+ preCheckStatus = RebalancePreCheckerResult.PreCheckStatus.PASS;
+ } else if (instanceAssignmentConfigConsuming != null &&
instanceAssignmentConfigCompleted != null) {
+ instanceAssignmentStatusMessage =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ && instanceAssignmentConfigCompleted.isMinimizeDataMovement()
+ ? "minimizeDataMovement is enabled"
+ : "minimizeDataMovement may not enabled for consuming or
completed but instance assignment is allowed "
+ + "for both";
+ preCheckStatus =
instanceAssignmentConfigConsuming.isMinimizeDataMovement()
+ && instanceAssignmentConfigCompleted.isMinimizeDataMovement()
+ ? RebalancePreCheckerResult.PreCheckStatus.PASS :
RebalancePreCheckerResult.PreCheckStatus.WARNING;
Review Comment:
same as above
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalancePreCheckerResult.java:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.rebalance;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import javax.annotation.Nullable;
+
+
+/**
+ * Holds the pre-check result for each pre-check performed as part of
RebalancePreChecker
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class RebalancePreCheckerResult {
+
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ private final PreCheckStatus _preCheckStatus;
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ private final String _message;
+
+ /**
+ * Constructor for RebalancePreCheckerResult
+ * @param preCheckStatus server related summary information
+ * @param message segment related summary information
+ */
+ @JsonCreator
+ public RebalancePreCheckerResult(@JsonProperty("preCheckStatus")
PreCheckStatus preCheckStatus,
+ @JsonProperty("message") @Nullable String message) {
+ _preCheckStatus = preCheckStatus;
+ _message = message;
+ }
+
Review Comment:
done
--
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
