Skip to content

Commit

Permalink
Switch latest OpenSearch version for tests to 2.12 (#18703)
Browse files Browse the repository at this point in the history
* Switch latest OpenSearch version for tests to 2.12

Fix the MessagesBatchIT test with OpenSearch 2.12 by using a low
"http.max_content_length" setting instead of an OpenSearch node with low
heap. This works around circuit breaker issues with OpenSearch and JDK
21.

* Revert some unnecessary changes

* Revert more changes

* Set indices.breaker.total.use_real_memory for MessagesBatchOS2IT

* Add comment

* Include the container env in the ContainerCacheKey
  • Loading branch information
bernd authored Mar 21, 2024
1 parent ec3d6ff commit 475f722
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.net.URL;
import java.nio.file.Paths;
import java.util.List;
import java.util.Map;

public class RunningElasticsearchInstanceES7 implements SearchServerInstance {
private final RestHighLevelClient restHighLevelClient;
Expand Down Expand Up @@ -91,7 +92,7 @@ public FixtureImporter fixtureImporter() {
}

@Override
public GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize) {
public GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize, Map<String, String> env) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@
import org.graylog2.indexer.messages.MessagesBatchIT;
import org.junit.Rule;

import java.util.Collections;

public class MessagesBatchOS2IT extends MessagesBatchIT {
@Rule
public final OpenSearchInstance openSearchInstance = OpenSearchInstanceBuilder.builder().heapSize("256m").build();
public final OpenSearchInstance openSearchInstance = OpenSearchInstanceBuilder.builder()
.heapSize("256m")
// Disable the real memory circuit breaker because it has issues with JDK 21. Turning it off relaxes
// the limits in the circuit breaker and makes our test work correctly.
// See: https://github.com/opensearch-project/OpenSearch/issues/12694
// https://opensearch.org/docs/latest/install-and-configure/configuring-opensearch/circuit-breaker/#parent-circuit-breaker-settings
.env("indices.breaker.total.use_real_memory", "false")
.build();

@Override
protected SearchServerInstance searchServer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ public class OpenSearchInstance extends TestableSearchServerInstance {
private List<String> featureFlags;

public OpenSearchInstance(final SearchVersion version, final String hostname, final Network network, final String heapSize, final List<String> featureFlags) {
super(version, hostname, network, heapSize);
this(version, hostname, network, heapSize, featureFlags, Map.of());
}

public OpenSearchInstance(final SearchVersion version, final String hostname, final Network network, final String heapSize, final List<String> featureFlags, Map<String, String> env) {
super(version, hostname, network, heapSize, env);
this.featureFlags = featureFlags;
}

Expand Down Expand Up @@ -226,6 +230,8 @@ public GenericContainer<?> buildContainer(String image, Network network) {
.withNetwork(network)
.withNetworkAliases(hostname);

getContainerEnv().forEach(container::withEnv);

// disabling the performance plugin in 2.0.1 consistently created errors during CI runs, but keeping it running
// in later versions sometimes created errors on CI, too.
if (version().satisfies(SearchVersion.Distribution.OPENSEARCH, "^2.7.0")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ public static OpenSearchInstanceBuilder builder() {

@Override
protected OpenSearchInstance instantiate() {
return new OpenSearchInstance(getVersion(), getHostname(), getNetwork(), getHeapSize(), getFeatureFlags()).init();
return new OpenSearchInstance(getVersion(), getHostname(), getNetwork(), getHeapSize(), getFeatureFlags(), getEnv()).init();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@
import org.testcontainers.containers.Network;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public abstract class SearchServerBuilder<T extends SearchServerInstance> {
public static final String DEFAULT_HEAP_SIZE = "2g";
private final SearchVersion version;
private String hostname = "indexer";
private Network network;
private String heapSize = DEFAULT_HEAP_SIZE;
private final Map<String, String> env = new HashMap<>();
private List<String> featureFlags = List.of();
private String mongoDbUri;
private String passwordSecret;
Expand Down Expand Up @@ -60,6 +63,15 @@ public String getHeapSize() {
return heapSize;
}

public SearchServerBuilder<T> env(final String key, final String value) {
this.env.put(key, value);
return this;
}

public Map<String, String> getEnv() {
return env;
}

public SearchServerBuilder<T> featureFlags(final List<String> featureFlags) {
this.featureFlags = new ArrayList<>(featureFlags);
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public enum SearchServer {
OS1(OPENSEARCH, "1.3.12"),
OS2(OPENSEARCH, "2.0.1"),
OS2_4(OPENSEARCH, "2.4.1"),
OS2_LATEST(OPENSEARCH, "2.11.0"),
OS2_LATEST(OPENSEARCH, "2.12.0"),
DATANODE_PRE_52(DATANODE, "5.1.0"),
DATANODE_DEV(DATANODE, "5.2.0");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,9 @@
*/
package org.graylog.testing.elasticsearch;

public record ContainerCacheKey(org.graylog2.storage.SearchVersion version, String heapSize) {
import org.graylog2.storage.SearchVersion;

import java.util.Map;

public record ContainerCacheKey(SearchVersion version, String heapSize, Map<String, String> env) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.testcontainers.containers.Network;

import java.io.Closeable;
import java.util.Map;

public interface SearchServerInstance extends Closeable {
Client client();
Expand All @@ -30,7 +31,7 @@ public interface SearchServerInstance extends Closeable {

FixtureImporter fixtureImporter();

GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize);
GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize, Map<String, String> env);

GenericContainer<?> buildContainer(String image, Network network);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public abstract class TestableSearchServerInstance extends ExternalResource impl
protected final String heapSize;
protected final Network network;
protected final String hostname;
private final Map<String, String> env;
protected GenericContainer<?> container;

protected static volatile boolean isFirstContainerStart = true;
Expand All @@ -59,22 +60,27 @@ public abstract class TestableSearchServerInstance extends ExternalResource impl
public abstract FixtureImporter fixtureImporter();

protected TestableSearchServerInstance(final SearchVersion version, final String hostname, final Network network, final String heapSize) {
this(version, hostname, network, heapSize, Map.of());
}

protected TestableSearchServerInstance(final SearchVersion version, final String hostname, final Network network, final String heapSize, Map<String, String> env) {
this.version = version;
this.heapSize = heapSize;
this.network = network;
this.hostname = hostname;
this.env = env;
}

protected abstract String imageName();

public void createContainer() {
this.container = createContainer(version, network, heapSize);
this.container = createContainer(version, network, heapSize, env);
}

@Override
public GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize) {
public GenericContainer<?> createContainer(SearchVersion version, Network network, String heapSize, Map<String, String> env) {
final var image = imageName();
final ContainerCacheKey cacheKey = new ContainerCacheKey(version, heapSize);
final ContainerCacheKey cacheKey = new ContainerCacheKey(version, heapSize, env);
if (!containersByVersion.containsKey(cacheKey)) {
LOG.debug("Creating instance {}", image);
GenericContainer<?> container = buildContainer(image, network);
Expand Down Expand Up @@ -145,6 +151,10 @@ protected String getEsJavaOpts() {
return StringUtils.f("-Xms%s -Xmx%s -Dlog4j2.formatMsgNoLookups=true", heapSize, heapSize);
}

protected Map<String, String> getContainerEnv() {
return env;
}

@Override
public String getHttpHostAddress() {
return this.container.getHost() + ":" + this.container.getMappedPort(9200);
Expand Down

0 comments on commit 475f722

Please sign in to comment.