Skip to content
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

Fix flaky integ tests - CreateRemoteIndexIT,CreateRemoteIndexTranslogDisabledIT,CreateRemoteIndexClusterDefaultDocRep #7141

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Fix flaky integ tests - CreateRemoteIndexIT,CreateRemoteIndexTranslogDisabledIT,CreateRemoteIndexClusterDefaultDocRep #7141

Show file tree
Hide file tree
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
15 changes: 6 additions & 9 deletions ...nalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ protected Settings nodeSettings(int nodeOriginal) {

@Override
public void testRemoteStoreTranslogDisabledByUser() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.build();
Expand All @@ -52,10 +51,9 @@ public void testRemoteStoreTranslogDisabledByUser() throws Exception {

@Override
public void testDefaultRemoteStoreNoUserOverride() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.build();
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
Expand All @@ -68,10 +66,9 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
}

public void testDefaultRemoteStoreNoUserOverrideExceptReplicationTypeSegment() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
Expand Down
97 changes: 43 additions & 54 deletions server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.opensearch.remotestore;

import org.junit.After;
import org.junit.Before;
import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
Expand All @@ -18,8 +19,6 @@
import org.opensearch.test.FeatureFlagSetter;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.nio.file.Path;

import static org.hamcrest.Matchers.containsString;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY;
Expand All @@ -37,6 +36,13 @@
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
public class CreateRemoteIndexIT extends OpenSearchIntegTestCase {

@After
public void teardown() {
assertAcked(clusterAdmin().prepareDeleteRepository("my-segment-repo-1"));
assertAcked(clusterAdmin().prepareDeleteRepository("my-translog-repo-1"));
assertAcked(clusterAdmin().prepareDeleteRepository("my-custom-repo"));
}

@Override
protected Settings nodeSettings(int nodeOriginal) {
Settings settings = super.nodeSettings(nodeOriginal);
Expand Down Expand Up @@ -65,29 +71,27 @@ public void setup() {
FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
FeatureFlagSetter.set(FeatureFlags.REPLICATION_TYPE);
internalCluster().startClusterManagerOnlyNode();
Path absolutePath = randomRepoPath().toAbsolutePath();
assertAcked(
clusterAdmin().preparePutRepository("my-segment-repo-1")
.setType("fs")
.setSettings(Settings.builder().put("location", absolutePath))
.setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
);
assertAcked(
clusterAdmin().preparePutRepository("my-translog-repo-1")
.setType("fs")
.setSettings(Settings.builder().put("location", absolutePath))
.setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
);
assertAcked(
clusterAdmin().preparePutRepository("my-custom-repo")
.setType("fs")
.setSettings(Settings.builder().put("location", absolutePath))
.setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
);
}

public void testDefaultRemoteStoreNoUserOverride() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
GetIndexResponse getIndexResponse = client().admin()
Expand All @@ -107,10 +111,9 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
}

public void testRemoteStoreDisabledByUser() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_STORE_ENABLED, false)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
Expand All @@ -123,10 +126,9 @@ public void testRemoteStoreDisabledByUser() throws Exception {
}

public void testRemoteStoreEnabledByUserWithoutRemoteRepoAndSegmentReplicationIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.build();

Expand All @@ -141,10 +143,9 @@ public void testRemoteStoreEnabledByUserWithoutRemoteRepoAndSegmentReplicationIl
}

public void testRemoteStoreEnabledByUserWithoutRemoteRepoIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.build();
Expand All @@ -160,10 +161,9 @@ public void testRemoteStoreEnabledByUserWithoutRemoteRepoIllegalArgumentExceptio
}

public void testReplicationTypeDocumentByUser() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
Expand All @@ -176,10 +176,9 @@ public void testReplicationTypeDocumentByUser() throws Exception {
}

public void testRemoteStoreSegmentRepoWithoutRemoteEnabledAndSegmentReplicationIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
.build();
IllegalArgumentException exc = expectThrows(
Expand All @@ -193,10 +192,9 @@ public void testRemoteStoreSegmentRepoWithoutRemoteEnabledAndSegmentReplicationI
}

public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -220,10 +218,9 @@ public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
}

public void testRemoteStoreTranslogDisabledByUser() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
Expand All @@ -236,10 +233,9 @@ public void testRemoteStoreTranslogDisabledByUser() throws Exception {
}

public void testRemoteStoreOverrideOnlyTranslogRepoIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, "my-custom-repo")
.build();
IllegalArgumentException exc = expectThrows(
Expand All @@ -255,10 +251,9 @@ public void testRemoteStoreOverrideOnlyTranslogRepoIllegalArgumentException() th
}

public void testRemoteStoreOverrideOnlyTranslogEnabled() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.build();

Expand All @@ -275,10 +270,9 @@ public void testRemoteStoreOverrideOnlyTranslogEnabled() throws Exception {
}

public void testRemoteStoreOverrideOnlyTranslogEnabledAndRepo() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, "my-custom-repo")
.build();
Expand All @@ -296,10 +290,9 @@ public void testRemoteStoreOverrideOnlyTranslogEnabledAndRepo() throws Exception
}

public void testRemoteStoreOverrideTranslogDisabledCorrectly() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -315,10 +308,9 @@ public void testRemoteStoreOverrideTranslogDisabledCorrectly() throws Exception
}

public void testRemoteStoreOverrideTranslogDisabledWithTranslogRepoIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -338,10 +330,9 @@ public void testRemoteStoreOverrideTranslogDisabledWithTranslogRepoIllegalArgume
}

public void testRemoteStoreOverrideOnlyTranslogRepoWithRemoteStoreEnabledIllegalArgumentException() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -360,10 +351,9 @@ public void testRemoteStoreOverrideOnlyTranslogRepoWithRemoteStoreEnabledIllegal
}

public void testRemoteStoreOverrideTranslogRepoCorrectly() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -388,10 +378,9 @@ public void testRemoteStoreOverrideTranslogRepoCorrectly() throws Exception {
}

public void testRemoteStoreOverrideReplicationTypeIndexSettings() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
Expand Down
10 changes: 4 additions & 6 deletions ...ernalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexTranslogDisabledIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ protected Settings nodeSettings(int nodeOriginal) {
}

public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
Expand All @@ -54,10 +53,9 @@ public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
}

public void testDefaultRemoteStoreNoUserOverride() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
Comment on lines -59 to +58
Copy link
Collaborator

Choose a reason for hiding this comment

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

why were the tests flaky ?

Copy link
Member

Choose a reason for hiding this comment

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

+1. Also how have you determined that they are no longer flaky?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The failures were due to the following error:

java.lang.AssertionError: [test-idx-1][2], node[vX6f0DTmRfC1il8YDrSIkQ], [P], s[STARTED], a[id=53rPm8H3SrepttaigUli0A] has unreleased snapshotted index commits

java.lang.RuntimeException: file handle leaks: [InputStream(/var/jenkins/workspace/gradle-check/search/server/build/testrun/internalClusterTest/temp/org.opensearch.remotestore.CreateRemoteIndexIT_58A3D614EB68203F-001/tempDir-008/repos/fgUzGkzgHm/qWmYzVJSTKeMhahfEsSuhw/0/segments/data/segment_infos_snapshot_filename__3__PXCpdocBnatMeQEWYB27)]

https://build.ci.opensearch.org/job/gradle-check/13937/testReport/junit/org.opensearch.remotestore/CreateRemoteIndexIT/testRemoteStoreDisabledByUser/

I was able to repro with following params:

./gradlew ':server:internalClusterTest' --tests "org.opensearch.remotestore.CreateRemoteIndexIT" -Dtests.seed=58A3D614EB68203F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en -Dtests.timezone=Etc/UTC -Dtests.iters=20

./gradlew ':server:internalClusterTest' --tests "org.opensearch.remotestore.CreateRemoteIndexTranslogDisabledIT" -Dtests.seed=58A3D614EB68203F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en -Dtests.timezone=Etc/UTC -Dtests.iters=20

./gradlew ':server:internalClusterTest' --tests "org.opensearch.remotestore.CreateRemoteIndexClusterDefaultDocRep" -Dtests.seed=58A3D614EB68203F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en -Dtests.timezone=Etc/UTC -Dtests.iters=20

Mostly it was because we were not cleaning up snapshot dir.

After my changes, i've ran above 3 commands multiple times and havent seen any failures.

changes around replica count and shard count are done just to simplify the tests further, our main goal with these tests is to verify cluster level settings behavior with remote store

.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
GetIndexResponse getIndexResponse = client().admin()
Expand Down