Skip to content

Test: don't modify defaultConfig on upgrade #55560

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 1 commit into from
Apr 22, 2020
Merged

Test: don't modify defaultConfig on upgrade #55560

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 23 additions & 22 deletions buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,44 +1049,45 @@ private void syncWithLinks(Path sourceRoot, Path destinationRoot) {

private void createConfiguration() {
String nodeName = nameCustomization.apply(safeName(name));
Map<String, String> baseConfig = new HashMap<>(defaultConfig);
if (nodeName != null) {
defaultConfig.put("node.name", nodeName);
}
defaultConfig.put("path.repo", confPathRepo.toAbsolutePath().toString());
defaultConfig.put("path.data", confPathData.toAbsolutePath().toString());
defaultConfig.put("path.logs", confPathLogs.toAbsolutePath().toString());
defaultConfig.put("path.shared_data", workingDir.resolve("sharedData").toString());
defaultConfig.put("node.attr.testattr", "test");
defaultConfig.put("node.portsfile", "true");
defaultConfig.put("http.port", httpPort);
baseConfig.put("node.name", nodeName);
}
baseConfig.put("path.repo", confPathRepo.toAbsolutePath().toString());
baseConfig.put("path.data", confPathData.toAbsolutePath().toString());
baseConfig.put("path.logs", confPathLogs.toAbsolutePath().toString());
baseConfig.put("path.shared_data", workingDir.resolve("sharedData").toString());
baseConfig.put("node.attr.testattr", "test");
baseConfig.put("node.portsfile", "true");
baseConfig.put("http.port", httpPort);
if (getVersion().onOrAfter(Version.fromString("6.7.0"))) {
defaultConfig.put("transport.port", transportPort);
baseConfig.put("transport.port", transportPort);
} else {
defaultConfig.put("transport.tcp.port", transportPort);
baseConfig.put("transport.tcp.port", transportPort);
}
// Default the watermarks to absurdly low to prevent the tests from failing on nodes without enough disk space
defaultConfig.put("cluster.routing.allocation.disk.watermark.low", "1b");
defaultConfig.put("cluster.routing.allocation.disk.watermark.high", "1b");
baseConfig.put("cluster.routing.allocation.disk.watermark.low", "1b");
baseConfig.put("cluster.routing.allocation.disk.watermark.high", "1b");
// increase script compilation limit since tests can rapid-fire script compilations
defaultConfig.put("script.max_compilations_rate", "2048/1m");
baseConfig.put("script.max_compilations_rate", "2048/1m");
if (getVersion().getMajor() >= 6) {
defaultConfig.put("cluster.routing.allocation.disk.watermark.flood_stage", "1b");
baseConfig.put("cluster.routing.allocation.disk.watermark.flood_stage", "1b");
}
// Temporarily disable the real memory usage circuit breaker. It depends on real memory usage which we have no full control
// over and the REST client will not retry on circuit breaking exceptions yet (see #31986 for details). Once the REST client
// can retry on circuit breaking exceptions, we can revert again to the default configuration.
if (getVersion().getMajor() >= 7) {
defaultConfig.put("indices.breaker.total.use_real_memory", "false");
baseConfig.put("indices.breaker.total.use_real_memory", "false");
}
// Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master
defaultConfig.put("discovery.initial_state_timeout", "0s");
baseConfig.put("discovery.initial_state_timeout", "0s");

if (getVersion().getMajor() >= 8) {
defaultConfig.put("cluster.service.slow_task_logging_threshold", "5s");
defaultConfig.put("cluster.service.slow_master_task_logging_threshold", "5s");
baseConfig.put("cluster.service.slow_task_logging_threshold", "5s");
baseConfig.put("cluster.service.slow_master_task_logging_threshold", "5s");
}

HashSet<String> overriden = new HashSet<>(defaultConfig.keySet());
HashSet<String> overriden = new HashSet<>(baseConfig.keySet());
overriden.retainAll(settings.keySet());
overriden.removeAll(OVERRIDABLE_SETTINGS);
if (overriden.isEmpty() == false) {
Expand All @@ -1095,12 +1096,12 @@ private void createConfiguration() {
);
}
// Make sure no duplicate config keys
settings.keySet().stream().filter(OVERRIDABLE_SETTINGS::contains).forEach(defaultConfig::remove);
settings.keySet().stream().filter(OVERRIDABLE_SETTINGS::contains).forEach(baseConfig::remove);

try {
Files.write(
configFile,
Stream.concat(settings.entrySet().stream(), defaultConfig.entrySet().stream())
Stream.concat(settings.entrySet().stream(), baseConfig.entrySet().stream())
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused. Is the configuration in defaultConfig not needed? ElasticsearchCluster.commonNodeConfig() places a bunch of stuff in there. Won't this change now no longer write those config items to the config file or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On line 1052 we initialize baseConfig with defaultConfig:
Map<String, String> baseConfig = new HashMap<>(defaultConfig);

Copy link
Contributor

Choose a reason for hiding this comment

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

That's indeed what I was missing.

.map(entry -> entry.getKey() + ": " + entry.getValue())
.collect(Collectors.joining("\n"))
.getBytes(StandardCharsets.UTF_8),
Expand Down