Skip to content

Enable write load constraint decider by default #135246

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Sep 30, 2025
Merged

Enable write load constraint decider by default #135246

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7365324
Enable write load constraint decider by default
nicktindall Sep 23, 2025
7a8536d
Fix disabled test
nicktindall Sep 23, 2025
2402099
Add label/description for YES decision
nicktindall Sep 24, 2025
14c936d
[CI] Update transport version definitions
Sep 24, 2025
b4f08dd
Remove assumptions around write load decider default enablement
nicktindall Sep 24, 2025
d33f49a
Merge remote-tracking branch 'origin/main' into enabled_write_load_de…
nicktindall Sep 24, 2025
5068f10
Only allow enablement of the write load decider when the feature flag…
nicktindall Sep 24, 2025
6a93c56
Fix feature flag, enable decider by default
nicktindall Sep 24, 2025
df43301
Move utility method to bottom of class
nicktindall Sep 24, 2025
c423850
Merge branch 'main' into enabled_write_load_decider
nicktindall Sep 29, 2025
149b2ea
Use feature flag to configure default
nicktindall Sep 29, 2025
9845c38
Tidy
nicktindall Sep 30, 2025
917c617
feature flag naming
nicktindall Sep 30, 2025
720ff18
Merge remote-tracking branch 'origin/main' into enabled_write_load_de…
nicktindall Sep 30, 2025
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
40 changes: 21 additions & 19 deletions server/src/internalClusterTest/java/org/elasticsearch/index/shard/IndexShardIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ public void testHeapUsageEstimateIsPresent() {
}

public void testNodeWriteLoadsArePresent() {
// Disable write load decider to begin with
setWriteLoadDeciderEnablement(WriteLoadConstraintSettings.WriteLoadDeciderStatus.DISABLED);

InternalClusterInfoService clusterInfoService = (InternalClusterInfoService) getInstanceFromNode(ClusterInfoService.class);
ClusterInfoServiceUtils.refresh(clusterInfoService);
Map<String, NodeUsageStatsForThreadPools> nodeThreadPoolStats = clusterInfoService.getClusterInfo()
Expand All @@ -315,15 +318,10 @@ public void testNodeWriteLoadsArePresent() {
assertTrue(nodeThreadPoolStats.isEmpty());

// Enable collection for node write loads.
updateClusterSettings(
Settings.builder()
.put(
WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey(),
randomBoolean()
? WriteLoadConstraintSettings.WriteLoadDeciderStatus.ENABLED
: WriteLoadConstraintSettings.WriteLoadDeciderStatus.LOW_THRESHOLD_ONLY
)
.build()
setWriteLoadDeciderEnablement(
randomBoolean()
? WriteLoadConstraintSettings.WriteLoadDeciderStatus.ENABLED
: WriteLoadConstraintSettings.WriteLoadDeciderStatus.LOW_THRESHOLD_ONLY
Copy link
Contributor

@DiannaHohensee DiannaHohensee Sep 29, 2025

Choose a reason for hiding this comment

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

Would your helper method be appropriate for an explicit DISABLED settings update in the finally block below, and in other tests?

IIUC, we'll still need the finally block's disable setting update, so that these tests will pass when the release build runs?

Copy link
Contributor Author

@nicktindall nicktindall Sep 30, 2025

Choose a reason for hiding this comment

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

ESSingleNodeTestCase checks that there is no persistent metadata (including settings) left behind in teardown, so we need to clear these settings before the test ends.

We need to clear the settings, rather than setting them to a specific value. I've added a helper for that too in 9845c38

Copy link
Contributor Author

@nicktindall nicktindall Sep 30, 2025

Choose a reason for hiding this comment

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

The changes here are just to stop depending on the default being one way or another, because this will change depending on the feature flag.

);
try {
// Force a ClusterInfo refresh to run collection of the node thread pool usage stats.
Expand Down Expand Up @@ -365,7 +363,10 @@ public void testShardWriteLoadsArePresent() {

final InternalClusterInfoService clusterInfoService = (InternalClusterInfoService) getInstanceFromNode(ClusterInfoService.class);

// Not collecting stats yet because allocation write load stats collection is disabled by default.
// Explicitly disable write load decider
setWriteLoadDeciderEnablement(WriteLoadConstraintSettings.WriteLoadDeciderStatus.DISABLED);

// Stats should not be collected when the decider is disabled
{
ClusterInfoServiceUtils.refresh(clusterInfoService);
final Map<ShardId, Double> shardWriteLoads = clusterInfoService.getClusterInfo().getShardWriteLoads();
Expand All @@ -374,15 +375,10 @@ public void testShardWriteLoadsArePresent() {
}

// Turn on collection of write-load stats.
updateClusterSettings(
Settings.builder()
.put(
WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey(),
randomBoolean()
? WriteLoadConstraintSettings.WriteLoadDeciderStatus.ENABLED
: WriteLoadConstraintSettings.WriteLoadDeciderStatus.LOW_THRESHOLD_ONLY
)
.build()
setWriteLoadDeciderEnablement(
randomBoolean()
? WriteLoadConstraintSettings.WriteLoadDeciderStatus.ENABLED
: WriteLoadConstraintSettings.WriteLoadDeciderStatus.LOW_THRESHOLD_ONLY
);

try {
Expand Down Expand Up @@ -764,6 +760,12 @@ public void postDelete(ShardId shardId, Engine.Delete delete, Engine.DeleteResul
}
}

private void setWriteLoadDeciderEnablement(WriteLoadConstraintSettings.WriteLoadDeciderStatus status) {
updateClusterSettings(
Settings.builder().put(WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey(), status).build()
);
}

public static final IndexShard recoverShard(IndexShard newShard) throws IOException {
DiscoveryNode localNode = DiscoveryNodeUtils.builder("foo").roles(emptySet()).build();
newShard.markAsRecovering("store", new RecoveryState(newShard.routingEntry(), localNode, null));
Expand Down
10 changes: 8 additions & 2 deletions ...c/main/java/org/elasticsearch/cluster/routing/allocation/WriteLoadConstraintSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.unit.RatioValue;
import org.elasticsearch.common.util.FeatureFlag;
import org.elasticsearch.core.TimeValue;

/**
Expand All @@ -23,6 +24,7 @@
public class WriteLoadConstraintSettings {

private static final String SETTING_PREFIX = "cluster.routing.allocation.write_load_decider.";
private static final FeatureFlag WRITE_LOAD_DECIDER_ENABLED_FF = new FeatureFlag("write_load_decider_enabled");
Copy link
Contributor

@DiannaHohensee DiannaHohensee Sep 29, 2025

Choose a reason for hiding this comment

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

I'd probably get rid of 'enabled' in both names, as it's redundant, but doesn't really matter.

Copy link
Contributor Author

@nicktindall nicktindall Sep 30, 2025

Choose a reason for hiding this comment

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

Good call, improved in 917c617


public enum WriteLoadDeciderStatus {
/**
Expand Down Expand Up @@ -59,7 +61,7 @@ public boolean disabled() {
public static final Setting<WriteLoadDeciderStatus> WRITE_LOAD_DECIDER_ENABLED_SETTING = Setting.enumSetting(
WriteLoadDeciderStatus.class,
SETTING_PREFIX + "enabled",
WriteLoadDeciderStatus.DISABLED,
WriteLoadDeciderStatus.ENABLED,
Setting.Property.Dynamic,
Setting.Property.NodeScope
);
Expand Down Expand Up @@ -113,7 +115,11 @@ public boolean disabled() {
private volatile TimeValue queueLatencyThreshold;

public WriteLoadConstraintSettings(ClusterSettings clusterSettings) {
clusterSettings.initializeAndWatch(WRITE_LOAD_DECIDER_ENABLED_SETTING, status -> this.writeLoadDeciderStatus = status);
if (WRITE_LOAD_DECIDER_ENABLED_FF.isEnabled()) {
clusterSettings.initializeAndWatch(WRITE_LOAD_DECIDER_ENABLED_SETTING, status -> this.writeLoadDeciderStatus = status);
} else {
writeLoadDeciderStatus = WriteLoadDeciderStatus.DISABLED;
}
Copy link
Member

@ywangd ywangd Sep 29, 2025

Choose a reason for hiding this comment

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

I think this means we cannot test by overriding the setting in a serverless QA enviroment. Is this intentional? Or maybe we should set the default accordingly based on the feature flag and leave the dynamic update always enabled?

Copy link
Contributor Author

@nicktindall nicktindall Sep 29, 2025

Choose a reason for hiding this comment

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

Good idea! changed in 149b2ea

Copy link
Contributor Author

@nicktindall nicktindall Sep 29, 2025

Choose a reason for hiding this comment

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

Originally I thought we'd just have to restart the cluster the first time it was turned on (to enable the feature flag). But your suggested approach is much better.

clusterSettings.initializeAndWatch(
WRITE_LOAD_DECIDER_REROUTE_INTERVAL_SETTING,
timeValue -> this.minimumRerouteInterval = timeValue
Expand Down
11 changes: 8 additions & 3 deletions ...org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ public void testWriteLoadDeciderDisabled() {
String indexName = "test-index";
var testHarness = createClusterStateAndRoutingAllocation(indexName);

// The write load decider is disabled by default.

var writeLoadDecider = createWriteLoadConstraintDecider(Settings.builder().build());
var writeLoadDecider = createWriteLoadConstraintDecider(
Settings.builder()
.put(
WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey(),
WriteLoadConstraintSettings.WriteLoadDeciderStatus.DISABLED
)
.build()
);

assertEquals(
Decision.Type.YES,
Expand Down