Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -64,17 +65,22 @@ protected Map<FieldType, Set<String>> getNonTrimmableFields(InstanceConfig insta
}

/**
* We should trim HELIX_INSTANCE_OPERATIONS field, it is used to filter instances in the
* BaseControllerDataProvider. That filtering will be used to determine if ResourceChangeSnapshot
* has changed as opposed to checking the actual value of the field.
* Strip HELIX_INSTANCE_OPERATIONS from the change-detection snapshot entirely (key and
* value). The field is filtered upstream in BaseControllerDataProvider, so leaving it in
* the snapshot would create false positives.
*
* <p>NOTE: {@code super.getNonTrimmableKeys} returns a live {@code keySet()} view over the
* underlying ZNRecord listFields map. Removing from that view would mutate the caller's
* InstanceConfig. Copy into a fresh {@link HashSet} before mutating.
*
* @param property the instance config
* @return a map contains all non-trimmable field keys that need to be kept.
* @return a map containing all non-trimmable field keys that need to be kept.
*/
protected Map<FieldType, Set<String>> getNonTrimmableKeys(InstanceConfig property) {
Map<FieldType, Set<String>> nonTrimmableKeys = super.getNonTrimmableKeys(property);
nonTrimmableKeys.get(FieldType.LIST_FIELD)
.remove(InstanceConfigProperty.HELIX_INSTANCE_OPERATIONS.name());
Set<String> listKeys = new HashSet<>(nonTrimmableKeys.get(FieldType.LIST_FIELD));
listKeys.remove(InstanceConfigProperty.HELIX_INSTANCE_OPERATIONS.name());
nonTrimmableKeys.put(FieldType.LIST_FIELD, listKeys);
return nonTrimmableKeys;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,28 @@ private boolean validateInstancesUnableToAcceptOnlineReplicasLimit(final Resourc
int maxInstancesUnableToAcceptOnlineReplicas =
cache.getClusterConfig().getMaxOfflineInstancesAllowed();
if (maxInstancesUnableToAcceptOnlineReplicas >= 0) {
// Instead of only checking the offline instances, we consider how many instances in the cluster
// are not assignable and live. This is because some instances may be online but have an unassignable
// InstanceOperation such as EVACUATE, and DISABLE. We will exclude SWAP_IN and UNKNOWN instances from
// they should not account against the capacity of the cluster.
int instancesUnableToAcceptOnlineReplicas = cache.getInstanceConfigMap().entrySet().stream()
// Build the set of instances that currently count toward the offline budget:
// routable InstanceConfig minus the enabled-live set. We exclude UNROUTABLE
// operations (e.g. SWAP_IN, UNKNOWN) up front because those should not consume
// cluster capacity. Then we drop instances carrying a valid instance-operation
// maintenance marker, since they're inside an operator-approved window.
//
// The same marker-based subtraction is applied at MM exit in MaintenanceRecoveryStage
// so a marker that lets an instance escape MM entry also lets the cluster recover
// when the marker expires.
Set<String> offlineBudgetInstances = cache.getInstanceConfigMap().entrySet().stream()
.filter(instanceEntry -> !InstanceConstants.UNROUTABLE_INSTANCE_OPERATIONS.contains(
instanceEntry.getValue().getInstanceOperation().getOperation()))
.collect(Collectors.toSet())
.size() - cache.getEnabledLiveInstances().size();
.map(Map.Entry::getKey)
.collect(Collectors.toCollection(HashSet::new));
offlineBudgetInstances.removeAll(cache.getEnabledLiveInstances());

long nowMs = System.currentTimeMillis();
offlineBudgetInstances.removeIf(instanceName -> {
InstanceConfig cfg = cache.getInstanceConfigMap().get(instanceName);
return cfg != null && cfg.isUnderInstanceOperationMaintenance(nowMs);
});
int instancesUnableToAcceptOnlineReplicas = offlineBudgetInstances.size();
if (instancesUnableToAcceptOnlineReplicas > maxInstancesUnableToAcceptOnlineReplicas) {
String errMsg = String.format(
"Instances unable to take ONLINE replicas count %d greater than allowed count %d. Put cluster %s into "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import org.apache.helix.HelixDefinedState;
import org.apache.helix.HelixManager;
Expand All @@ -31,6 +32,7 @@
import org.apache.helix.controller.pipeline.AsyncWorkerType;
import org.apache.helix.model.BuiltInStateModelDefinitions;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.InstanceConfig;
import org.apache.helix.model.MaintenanceSignal;
import org.apache.helix.model.Partition;
import org.slf4j.Logger;
Expand Down Expand Up @@ -89,9 +91,25 @@ public void execute(final ClusterEvent event) throws Exception {
if (numOfflineInstancesForAutoExit < 0) {
return; // Config is not set, no auto-exit
}
// Get the count of all instances that are either offline or disabled
// Count offline-or-disabled assignable instances, then subtract those carrying a
// valid instance-operation maintenance marker so a deploy window can't trap the
// cluster in MM. The same marker-based subtraction runs at MM entry in
// BestPossibleStateCalcStage.
Set<String> assignable = cache.getAssignableInstances();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base is different from baseline, doesn't have evacuation here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre existing from long back, will revisit in a different PR

Set<String> enabledLive = cache.getEnabledLiveInstances();
long nowMs = System.currentTimeMillis();
int markedAndOfflineCount = 0;
for (String instanceName : assignable) {
if (enabledLive.contains(instanceName)) {
continue;
}
InstanceConfig cfg = cache.getInstanceConfigMap().get(instanceName);
if (cfg != null && cfg.isUnderInstanceOperationMaintenance(nowMs)) {
markedAndOfflineCount++;
}
}
int offlineDisabledCount =
cache.getAssignableInstances().size() - cache.getEnabledLiveInstances().size();
assignable.size() - enabledLive.size() - markedAndOfflineCount;
shouldExitMaintenance = offlineDisabledCount <= numOfflineInstancesForAutoExit;
reason = String.format(
"Auto-exiting maintenance mode for cluster %s; Num. of offline/disabled instances is %d, less than or equal to the exit threshold %d",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ public enum ClusterConfigProperty {
MAX_OFFLINE_INSTANCES_ALLOWED,
NUM_OFFLINE_INSTANCES_FOR_AUTO_EXIT, // For auto-exiting maintenance mode

// Instance-operation maintenance budget. Instances carrying a valid
// INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS marker on their InstanceConfig are excluded
// from the MAX_OFFLINE_INSTANCES_ALLOWED count while the marker has not expired.
//
// The two fields below define the cap on simultaneous markers. They are mutually
// exclusive: setters reject writing one while the other is already set. -1 means the
// cap is not configured.
INSTANCE_OPERATION_MAINTENANCE_BUDGET,
INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE,
// Fallback TTL (in millis) applied by the instance-operation maintenance write path
// when the caller omits expiresAtMillis. When unset (-1), callers must always supply an
// explicit expiresAtMillis; otherwise the write is rejected.
DEFAULT_INSTANCE_OPERATION_MAINTENANCE_DURATION_MS,

TARGET_EXTERNALVIEW_ENABLED,
@Deprecated // ERROR_OR_RECOVERY_PARTITION_THRESHOLD_FOR_LOAD_BALANCE will take
// precedence if it is set
Expand Down Expand Up @@ -579,6 +593,88 @@ public int getMaxOfflineInstancesAllowed() {
return _record.getIntField(ClusterConfigProperty.MAX_OFFLINE_INSTANCES_ALLOWED.name(), -1);
}

/**
* Set the absolute cap on the number of instances that may simultaneously carry an
* INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS marker. Pass {@code -1} to clear; the field is
* mutually exclusive with {@link #setInstanceOperationMaintenanceBudgetPercentage(int)}
* and the setter throws when the other form is already set.
*/
public void setInstanceOperationMaintenanceBudget(int instanceOperationMaintenanceBudget)
throws HelixException {
if (instanceOperationMaintenanceBudget >= 0
&& getInstanceOperationMaintenanceBudgetPercentage() >= 0) {
throw new HelixException("INSTANCE_OPERATION_MAINTENANCE_BUDGET and "
+ "INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE are mutually exclusive; "
+ "clear the percentage form before setting the absolute form.");
}
_record.setIntField(ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET.name(),
instanceOperationMaintenanceBudget);
}

/**
* @return the configured absolute cap on instance-operation maintenance markers, or
* {@code -1} when not set.
*/
public int getInstanceOperationMaintenanceBudget() {
return _record.getIntField(
ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET.name(), -1);
}

/**
* Set the percentage of cluster instances that may simultaneously carry an
* INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS marker. Valid range is {@code [0, 100]}; pass
* {@code -1} to clear. The field is mutually exclusive with
* {@link #setInstanceOperationMaintenanceBudget(int)} and the setter throws when the
* other form is already set or the value is outside the valid range.
*/
public void setInstanceOperationMaintenanceBudgetPercentage(
int instanceOperationMaintenanceBudgetPercentage) throws HelixException {
if (instanceOperationMaintenanceBudgetPercentage < -1
|| instanceOperationMaintenanceBudgetPercentage > 100) {
throw new HelixException(
"INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE must be in the range [0, 100] "
+ "or -1 to clear, got " + instanceOperationMaintenanceBudgetPercentage);
}
if (instanceOperationMaintenanceBudgetPercentage >= 0
&& getInstanceOperationMaintenanceBudget() >= 0) {
throw new HelixException("INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE and "
+ "INSTANCE_OPERATION_MAINTENANCE_BUDGET are mutually exclusive; clear the "
+ "absolute form before setting the percentage form.");
}
_record.setIntField(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add validation to ensure % is between 0 and 100 here before writing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE.name(),
instanceOperationMaintenanceBudgetPercentage);
}

/**
* @return the configured percentage cap on instance-operation maintenance markers, or
* {@code -1} when not set.
*/
public int getInstanceOperationMaintenanceBudgetPercentage() {
return _record.getIntField(
ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE.name(), -1);
}

/**
* Set the fallback duration (millis) applied by the instance-operation maintenance write
* path when the caller omits expiresAtMillis. Pass {@code -1L} to clear; when cleared,
* writes that omit expiresAtMillis are rejected.
*/
public void setDefaultInstanceOperationMaintenanceDurationMs(
long defaultInstanceOperationMaintenanceDurationMs) {
_record.setLongField(
ClusterConfigProperty.DEFAULT_INSTANCE_OPERATION_MAINTENANCE_DURATION_MS.name(),
defaultInstanceOperationMaintenanceDurationMs);
}

/**
* @return the configured fallback duration in millis, or {@code -1L} when not set.
*/
public long getDefaultInstanceOperationMaintenanceDurationMs() {
return _record.getLongField(
ClusterConfigProperty.DEFAULT_INSTANCE_OPERATION_MAINTENANCE_DURATION_MS.name(), -1L);
}

/**
* Sets the number of offline instances for auto-exit threshold so that MaintenanceRecoveryStage
* could use this number to determine whether the cluster could auto-exit maintenance mode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,17 @@ public enum InstanceConfigProperty {
INSTANCE_CAPACITY_MAP,
TARGET_TASK_THREAD_POOL_SIZE,
HELIX_INSTANCE_OPERATIONS,
INSTANCE_OPERATION_STATE
INSTANCE_OPERATION_STATE,
INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS
}

/**
* Sentinel returned by {@link #getInstanceOperationMaintenanceUntilMs()} when no marker is
* set. Callers clear the marker by passing this value to
* {@link #setInstanceOperationMaintenanceUntilMs(long)}.
*/
public static final long INSTANCE_OPERATION_MAINTENANCE_NOT_SET = -1L;

public static class InstanceOperation {
private static final String DEFAULT_INSTANCE_OPERATION_SOURCE =
InstanceConstants.InstanceOperationSource.USER.name();
Expand Down Expand Up @@ -234,11 +242,14 @@ private Map<String, String> getProperties() {
private static final ObjectMapper _objectMapper = new ObjectMapper();

// These fields are not allowed to be overwritten by the merge method because
// they are unique properties of an instance.
// they are unique properties of an instance. INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS is
// included because the marker is tied to a specific operation window on the source
// instance and must not transfer onto a swap-in target via overwriteInstanceConfig.
private static final ImmutableSet<InstanceConfigProperty> NON_OVERWRITABLE_PROPERTIES =
ImmutableSet.of(InstanceConfigProperty.HELIX_HOST, InstanceConfigProperty.HELIX_PORT,
InstanceConfigProperty.HELIX_ZONE_ID, InstanceConfigProperty.DOMAIN,
InstanceConfigProperty.INSTANCE_INFO_MAP);
InstanceConfigProperty.INSTANCE_INFO_MAP,
InstanceConfigProperty.INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS);

private static final Logger _logger = LoggerFactory.getLogger(InstanceConfig.class.getName());

Expand Down Expand Up @@ -437,6 +448,52 @@ public long getInstanceEnabledTime() {
HELIX_ENABLED_TIMESTAMP_DEFAULT_VALUE);
}

/**
* Get the instance-operation maintenance expiry timestamp (Unix millis). While
* {@code now < returnedValue}, this instance is excluded from the cluster-wide offline
* budget check (`MAX_OFFLINE_INSTANCES_ALLOWED`) that drives auto Maintenance Mode entry.
* The marker is an attribute on the instance only; it does not change cluster-level state.
*
* @return the expiry timestamp, or {@link #INSTANCE_OPERATION_MAINTENANCE_NOT_SET} when no
* marker is set.
*/
public long getInstanceOperationMaintenanceUntilMs() {
return _record.getLongField(
InstanceConfigProperty.INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS.name(),
INSTANCE_OPERATION_MAINTENANCE_NOT_SET);
}

/**
* Set or clear the instance-operation maintenance expiry timestamp. Passing
* {@link #INSTANCE_OPERATION_MAINTENANCE_NOT_SET} (or any non-positive value) clears the
* marker. Callers writing a real expiry must supply a Unix-millis value strictly greater
* than zero.
*
* @param expiresAtMillis the new expiry timestamp, or a non-positive value to clear.
*/
public void setInstanceOperationMaintenanceUntilMs(long expiresAtMillis) {
if (expiresAtMillis <= 0L) {
_record.getSimpleFields()
.remove(InstanceConfigProperty.INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS.name());
return;
}
_record.setLongField(
InstanceConfigProperty.INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS.name(),
expiresAtMillis);
}

/**
* Returns true when this instance currently carries an unexpired instance-operation
* maintenance marker. The check is purely a wall-clock comparison against {@code nowMs};
* callers pass {@code System.currentTimeMillis()} (or a deterministic clock in tests).
*
* @param nowMs the current time in Unix millis.
*/
public boolean isUnderInstanceOperationMaintenance(long nowMs) {
long until = getInstanceOperationMaintenanceUntilMs();
return until > 0L && nowMs < until;
}

/**
* Set the enabled state of the instance If user enables the instance, HELIX_DISABLED_REASON filed
* will be removed.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package org.apache.helix.controller.changedetector.trimmer;

/*
* 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.
*/

import org.apache.helix.model.ClusterConfig;
import org.apache.helix.model.ClusterConfig.ClusterConfigProperty;
import org.apache.helix.model.InstanceConfig;
import org.apache.helix.model.InstanceConfig.InstanceConfigProperty;
import org.testng.Assert;
import org.testng.annotations.Test;

/**
* Locks in the contract that instance-operation maintenance fields are non-topology and
* therefore trimmed before change-detection compares old vs new InstanceConfig/ClusterConfig
* snapshots. Marker writes must not trigger spurious rebalance pipeline runs.
*/
public class TestInstanceOperationMaintenanceTrimming {

@Test
public void testInstanceConfigMaintenanceUntilMsIsTrimmed() {
InstanceConfig original = new InstanceConfig("h1");
original.setHostName("host");
original.setPort("1234");
original.setInstanceOperationMaintenanceUntilMs(System.currentTimeMillis() + 60_000L);

InstanceConfig trimmed = InstanceConfigTrimmer.getInstance().trimProperty(original);

Assert.assertFalse(trimmed.getRecord().getSimpleFields().containsKey(
InstanceConfigProperty.INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS.name()),
"INSTANCE_OPERATION_MAINTENANCE_UNTIL_MS must be trimmed from change-detection "
+ "snapshots so marker writes do not trigger rebalance");
Assert.assertTrue(trimmed.getRecord().getSimpleFields()
.containsKey(InstanceConfigProperty.HELIX_HOST.name()),
"Sanity: topology-relevant fields are still preserved through the trimmer");
}

@Test
public void testClusterConfigInstanceOperationMaintenanceFieldsAreTrimmed() throws Exception {
ClusterConfig original = new ClusterConfig("c");
original.setInstanceOperationMaintenanceBudget(20);
original.setDefaultInstanceOperationMaintenanceDurationMs(3_600_000L);

ClusterConfig trimmed = ClusterConfigTrimmer.getInstance().trimProperty(original);

Assert.assertFalse(trimmed.getRecord().getSimpleFields().containsKey(
ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET.name()));
Assert.assertFalse(trimmed.getRecord().getSimpleFields().containsKey(
ClusterConfigProperty.INSTANCE_OPERATION_MAINTENANCE_BUDGET_PERCENTAGE.name()));
Assert.assertFalse(trimmed.getRecord().getSimpleFields().containsKey(
ClusterConfigProperty.DEFAULT_INSTANCE_OPERATION_MAINTENANCE_DURATION_MS.name()));
}
}
Loading
Loading