zpinto commented on code in PR #2772:
URL: https://github.com/apache/helix/pull/2772#discussion_r1546796720
##########
helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java:
##########
@@ -321,29 +330,93 @@ public String getInstanceDisabledReason() {
* Default is am empty string.
*/
public String getInstanceDisabledType() {
- if (getInstanceEnabled()) {
+ if
(!getInstanceOperation().equals(InstanceConstants.InstanceOperation.DISABLE)) {
return InstanceConstants.INSTANCE_NOT_DISABLED;
}
return
_record.getStringField(InstanceConfigProperty.HELIX_DISABLED_TYPE.name(),
InstanceConstants.InstanceDisabledType.DEFAULT_INSTANCE_DISABLE_TYPE.name());
}
/**
- * Get the timestamp (milliseconds from epoch) when this instance was
enabled/disabled last time.
+ * Set the instance operation for this instance.
*
- * @return
+ * @param operation the instance operation
*/
- public long getInstanceEnabledTime() {
- return
_record.getLongField(InstanceConfigProperty.HELIX_ENABLED_TIMESTAMP.name(), -1);
- }
-
public void setInstanceOperation(InstanceConstants.InstanceOperation
operation) {
_record.setSimpleField(InstanceConfigProperty.INSTANCE_OPERATION.name(),
operation == null ? "" : operation.name());
+ if (operation == null || operation ==
InstanceConstants.InstanceOperation.ENABLE
+ || operation == InstanceConstants.InstanceOperation.DISABLE) {
+ // We are still setting the HELIX_ENABLED field for backwards
compatibility.
+ // It is possible that users will be using earlier version of HelixAdmin
or helix-rest
+ // is on older version.
+ // TODO: Remove this when we are sure that all users are using the new
field INSTANCE_OPERATION.
+ setInstanceEnabledHelper(!(operation ==
InstanceConstants.InstanceOperation.DISABLE));
+ }
+ }
+
+ private void setInstanceOperationInit(InstanceConstants.InstanceOperation
operation) {
+ if (operation == null) {
+ return;
+ }
+ _record.setSimpleField(InstanceConfigProperty.INSTANCE_OPERATION.name(),
operation.name());
+ }
+
+ /**
+ * Get the InstanceOperation of this instance, default is ENABLE if nothing
is set. If
+ * HELIX_ENABLED is set to false, then the instance operation is DISABLE for
backwards
+ * compatibility.
+ *
+ * @return the instance operation
+ */
+ public InstanceConstants.InstanceOperation getInstanceOperation() {
+ String instanceOperationString =
+
_record.getSimpleField(InstanceConfigProperty.INSTANCE_OPERATION.name());
+
+ InstanceConstants.InstanceOperation instanceOperation;
+ try {
+ // If INSTANCE_OPERATION is not set, then the instance is enabled.
+ instanceOperation = instanceOperationString == null ||
instanceOperationString.isEmpty()
+ ? InstanceConstants.InstanceOperation.ENABLE
+ :
InstanceConstants.InstanceOperation.valueOf(instanceOperationString);
+ } catch (IllegalArgumentException e) {
+ _logger.error("Invalid instance operation: " + instanceOperationString +
" for instance: "
+ + _record.getId()
+ + ". You may need to update your version of Helix to get support for
this "
+ + "type of InstanceOperation. Defaulting to UNKNOWN.");
+ return InstanceConstants.InstanceOperation.UNKNOWN;
+ }
+
+ // Always respect the HELIX_ENABLED being set to false when instance
operation is unset
+ // for backwards compatibility.
+ if (!_record.getBooleanField(InstanceConfigProperty.HELIX_ENABLED.name(),
+ HELIX_ENABLED_DEFAULT_VALUE)
+ &&
(InstanceConstants.INSTANCE_DISABLED_OVERRIDABLE_OPERATIONS.contains(
+ instanceOperation))) {
+ return InstanceConstants.InstanceOperation.DISABLE;
+ }
+
+ return instanceOperation;
}
- public String getInstanceOperation() {
- return
_record.getStringField(InstanceConfigProperty.INSTANCE_OPERATION.name(), "");
+ /**
+ * Check if this instance is enabled. This is used to determine if the
instance can host online
+ * replicas and take new assignment.
+ *
+ * @return true if enabled, false otherwise
+ */
+ public boolean getInstanceEnabled() {
Review Comment:
This API is backwards compatible because getInstanceOperation will translate
the old HELIX_ENABLED field to the new InstanceOperation field.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
