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 @@ -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

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

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

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 All @@ -346,9 +344,7 @@ public void testNodeWriteLoadsArePresent() {
assertThat(writeThreadPoolStats.maxThreadPoolQueueLatencyMillis(), greaterThanOrEqualTo(0L));
}
} finally {
updateClusterSettings(
Settings.builder().putNull(WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey()).build()
);
clearWriteLoadDeciderEnablementSetting();
}
}

Expand All @@ -365,27 +361,25 @@ public void testShardWriteLoadsArePresent() {

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

// Not collecting stats yet because allocation write load stats collection is disabled by default.
{
ClusterInfoServiceUtils.refresh(clusterInfoService);
final Map<ShardId, Double> shardWriteLoads = clusterInfoService.getClusterInfo().getShardWriteLoads();
assertNotNull(shardWriteLoads);
assertTrue(shardWriteLoads.isEmpty());
}

// 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()
);
// Explicitly disable write load decider
setWriteLoadDeciderEnablement(WriteLoadConstraintSettings.WriteLoadDeciderStatus.DISABLED);

try {
// Stats should not be collected when the decider is disabled
{
ClusterInfoServiceUtils.refresh(clusterInfoService);
final Map<ShardId, Double> shardWriteLoads = clusterInfoService.getClusterInfo().getShardWriteLoads();
assertNotNull(shardWriteLoads);
assertTrue(shardWriteLoads.isEmpty());
}

// Turn on collection of write-load stats.
setWriteLoadDeciderEnablement(
randomBoolean()
? WriteLoadConstraintSettings.WriteLoadDeciderStatus.ENABLED
: WriteLoadConstraintSettings.WriteLoadDeciderStatus.LOW_THRESHOLD_ONLY
);

// Force a ClusterInfo refresh to run collection of the write-load stats.
ClusterInfoServiceUtils.refresh(clusterInfoService);
final Map<ShardId, Double> shardWriteLoads = clusterInfoService.getClusterInfo().getShardWriteLoads();
Expand All @@ -404,12 +398,14 @@ public void testShardWriteLoadsArePresent() {
assertThat(maximumLoadRecorded, greaterThan(0.0));
}
} finally {
updateClusterSettings(
Settings.builder().putNull(WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey()).build()
);
clearWriteLoadDeciderEnablementSetting();
}
}

private void clearWriteLoadDeciderEnablementSetting() {
updateClusterSettings(Settings.builder().putNull(WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey()).build());
}

public void testMaxHeapPerNodeIsPresent() {
InternalClusterInfoService clusterInfoService = (InternalClusterInfoService) getInstanceFromNode(ClusterInfoService.class);
ClusterInfoServiceUtils.refresh(clusterInfoService);
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
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_FEATURE_FLAG = new FeatureFlag("write_load_decider");

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,
WRITE_LOAD_DECIDER_FEATURE_FLAG.isEnabled() ? WriteLoadDeciderStatus.ENABLED : WriteLoadDeciderStatus.DISABLED,
Setting.Property.Dynamic,
Setting.Property.NodeScope
);
Expand Down
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