Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -1134,22 +1134,22 @@ private boolean tryRelocateShard(ModelNode minNode, ModelNode maxNode, ProjectIn
continue;
}

final Decision decision = new Decision.Multi().add(allocationDecision).add(rebalanceDecision);
final Decision.Type canAllocateOrRebalance = Decision.Type.min(allocationDecision.type(), rebalanceDecision.type());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this place could create nested multi decision


maxNode.removeShard(projectIndex(shard), shard);
long shardSize = allocation.clusterInfo().getShardSize(shard, ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE);

assert decision.type() == Type.YES || decision.type() == Type.THROTTLE : decision.type();
assert canAllocateOrRebalance == Type.YES || canAllocateOrRebalance == Type.THROTTLE : canAllocateOrRebalance;
logger.debug(
"decision [{}]: relocate [{}] from [{}] to [{}]",
decision.type(),
canAllocateOrRebalance,
shard,
maxNode.getNodeId(),
minNode.getNodeId()
);
minNode.addShard(
projectIndex(shard),
decision.type() == Type.YES
canAllocateOrRebalance == Type.YES
/* only allocate on the cluster if we are not throttled */
? routingNodes.relocateShard(shard, minNode.getNodeId(), shardSize, "rebalance", allocation.changes()).v1()
: shard.relocate(minNode.getNodeId(), shardSize)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ private Decision withDeciders(
BiFunction<String, Decision, String> logMessageCreator
) {
if (debugMode == RoutingAllocation.DebugMode.OFF) {
var result = Decision.YES;
Decision result = Decision.YES;
for (AllocationDecider decider : deciders) {
var decision = deciderAction.apply(decider);
if (decision.type() == Decision.Type.NO) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,19 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Objects;

/**
* This abstract class defining basic {@link Decision} used during shard
* allocation process.
*
* @see AllocationDecider
*/
public abstract class Decision implements ToXContent, Writeable {
public sealed interface Decision extends ToXContent, Writeable permits Decision.Single, Decision.Multi {

public static final Decision ALWAYS = new Single(Type.YES);
public static final Decision YES = new Single(Type.YES);
public static final Decision NO = new Single(Type.NO);
public static final Decision THROTTLE = new Single(Type.THROTTLE);
Single ALWAYS = new Single(Type.YES);
Single YES = new Single(Type.YES);
Single NO = new Single(Type.NO);
Single THROTTLE = new Single(Type.THROTTLE);

/**
* Creates a simple decision
Expand All @@ -46,40 +45,69 @@ public abstract class Decision implements ToXContent, Writeable {
* @param explanationParams additional parameters for the decision
* @return new {@link Decision} instance
*/
public static Decision single(Type type, @Nullable String label, @Nullable String explanation, @Nullable Object... explanationParams) {
static Decision single(Type type, @Nullable String label, @Nullable String explanation, @Nullable Object... explanationParams) {
return new Single(type, label, explanation, explanationParams);
}

public static Decision readFrom(StreamInput in) throws IOException {
static Decision readFrom(StreamInput in) throws IOException {
// Determine whether to read a Single or Multi Decision
if (in.readBoolean()) {
Multi result = new Multi();
int decisionCount = in.readVInt();
for (int i = 0; i < decisionCount; i++) {
Decision s = readFrom(in);
result.decisions.add(s);
var flag = in.readBoolean();
assert flag == false : "nested multi decision is not permitted";
var single = readSingleFrom(in);
result.decisions.add(single);
}
return result;
} else {
final Type type = Type.readFrom(in);
final String label = in.readOptionalString();
final String explanation = in.readOptionalString();
if (label == null && explanation == null) {
return switch (type) {
case YES -> YES;
case THROTTLE -> THROTTLE;
case NO -> NO;
};
}
return new Single(type, label, explanation);
return readSingleFrom(in);
}
}

private static Single readSingleFrom(StreamInput in) throws IOException {
final Type type = Type.readFrom(in);
final String label = in.readOptionalString();
final String explanation = in.readOptionalString();
if (label == null && explanation == null) {
return switch (type) {
case YES -> YES;
case THROTTLE -> THROTTLE;
case NO -> NO;
};
}
return new Single(type, label, explanation);
}

/**
* Get the {@link Type} of this decision
* @return {@link Type} of this decision
*/
Type type();

/**
* Get the description label for this decision.
*/
@Nullable
String label();

/**
* Get the explanation for this decision.
*/
@Nullable
String getExplanation();

/**
* Return the list of all decisions that make up this decision
*/
List<Decision> getDecisions();

/**
* This enumeration defines the
* possible types of decisions
*/
public enum Type implements Writeable {
enum Type implements Writeable {
YES(1),
THROTTLE(2),
NO(0);
Expand Down Expand Up @@ -110,45 +138,22 @@ public boolean higherThan(Type other) {
return false;
} else if (other == NO) {
return true;
} else if (other == THROTTLE && this == YES) {
return true;
}
return false;
} else return other == THROTTLE && this == YES;
Copy link
Contributor

Choose a reason for hiding this comment

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

slight nit that this isn't our usual style for if-else statements, skipping the {} brackets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm intellij auto-format did that and spotless didn't complain either.

}

}

/**
* Get the {@link Type} of this decision
* @return {@link Type} of this decision
*/
public abstract Type type();

/**
* Get the description label for this decision.
*/
@Nullable
public abstract String label();

/**
* Get the explanation for this decision.
*/
@Nullable
public abstract String getExplanation();
/**
* @return lowest decision by precedence NO->THROTTLE->YES
*/
public static Type min(Type a, Type b) {
return a.higherThan(b) ? b : a;
}

/**
* Return the list of all decisions that make up this decision
*/
public abstract List<Decision> getDecisions();
}

/**
* Simple class representing a single decision
*/
public static class Single extends Decision implements ToXContentObject {
private final Type type;
private final String label;
private final String explanationString;

record Single(Type type, String label, String explanationString) implements Decision, ToXContentObject {
/**
* Creates a new {@link Single} decision of a given type
* @param type {@link Type} of the decision
Expand All @@ -165,24 +170,13 @@ private Single(Type type) {
* @param explanationParams A set of additional parameters
*/
public Single(Type type, @Nullable String label, @Nullable String explanation, @Nullable Object... explanationParams) {
this.type = type;
this.label = label;
if (explanationParams != null && explanationParams.length > 0) {
this.explanationString = String.format(Locale.ROOT, explanation, explanationParams);
} else {
this.explanationString = explanation;
}
}

@Override
public Type type() {
return this.type;
}

@Override
@Nullable
public String label() {
return this.label;
this(
type,
label,
explanationParams != null && explanationParams.length > 0
? String.format(Locale.ROOT, explanation, explanationParams)
: explanation
);
}

@Override
Expand All @@ -199,29 +193,6 @@ public String getExplanation() {
return this.explanationString;
}

@Override
public boolean equals(Object object) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you remove this because equality is unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I keep toString for bwc. If exact format is not important can remove it too.

Copy link
Contributor

@DiannaHohensee DiannaHohensee Jul 11, 2025

Choose a reason for hiding this comment

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

I think a change might affect the allocation explain response? I'd leave it for bwc, as you say.

if (this == object) {
return true;
}

if (object == null || getClass() != object.getClass()) {
return false;
}

Decision.Single s = (Decision.Single) object;
return this.type == s.type && Objects.equals(label, s.label) && Objects.equals(explanationString, s.explanationString);
}

@Override
public int hashCode() {
int result = type.hashCode();
result = 31 * result + (label == null ? 0 : label.hashCode());
String explanationStr = explanationString;
result = 31 * result + (explanationStr == null ? 0 : explanationStr.hashCode());
return result;
}

@Override
public String toString() {
if (explanationString != null) {
Expand Down Expand Up @@ -254,17 +225,20 @@ public void writeTo(StreamOutput out) throws IOException {
/**
* Simple class representing a list of decisions
*/
public static class Multi extends Decision implements ToXContentFragment {
record Multi(List<Single> decisions) implements Decision, ToXContentFragment {

private final List<Decision> decisions = new ArrayList<>();
public Multi() {
this(new ArrayList<>());
}

/**
* Add a decision to this {@link Multi}decision instance
* @param decision {@link Decision} to add
* @return {@link Multi}decision instance with the given decision added
*/
public Multi add(Decision decision) {
decisions.add(decision);
assert decision instanceof Single;
decisions.add((Single) decision);
return this;
}

Expand Down Expand Up @@ -300,26 +274,6 @@ public List<Decision> getDecisions() {
return Collections.unmodifiableList(this.decisions);
}

@Override
public boolean equals(final Object object) {
if (this == object) {
return true;
}

if (object == null || getClass() != object.getClass()) {
return false;
}

final Decision.Multi m = (Decision.Multi) object;

return this.decisions.equals(m.decisions);
}

@Override
public int hashCode() {
return 31 * decisions.hashCode();
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
Expand Down