Skip to content

Commit

Permalink
Dry up additional spots of setting cluster settings in internal clust…
Browse files Browse the repository at this point in the history
…er tests (#94243)

Follow up to #94213 dealing with the remaining spots.
This also moves some use of deprecated transient settings to
persisted settings and deprecates the request builder methods for
transient settings to hopefully prevent more use of transients.
  • Loading branch information
original-brownbear committed Mar 9, 2023
1 parent ba499e7 commit 1793226
Show file tree
Hide file tree
Showing 18 changed files with 79 additions and 207 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.search.MultiSearchResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.bytes.BytesArray;
Expand Down Expand Up @@ -1228,9 +1227,7 @@ public void testDisallowExpensiveQueries() throws IOException {
assertThat(response.getHits().getAt(0).getFields().get("_percolator_document_slot").getValue(), equalTo(0));

// Set search.allow_expensive_queries to "false" => assert failure
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", false));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", false));

ElasticsearchException e = expectThrows(
ElasticsearchException.class,
Expand All @@ -1242,18 +1239,14 @@ public void testDisallowExpensiveQueries() throws IOException {
);

// Set search.allow_expensive_queries setting to "true" ==> success
updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", true));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", true));

response = client().prepareSearch().setQuery(new PercolateQueryBuilder("query", source, XContentType.JSON)).get();
assertHitCount(response, 1);
assertThat(response.getHits().getAt(0).getId(), equalTo("1"));
assertThat(response.getHits().getAt(0).getFields().get("_percolator_document_slot").getValue(), equalTo(0));
} finally {
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().putNull("search.allow_expensive_queries"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import joptsimple.OptionSet;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cluster.ClusterState;
Expand Down Expand Up @@ -358,10 +357,7 @@ public void testCanRunUnsafeBootstrapAfterErroneousDetachWithoutLoosingMetadata(
internalCluster().setBootstrapMasterNodeIndex(0);
String masterNode = internalCluster().startMasterOnlyNode();
Settings masterNodeDataPathSettings = internalCluster().dataPathSettings(masterNode);
ClusterUpdateSettingsRequest req = new ClusterUpdateSettingsRequest().persistentSettings(
Settings.builder().put(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey(), "1234kb")
);
internalCluster().client().admin().cluster().updateSettings(req).get();
updateClusterSettings(Settings.builder().put(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey(), "1234kb"));

ClusterState state = internalCluster().client().admin().cluster().prepareState().execute().actionGet().getState();
assertThat(state.metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()), equalTo("1234kb"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,17 +424,12 @@ public void testMovesShardsOffSpecificDataPathAboveWatermark() throws Exception
// start with all paths below the watermark
clusterInfoService.setDiskUsageFunctionAndRefresh((discoveryNode, fsInfoPath) -> setDiskUsage(fsInfoPath, 100, between(10, 100)));

assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "90%")
.put(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "90%")
.put(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "100%")
.put(CLUSTER_ROUTING_ALLOCATION_REROUTE_INTERVAL_SETTING.getKey(), "0ms")
)
updateClusterSettings(
Settings.builder()
.put(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "90%")
.put(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "90%")
.put(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "100%")
.put(CLUSTER_ROUTING_ALLOCATION_REROUTE_INTERVAL_SETTING.getKey(), "0ms")
);

final List<String> nodeIds = client().admin()
Expand Down Expand Up @@ -467,13 +462,8 @@ public void testMovesShardsOffSpecificDataPathAboveWatermark() throws Exception
logger.info("--> shards on good path: [{}]", shardsOnGoodPath);

// disable rebalancing, or else we might move shards back onto the over-full path since we're not faking that
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder().put(CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), EnableAllocationDecider.Rebalance.NONE)
)
updateClusterSettings(
Settings.builder().put(CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), EnableAllocationDecider.Rebalance.NONE)
);

// one of the paths on node0 suddenly exceeds the high watermark
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,8 @@ private void assertClusterStateNotSaved(CountDownLatch savedClusterState, Atomic

assertThat(clusterStateResponse.getState().metadata().persistentSettings().get("search.allow_expensive_queries"), nullValue());

ClusterUpdateSettingsRequest req = new ClusterUpdateSettingsRequest().persistentSettings(
Settings.builder().put("search.allow_expensive_queries", "false")
);
// This should succeed, nothing was reserved
client().admin().cluster().updateSettings(req).get();
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", "false"));
}

public void testErrorSaved() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.search.query;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.fielddata.ScriptDocValues;
Expand Down Expand Up @@ -143,9 +142,7 @@ public void testDisallowExpensiveQueries() {
assertNoFailures(resp);

// Set search.allow_expensive_queries to "false" => assert failure
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", false));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", false));

ElasticsearchException e = expectThrows(
ElasticsearchException.class,
Expand All @@ -157,15 +154,11 @@ public void testDisallowExpensiveQueries() {
);

// Set search.allow_expensive_queries to "true" => success
updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", true));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", true));
resp = client().prepareSearch("test-index").setQuery(scriptScoreQuery(matchQuery("field1", "text0"), script)).get();
assertNoFailures(resp);
} finally {
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.search.scriptfilter;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexModule;
Expand Down Expand Up @@ -230,9 +229,7 @@ public void testDisallowExpensiveQueries() {
SearchResponse resp = client().prepareSearch("test-index").setQuery(scriptQuery(script)).get();
assertNoFailures(resp);

ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", false));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", false));

// Set search.allow_expensive_queries to "false" => assert failure
ElasticsearchException e = expectThrows(
Expand All @@ -245,15 +242,11 @@ public void testDisallowExpensiveQueries() {
);

// Set search.allow_expensive_queries to "true" => success
updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", true));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", true));
resp = client().prepareSearch("test-index").setQuery(scriptQuery(script)).get();
assertNoFailures(resp);
} finally {
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,26 @@ public ClusterUpdateSettingsRequestBuilder(ElasticsearchClient client, ClusterUp

/**
* Sets the transient settings to be updated. They will not survive a full cluster restart
* @deprecated Transient settings are in the process of being removed. Use
* persistent settings to update your cluster settings instead.
*/
@Deprecated
public ClusterUpdateSettingsRequestBuilder setTransientSettings(Settings settings) {
request.transientSettings(settings);
return this;
}

/**
* Sets the transient settings to be updated. They will not survive a full cluster restart
* @deprecated Transient settings are in the process of being removed. Use
* persistent settings to update your cluster settings instead.
*/
@Deprecated
public ClusterUpdateSettingsRequestBuilder setTransientSettings(Settings.Builder settings) {
request.transientSettings(settings);
return this;
}

/**
* Sets the source containing the transient settings to be updated. They will not survive a full cluster restart
*/
public ClusterUpdateSettingsRequestBuilder setTransientSettings(String settings, XContentType xContentType) {
request.transientSettings(settings, xContentType);
return this;
}

/**
* Sets the transient settings to be updated. They will not survive a full cluster restart
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.action.bulk.BulkResponse;
Expand Down Expand Up @@ -254,9 +253,7 @@ public void testDisallowExpensiveQueries() throws InterruptedException, IOExcept

try {
// Execute with search.allow_expensive_queries to false
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", false));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", false));

SearchRequestBuilder builder = client().prepareSearch("test")
.setQuery(queryBuilder().shapeQuery("shape", new Circle(0, 0, 77000)));
Expand All @@ -272,20 +269,14 @@ public void testDisallowExpensiveQueries() throws InterruptedException, IOExcept
}

// Set search.allow_expensive_queries to "null"
updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertThat(builder.get().getHits().getTotalHits().value, equalTo(1L));

// Set search.allow_expensive_queries to "true"
updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", true));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", true));
assertThat(builder.get().getHits().getTotalHits().value, equalTo(1L));
} finally {
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", (String) null));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.TransportVersion;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
Expand Down Expand Up @@ -42,7 +41,6 @@

import static org.elasticsearch.search.SearchService.MAX_ASYNC_SEARCH_RESPONSE_SIZE_SETTING;
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
Expand Down Expand Up @@ -509,9 +507,7 @@ public void testMaxResponseSize() {
).setKeepOnCompletion(true);

int limit = 1000; // is not big enough to store the response
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.max_async_search_response_size", limit + "b"));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.max_async_search_response_size", limit + "b"));

ExecutionException e = expectThrows(ExecutionException.class, () -> submitAsyncSearch(request));
assertNotNull(e.getCause());
Expand All @@ -527,9 +523,7 @@ public void testMaxResponseSize() {
)
);

updateSettingsRequest = new ClusterUpdateSettingsRequest();
updateSettingsRequest.persistentSettings(Settings.builder().put("search.max_async_search_response_size", (String) null));
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
updateClusterSettings(Settings.builder().put("search.max_async_search_response_size", (String) null));
}

public void testCCSCheckCompatibility() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,8 @@ private void assertClusterStateNotSaved(CountDownLatch savedClusterState, Atomic
.contains("search.allow_expensive_queries") == false
);

ClusterUpdateSettingsRequest req = new ClusterUpdateSettingsRequest().persistentSettings(
Settings.builder().put("search.allow_expensive_queries", "false")
);
// This should succeed, nothing was reserved
client().admin().cluster().updateSettings(req).get();
updateClusterSettings(Settings.builder().put("search.allow_expensive_queries", "false"));
// This will fail because repo-new isn't there, not because we can't write test-snapshots-err, meaning we were allowed to
// make the request
assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,16 @@ public class AutoscalingIT extends MlNativeAutodetectIntegTestCase {

@Before
public void putSettings() {
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder().put(MachineLearning.MAX_LAZY_ML_NODES.getKey(), 100).put("logger.org.elasticsearch.xpack.ml", "DEBUG")
)
.get();
updateClusterSettings(
Settings.builder().put(MachineLearning.MAX_LAZY_ML_NODES.getKey(), 100).put("logger.org.elasticsearch.xpack.ml", "DEBUG")
);
}

@After
public void removeSettings() {
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder().putNull(MachineLearning.MAX_LAZY_ML_NODES.getKey()).putNull("logger.org.elasticsearch.xpack.ml")
)
.get();
updateClusterSettings(
Settings.builder().putNull(MachineLearning.MAX_LAZY_ML_NODES.getKey()).putNull("logger.org.elasticsearch.xpack.ml")
);
cleanUp();
}

Expand Down

0 comments on commit 1793226

Please sign in to comment.