Short-circuit rebalancing when disabled by DaveCTurner · Pull Request #40942 · elastic/elasticsearch

DaveCTurner · 2019-04-08T07:45:45Z

Today if cluster.routing.rebalance.enable: none then rebalancing is disabled,
but we still execute balanceByWeights() and perform some rather expensive
calculations before discovering that we cannot rebalance any shards. In a large
cluster this can make cluster state updates occur rather slowly. With this
change we check earlier whether rebalancing is globally disabled and, if so,
avoid the rebalancing process entirely.

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely.

elasticmachine · 2019-04-08T07:45:47Z

Pinging @elastic/es-distributed

original-brownbear

LGTM :)

.../org/elasticsearch/cluster/routing/allocation/decider/EnableAllocationShortCircuitTests.java

henningandersen

Looking good, I added a few smaller things to address.

henningandersen · 2019-04-08T08:47:36Z

.../main/java/org/elasticsearch/cluster/routing/allocation/decider/EnableAllocationDecider.java

+        }
+
+        for (IndexMetaData indexMetaData : allocation.metaData()) {
+            if (INDEX_ROUTING_REBALANCE_ENABLE_SETTING.exists(indexMetaData.getSettings())) {


Would it make sense to also be able to short circuit if that setting is set to Rebalance.NONE?

Ok, I added this in 0c5c04b. I don't think this is a common configuration, so it'd be ok either way.

henningandersen · 2019-04-08T09:03:37Z

.../org/elasticsearch/cluster/routing/allocation/decider/EnableAllocationShortCircuitTests.java

+
+        ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(Settings.EMPTY))
+            .nodes(DiscoveryNodes.builder().add(newNode("node1"))).build();
+


I think it makes sense to also add some indices in this test case?

Sort of, although then we have to allocate their shards or else the ClusterRebalanceAllocationDecider steps in and prevents rebalancing. I expanded the tests to do this.

…isabled

DaveCTurner · 2019-04-08T10:32:08Z

@elasticmachine run elasticsearch-ci/2

henningandersen

LGTM.

Thanks @DaveCTurner

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely.

This reverts commit b53488e.

This reverts commit 4544b29.

This reverts commit f78e6ef.

This reverts commit 2ae9e27. This reverts commit 314890b.

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely. Relates elastic#40942 which was reverted because of egregiously faulty tests.

…e-unsafe-publication * elastic/master: Update contributing docs to reflect JDK 11 (elastic#40955) Docs: Simplifying setup by using module configuration variant syntax (elastic#40879) Unmute CreateIndexIT testCreateAndDeleteIndexConcurrently CI failures (elastic#40960) Revert "Short-circuit rebalancing when disabled (elastic#40942)" Mute EnableAllocationShortCircuitTests SQL: Refactor args verification of In & conditionals (elastic#40916) Avoid sharing source directories as it breaks intellij (elastic#40877) Short-circuit rebalancing when disabled (elastic#40942) SQL: Prefer resultSets over exceptions in metadata (elastic#40641) Mute ClusterPrivilegeTests.testThatSnapshotAndRestore Fix Race in AsyncTwoPhaseIndexerTests.testStateMachine (elastic#40947) Relax Overly Strict Assertion in TransportShardBulkAction (elastic#40940)

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely. Relates #40942 which was reverted because of egregiously faulty tests.

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely.

This reverts commit 2ae9e27. This reverts commit 314890b.

Today if `cluster.routing.rebalance.enable: none` then rebalancing is disabled, but we still execute `balanceByWeights()` and perform some rather expensive calculations before discovering that we cannot rebalance any shards. In a large cluster this can make cluster state updates occur rather slowly. With this change we check earlier whether rebalancing is globally disabled and, if so, avoid the rebalancing process entirely. Relates elastic#40942 which was reverted because of egregiously faulty tests.

DaveCTurner added >bug :Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) v8.0.0 v7.2.0 v6.7.2 v7.0.1 labels Apr 8, 2019

DaveCTurner requested review from henningandersen and original-brownbear April 8, 2019 07:46

original-brownbear approved these changes Apr 8, 2019

View reviewed changes

DaveCTurner added 4 commits April 8, 2019 09:32

Moar static

149ff95

EVEN MOAR STATIC

7eb4190

No need to check each index if we're going to answer YES anyway

84e14c3

Unnecessary 'this.'

003faab

henningandersen requested changes Apr 8, 2019

View reviewed changes

DaveCTurner added 2 commits April 8, 2019 10:23

Merge branch 'master' into 2019-04-08-short-circuit-allocation-when-d…

7491b9b

…isabled

Skip rebalancing if disabled and explicitly disabled on some indices too

0c5c04b

DaveCTurner requested a review from henningandersen April 8, 2019 10:17

henningandersen approved these changes Apr 8, 2019

View reviewed changes

DaveCTurner merged commit 314890b into elastic:master Apr 8, 2019

DaveCTurner deleted the 2019-04-08-short-circuit-allocation-when-disabled branch April 8, 2019 14:36

DaveCTurner added a commit that referenced this pull request Apr 8, 2019

Revert "Short-circuit rebalancing when disabled (#40942)"

cfbc525

This reverts commit b53488e.

DaveCTurner added a commit that referenced this pull request Apr 8, 2019

Revert "Short-circuit rebalancing when disabled (#40942)"

9aefa28

This reverts commit 4544b29.

DaveCTurner added a commit that referenced this pull request Apr 8, 2019

Revert "Short-circuit rebalancing when disabled (#40942)"

8eef92f

This reverts commit f78e6ef.

DaveCTurner added a commit that referenced this pull request Apr 8, 2019

Revert "Short-circuit rebalancing when disabled (#40942)"

f1302a5

This reverts commit 2ae9e27. This reverts commit 314890b.

DaveCTurner mentioned this pull request Apr 8, 2019

EnableAllocationShortCircuitTests CI Failure #40964

Closed

DaveCTurner mentioned this pull request Apr 8, 2019

Short-circuit rebalancing when disabled #40966

Merged

DaveCTurner removed v6.7.2 v7.0.1 v7.2.0 v8.0.0 labels Apr 8, 2019

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019

Revert "Short-circuit rebalancing when disabled (elastic#40942)"

bda012f

This reverts commit 2ae9e27. This reverts commit 314890b.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Short-circuit rebalancing when disabled#40942

Short-circuit rebalancing when disabled#40942
DaveCTurner merged 7 commits intoelastic:masterfrom
DaveCTurner:2019-04-08-short-circuit-allocation-when-disabled

DaveCTurner commented Apr 8, 2019

Uh oh!

elasticmachine commented Apr 8, 2019

Uh oh!

original-brownbear left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henningandersen left a comment

Uh oh!

henningandersen Apr 8, 2019

Uh oh!

DaveCTurner Apr 8, 2019

Uh oh!

henningandersen Apr 8, 2019

Uh oh!

DaveCTurner Apr 8, 2019

Uh oh!

DaveCTurner commented Apr 8, 2019

Uh oh!

henningandersen left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(Settings.EMPTY))
		.nodes(DiscoveryNodes.builder().add(newNode("node1"))).build();

Conversation

DaveCTurner commented Apr 8, 2019

Uh oh!

elasticmachine commented Apr 8, 2019

Uh oh!

original-brownbear left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

henningandersen Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

DaveCTurner commented Apr 8, 2019

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants