Skip to content

set default value of cluster.routing.allocation.cluster_concurrent_rebalance to -1 #109210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
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 @@ -34,9 +34,11 @@ public class ConcurrentRebalanceAllocationDecider extends AllocationDecider {

public static final String NAME = "concurrent_rebalance";

public static final int DEFAULT_CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE = -1;

public static final Setting<Integer> CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE_SETTING = Setting.intSetting(
"cluster.routing.allocation.cluster_concurrent_rebalance",
2,
DEFAULT_CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add two new separate settings controlling inbound and outbound rebalances - just like we have for recoveries.

We can assume that any existing relocation is a rebalance when figuring out whether to allow one more rebalance. However, that will only be able to determine based on source node, not target node AFAICS. I think we will need another method on AllocationDecider to help us decide whether a specific rebalance (to a specific node) is possible, otherwise you'll not be able to make the determination.

-1,
Property.Dynamic,
Property.NodeScope
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ public void testMinimalRelocationsNoLimit() {
ClusterRebalanceAllocationDecider.ClusterRebalanceType.ALWAYS.toString()
)
.put("cluster.routing.allocation.node_concurrent_recoveries", 100)
.put("cluster.routing.allocation.node_initial_primaries_recoveries", 100);
.put("cluster.routing.allocation.node_initial_primaries_recoveries", 100)
Copy link
Author

Choose a reason for hiding this comment

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

test case conditions assume previous default value of 2 for cluster.routing.allocation.cluster_concurrent_rebalance

planning to add new test for new behaviour as per new default .

.put("cluster.routing.allocation.cluster_concurrent_rebalance", 2);
AllocationService service = createAllocationService(settings.build());

ClusterState clusterState = initCluster(service, 1, 3, 3, 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ public void testRebalanceDoesNotCauseHotSpots() {
assertThat(
"Reconciling nodes should all have same amount (max 1 delta) of moves: " + totalOutgoingMoves,
summary.getMax() - summary.getMin(),
lessThanOrEqualTo(1)
lessThanOrEqualTo(numberOfNodes)
Copy link
Author

Choose a reason for hiding this comment

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

number of concurrent rebalance now is controlled by cluster.routing.allocation.node_concurrent_incoming_recoveries / cluster.routing.allocation.node_concurrent_outgoing_recoveries

which has default value = number of nodes.

);

totalOutgoingMoves.keySet().removeIf(nodeId -> isReconciled(allocation.routingNodes().node(nodeId), balance));
Expand Down