zpinto commented on code in PR #2801:
URL: https://github.com/apache/helix/pull/2801#discussion_r1631789223
##########
helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java:
##########
@@ -328,75 +462,192 @@ public String getInstanceDisabledReason() {
*
* @return Return instance disabled type
(org.apache.helix.constants.InstanceConstants.InstanceDisabledType)
* Default is am empty string.
+ * @deprecated This method is deprecated. Please use
getInstanceOperation().getSource
+ *() instead.
*/
+ @Deprecated
public String getInstanceDisabledType() {
- if
(!getInstanceOperation().equals(InstanceConstants.InstanceOperation.DISABLE)) {
+ if (_record.getBooleanField(InstanceConfigProperty.HELIX_ENABLED.name(),
+ HELIX_ENABLED_DEFAULT_VALUE)) {
return InstanceConstants.INSTANCE_NOT_DISABLED;
}
return
_record.getStringField(InstanceConfigProperty.HELIX_DISABLED_TYPE.name(),
InstanceConstants.InstanceDisabledType.DEFAULT_INSTANCE_DISABLE_TYPE.name());
}
+ private List<InstanceOperation> getInstanceOperations() {
+ if (_deserializedInstanceOperations == null ||
_deserializedInstanceOperations.isEmpty()) {
+ // If the _deserializedInstanceOperationMap is not set, then we need to
build it from the real
+ // helix property HELIX_INSTANCE_OPERATION_MAP.
+ List<String> instanceOperations =
+
_record.getListField(InstanceConfigProperty.HELIX_INSTANCE_OPERATIONS.name());
+ List<InstanceOperation> newDeserializedInstanceOperations = new
ArrayList<>();
+ if (instanceOperations != null) {
+ for (String serializedInstanceOperation : instanceOperations) {
+ try {
+ Map<String, String> properties =
_objectMapper.readValue(serializedInstanceOperation,
+ new TypeReference<Map<String, String>>() {
+ });
+ newDeserializedInstanceOperations.add(new
InstanceOperation(properties));
+ } catch (JsonProcessingException e) {
+ _logger.error(
+ "Failed to deserialize instance operation for instance: " +
_record.getId(), e);
+ }
+ }
+ }
+ _deserializedInstanceOperations = newDeserializedInstanceOperations;
+ }
+
+ return _deserializedInstanceOperations;
+ }
+
/**
* Set the instance operation for this instance.
+ * This method also sets the HELIX_ENABLED, HELIX_DISABLED_REASON, and
HELIX_DISABLED_TYPE fields
+ * for backwards compatibility.
*
* @param operation the instance operation
*/
- 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) {
+ public void setInstanceOperation(InstanceOperation operation) {
+ List<InstanceOperation> deserializedInstanceOperations =
getInstanceOperations();
+
+ if (operation.getSource() ==
InstanceConstants.InstanceOperationSource.ADMIN) {
+ deserializedInstanceOperations.clear();
+ } else {
+ // Remove the instance operation with the same trigger if it exists.
+ deserializedInstanceOperations.removeIf(
+ instanceOperation -> instanceOperation.getSource() ==
operation.getSource());
+ }
+ if (operation.getOperation() !=
InstanceConstants.InstanceOperation.ENABLE) {
+ deserializedInstanceOperations.add(operation);
+ }
+ // Set the actual field in the ZnRecord
+
_record.setListField(InstanceConfigProperty.HELIX_INSTANCE_OPERATIONS.name(),
+ deserializedInstanceOperations.stream().map(instanceOperation -> {
+ try {
+ return
_objectMapper.writeValueAsString(instanceOperation.getProperties());
+ } catch (JsonProcessingException e) {
+ throw new HelixException(
+ "Failed to serialize instance operation for instance: " +
_record.getId()
+ + " Can't set the instance operation to: " +
operation.getOperation(), e);
+ }
+ }).collect(Collectors.toList()));
+
+ // TODO: Remove this when we are sure that all users are using the new
InstanceOperation only and HELIX_ENABLED is removed.
+ if (operation.getOperation() ==
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));
+
+ if (_record.getBooleanField(InstanceConfigProperty.HELIX_ENABLED.name(),
true)) {
+ // Check if it is already disabled, if yes, then we don't need to set
HELIX_ENABLED and HELIX_ENABLED_TIMESTAMP
+ setInstanceEnabledHelper(false, operation.getTimestamp());
+ }
+
+
_record.setSimpleField(InstanceConfigProperty.HELIX_DISABLED_REASON.name(),
+ operation.getReason());
+ _record.setSimpleField(InstanceConfigProperty.HELIX_DISABLED_TYPE.name(),
+
InstanceConstants.InstanceOperationSource.instanceOperationSourceToInstanceDisabledType(
+ operation.getSource()).name());
+ } else if (operation.getOperation() ==
InstanceConstants.InstanceOperation.ENABLE) {
Review Comment:
Should we write back to old fields when setting InstanceOperations with new
API? That is the only way that a user of an old version of HelixAdmin can now
if the instance is disabled from the new API.
--
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]
