Pass override settings when creating test cluster #71203
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today when creating an internal test cluster, we allow the test to supply the node settings that are applied. The extension point to provide these settings has a single integer parameter, indicating the index (zero-based) of the node being constructed. This allows the test to make some decisions about the settings to return, but it is too simplistic. For example, imagine a test that wants to provide a setting, but some values for that setting are not valid on non-data nodes. Since the only information the test has about the node being constructed is its index, it does not have sufficient information to determine if the node being constructed is a non-data node or not, since this is done by the test framework externally by overriding the final settings with specific settings that dicate the roles of the node. This commit changes the test framework so that the test has information about what settings are going to be overriden by the test framework after the test provide its test-specific settings. This allows the test to make informed decisions about what values it can return to the test framework.
jasontedor
added
>non-issue
:Delivery/Build
elasticmachine
added
Team:Delivery
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
Pinging @elastic/es-delivery (Team:Delivery) |
|
Here is an example of how this would be used: diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java
index bf4d97d2b2f..286e9a41c92 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java
@@ -16,7 +16,8 @@ package org.elasticsearch.xpack.searchablesnapshots;
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
-import org.elasticsearch.xpack.searchablesnapshots.cache.blob.BlobStoreCacheService;
+import org.elasticsearch.cluster.node.DiscoveryNode;
+import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -24,9 +25,11 @@ import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
+import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction;
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest;
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest.Storage;
+import org.elasticsearch.xpack.searchablesnapshots.cache.blob.BlobStoreCacheService;
import org.elasticsearch.xpack.searchablesnapshots.cache.full.CacheService;
import org.elasticsearch.xpack.searchablesnapshots.cache.shared.FrozenCacheService;
import org.junit.After;
@@ -35,13 +38,16 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
+import java.util.Set;
import java.util.concurrent.TimeUnit;
import static org.elasticsearch.license.LicenseService.SELF_GENERATED_LICENSE_TYPE;
+import static org.elasticsearch.test.NodeRoles.addRoles;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.xpack.searchablesnapshots.cache.shared.SharedBytes.pageAligned;
import static org.hamcrest.Matchers.equalTo;
+@ESIntegTestCase.ClusterScope(supportsDedicatedMasters = false, numClientNodes = 0)
public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnapshotIntegTestCase {
@Override
protected boolean addMockInternalEngine() {
@@ -55,9 +61,16 @@ public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnaps
@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
- final Settings.Builder builder = Settings.builder()
- .put(super.nodeSettings(nodeOrdinal, otherSettings))
- .put(SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
+ final Settings settings;
+ {
+ final Settings initialSettings = super.nodeSettings(nodeOrdinal, otherSettings);
+ if (DiscoveryNode.canContainData(otherSettings)) {
+ settings = addRoles(initialSettings, Set.of(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE));
+ } else {
+ settings = initialSettings;
+ }
+ }
+ final Settings.Builder builder = Settings.builder().put(settings).put(SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
if (randomBoolean()) {
builder.put(
CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(),
@@ -76,14 +89,16 @@ public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnaps
: new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB)
);
}
- builder.put(
- FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(),
- rarely()
- ? randomBoolean()
- ? new ByteSizeValue(randomIntBetween(0, 10), ByteSizeUnit.KB)
- : new ByteSizeValue(randomIntBetween(0, 1000), ByteSizeUnit.BYTES)
- : new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB)
- );
+ if (DiscoveryNode.canContainData(otherSettings)) {
+ builder.put(
+ FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(),
+ rarely()
+ ? randomBoolean()
+ ? new ByteSizeValue(randomIntBetween(0, 10), ByteSizeUnit.KB)
+ : new ByteSizeValue(randomIntBetween(0, 1000), ByteSizeUnit.BYTES)
+ : new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB)
+ );
+ }
builder.put(
FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(),
rarely()
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java
index c42297c914a..b3c84f4e5ac 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsCanMatchOnCoordinatorIntegTests.java
@@ -14,6 +14,7 @@ import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -64,14 +65,19 @@ public class SearchableSnapshotsCanMatchOnCoordinatorIntegTests extends BaseSear
}
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
- return Settings.builder()
- .put(super.nodeSettings(nodeOrdinal))
- // Use an unbound cache so we can recover the searchable snapshot completely all the times
- .put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES))
- // Have a shared cache of reasonable size available on each node because tests randomize over frozen and cold allocation
- .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), ByteSizeValue.ofMb(randomLongBetween(1, 10)))
- .build();
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
+ final Settings initialSettings = super.nodeSettings(nodeOrdinal, otherSettings);
+ if (DiscoveryNode.canContainData(otherSettings)) {
+ return Settings.builder()
+ .put(initialSettings)
+ // Use an unbound cache so we can recover the searchable snapshot completely all the times
+ .put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES))
+ // Have a shared cache of reasonable size available on each node because tests randomize over frozen and cold allocation
+ .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), ByteSizeValue.ofMb(randomLongBetween(1, 10)))
+ .build();
+ } else {
+ return initialSettings;
+ }
}
public void testSearchableSnapshotShardsAreSkippedWithoutQueryingAnyNodeWhenTheyAreOutsideOfTheQueryRange() throws Exception {
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotAllocationIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotAllocationIntegTests.java
index 41c92acd05e..5ed1b7d38e4 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotAllocationIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotAllocationIntegTests.java
@@ -29,9 +29,9 @@ import static org.hamcrest.Matchers.is;
public class SearchableSnapshotAllocationIntegTests extends BaseSearchableSnapshotsIntegTestCase {
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
- .put(super.nodeSettings(nodeOrdinal))
+ .put(super.nodeSettings(nodeOrdinal, otherSettings))
// ensure the cache is definitely used
.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(1L, ByteSizeUnit.GB))
.build();
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java
index c0cf519d0c7..ad3fadf4c85 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java
@@ -42,9 +42,9 @@ public class SearchableSnapshotEnableAllocationDeciderIntegTests extends BaseSea
}
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
- .put(super.nodeSettings(nodeOrdinal))
+ .put(super.nodeSettings(nodeOrdinal, otherSettings))
// Use an unbound cache so we can recover the searchable snapshot completely all the times
.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES))
.build();
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/blob/SearchableSnapshotsBlobStoreCacheIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/blob/SearchableSnapshotsBlobStoreCacheIntegTests.java
index 6f5ada0d1a4..5406559e2c7 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/blob/SearchableSnapshotsBlobStoreCacheIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/blob/SearchableSnapshotsBlobStoreCacheIntegTests.java
@@ -110,8 +110,8 @@ public class SearchableSnapshotsBlobStoreCacheIntegTests extends BaseSearchableS
}
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
- return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(cacheSettings).build();
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
+ return Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings)).put(cacheSettings).build();
}
public void testBlobStoreCache() throws Exception {
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/full/SearchableSnapshotsPersistentCacheIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/full/SearchableSnapshotsPersistentCacheIntegTests.java
index 8ca4f94e1f7..25d348a40c2 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/full/SearchableSnapshotsPersistentCacheIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/full/SearchableSnapshotsPersistentCacheIntegTests.java
@@ -43,9 +43,9 @@ import static org.hamcrest.Matchers.notNullValue;
public class SearchableSnapshotsPersistentCacheIntegTests extends BaseSearchableSnapshotsIntegTestCase {
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
- .put(super.nodeSettings(nodeOrdinal))
+ .put(super.nodeSettings(nodeOrdinal, otherSettings))
// ensure the cache is definitely used
.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(1L, ByteSizeUnit.GB))
// to make cache synchronization predictable
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/PartiallyCachedShardAllocationIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/PartiallyCachedShardAllocationIntegTests.java
index 64a32504227..0099027f427 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/PartiallyCachedShardAllocationIntegTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/PartiallyCachedShardAllocationIntegTests.java
@@ -58,9 +58,9 @@ import static org.hamcrest.Matchers.hasItem;
public class PartiallyCachedShardAllocationIntegTests extends BaseSearchableSnapshotsIntegTestCase {
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return Settings.builder()
- .put(super.nodeSettings(nodeOrdinal))
+ .put(super.nodeSettings(nodeOrdinal, otherSettings))
// default to no cache: the tests create nodes with a cache configured as needed
.put(SNAPSHOT_CACHE_SIZE_SETTING.getKey(), ByteSizeValue.ZERO)
.build();
diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/recovery/SearchableSnapshotRecoveryStateIntegrationTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/recovery/SearchableSnapshotRecoveryStateIntegrationTests.java
index bbdd3c38fdc..e35410053ac 100644
--- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/recovery/SearchableSnapshotRecoveryStateIntegrationTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/recovery/SearchableSnapshotRecoveryStateIntegrationTests.java
@@ -68,8 +68,8 @@ public class SearchableSnapshotRecoveryStateIntegrationTests extends BaseSearcha
}
@Override
- protected Settings nodeSettings(int nodeOrdinal) {
- final Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal));
+ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
+ final Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
builder.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES));
return builder.build(); |
mark-vieira
approved these changes
Apr 1, 2021
|
@elasticmachine update branch |
jasontedor
added a commit
that referenced
this pull request
Apr 2, 2021
Today when creating an internal test cluster, we allow the test to supply the node settings that are applied. The extension point to provide these settings has a single integer parameter, indicating the index (zero-based) of the node being constructed. This allows the test to make some decisions about the settings to return, but it is too simplistic. For example, imagine a test that wants to provide a setting, but some values for that setting are not valid on non-data nodes. Since the only information the test has about the node being constructed is its index, it does not have sufficient information to determine if the node being constructed is a non-data node or not, since this is done by the test framework externally by overriding the final settings with specific settings that dicate the roles of the node. This commit changes the test framework so that the test has information about what settings are going to be overriden by the test framework after the test provide its test-specific settings. This allows the test to make informed decisions about what values it can return to the test framework.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today when creating an internal test cluster, we allow the test to supply the node settings that are applied. The extension point to provide these settings has a single integer parameter, indicating the index (zero-based) of the node being constructed. This allows the test to make some decisions about the settings to return, but it is too simplistic. For example, imagine a test that wants to provide a setting, but some values for that setting are not valid on non-data nodes. Since the only information the test has about the node being constructed is its index, it does not have sufficient information to determine if the node being constructed is a non-data node or not, since this is done by the test framework externally by overriding the final settings with specific settings that dictate the roles of the node. This commit changes the test framework so that the test has information about what settings are going to be overridden by the test framework after the test provide its test-specific settings. This allows the test to make informed decisions about what values it can return to the test framework.
Blocks #71013