[Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT #6637

Rishikesh1159 · 2023-03-11T23:06:45Z

Description

This PR triggers refreshes on shards using NRT Engine. We made the decision to not support that behavior wait_until with segrep. So this PR reverts changes made to PublishCheckpointAction class by #6366.

This PR fixes relocation bugs and also fixes many of the flaky tests failures in SegmentReplicationIT. Mainly tests:
testRelocateWhileContinuouslyIndexingAndWaitingForRefresh() and testPrimaryRelocation()

Issues Resolved

#6531 and #6665

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-11T23:39:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermarkWithAutoReleaseEnabled
      1 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search.aggregation/20_terms/string profiler via global ordinals}

URL: https://build.ci.opensearch.org/job/gradle-check/12298/
CommitID: f25361f
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov-commenter · 2023-03-11T23:41:09Z

Codecov Report

Merging #6637 (ffd94b1) into main (73a2279) will decrease coverage by 0.51%.
The diff coverage is 37.50%.

❗ Current head ffd94b1 differs from pull request most recent head f3fa803. Consider uploading reports for the commit f3fa803 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6637      +/-   ##
============================================
- Coverage     71.22%   70.71%   -0.51%     
+ Complexity    59521    59132     -389     
============================================
  Files          4803     4803              
  Lines        283208   283190      -18     
  Branches      40842    40836       -6     
============================================
- Hits         201712   200258    -1454     
- Misses        65266    66549    +1283     
- Partials      16230    16383     +153

Impacted Files	Coverage Δ
...eplication/checkpoint/PublishCheckpointAction.java	`28.57% <10.00%> (+5.13%)`	⬆️
.../opensearch/index/engine/NRTReplicationEngine.java	`70.86% <33.33%> (-2.06%)`	⬇️
...arch/index/engine/NRTReplicationReaderManager.java	`88.46% <100.00%> (+0.96%)`	⬆️
...nsearch/index/shard/CheckpointRefreshListener.java	`100.00% <100.00%> (+11.11%)`	⬆️
...in/java/org/opensearch/index/shard/IndexShard.java	`69.71% <100.00%> (-0.85%)`	⬇️

... and 490 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

…6366 Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-12T00:09:47Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12301/
CommitID: 5407a68
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

dreamer-89 · 2023-03-12T18:28:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌

URL: https://build.ci.opensearch.org/job/gradle-check/12301/

CommitID: 5407a68
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Looks like a legit failure @Rishikesh1159 which needs unit test updates.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.indices.replication.checkpoint.PublishCheckpointActionTests.testPublishCheckpointActionOnPrimary" -Dtests.seed=D77C43ABBEE6E324 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ms -Dtests.timezone=Etc/GMT+12 -Druntime.java=19

org.opensearch.indices.replication.checkpoint.PublishCheckpointActionTests > testPublishCheckpointActionOnPrimary FAILED
    junit.framework.AssertionFailedError: Expected exception OpenSearchException but no exception was thrown
        at __randomizedtesting.SeedInfo.seed([D77C43ABBEE6E324:CD7AA1203C0F125B]:0)
        at org.apache.lucene.tests.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2864)
        at org.apache.lucene.tests.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2850)
        at org.opensearch.indices.replication.checkpoint.PublishCheckpointActionTests.testPublishCheckpointActionOnPrimary(PublishCheckpointActionTests.java:109)

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-13T18:00:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.http.SearchRestCancellationIT.testAutomaticCancellationMultiSearchDuringFetchPhase

URL: https://build.ci.opensearch.org/job/gradle-check/12378/
CommitID: a5711a1
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-03-13T22:41:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12389/
CommitID: c8e5154
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-13T23:23:14Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12391/
CommitID: d4379e3

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-14T00:56:54Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermarkWithAutoReleaseEnabled

URL: https://build.ci.opensearch.org/job/gradle-check/12394/
CommitID: 4e3272b
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

mch2 · 2023-03-14T02:24:53Z

server/src/main/java/org/opensearch/index/shard/IndexShard.java

@@ -566,6 +566,9 @@ public void updateShardState(
                    : "a primary relocation is completed by the cluster-managerr, but primary mode is not active " + currentRouting;

                changeState(IndexShardState.STARTED, "global state is [" + newRouting.state() + "]");
+                if (indexSettings.isSegRepEnabled()) {


Can we add a comment here on why this is required? I think this is so new primary shards will push to replicas after relocation rather than waiting for more docs to get indexed?

+1
Also, it would be better to add a test verifying segrep is forced on replica copies.

Yes, new primary shards will push to replicas after relocation rather than waiting for more docs to get indexed

mch2 · 2023-03-14T02:25:34Z

server/src/test/java/org/opensearch/index/shard/SegmentReplicationIndexShardTests.java


            shards.promoteReplicaToPrimary(replica_2).get();
            primary.close("demoted", false);
            primary.store().close();
            IndexShard oldPrimary = shards.addReplicaWithExistingPath(primary.shardPath(), primary.routingEntry().currentNodeId());
            shards.recoverReplica(oldPrimary);
-            assertLatestCommitGen(5, oldPrimary);
-            assertLatestCommitGen(5, replica_2);


Thanks for doing this, this test is flaky and we really don't care about the generation only that the doc counts are equal.

mch2 · 2023-03-14T02:26:56Z

server/src/main/java/org/opensearch/index/engine/NRTReplicationReaderManager.java

@@ -51,6 +51,10 @@ public class NRTReplicationReaderManager extends OpenSearchReaderManager {
    @Override
    protected OpenSearchDirectoryReader refreshIfNeeded(OpenSearchDirectoryReader referenceToRefresh) throws IOException {
        Objects.requireNonNull(referenceToRefresh);
+        // checks if an actual refresh (change in segments) happened
+        if (unwrapStandardReader(referenceToRefresh).getSegmentInfos().version == currentInfos.version) {


Can you pls add a unit test for this change in NRTReplicationEngineTests ?

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

dreamer-89 · 2023-03-14T22:29:46Z

...ernalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationRelocationIT.java

+        // Verify if all docs are present in replica after flush, if new relocated primary doesn't flush after relocation the below assert
+        // will fail


Without waiting for all indexing operations to complete, this test will flaky. I think a better way to test changes would be:

Block operations from older primary to replica.

Insert all docs after above.

Relocate the primary shard.

Without auto refreshes (refresh -1), only way replica will have doc count is via the force flush from new primary.

Basically, we need not perform any refresh and turn all aync calls to sync here. Below is the set of steps we can follow (we don't need to block segment replication to replica).

Index docs in sync without refresh so that only primary contains.

Assert doc count on primary is matching ingested doc count but replica should have 0 doc count (note: we are not performing any refresh).

Relocate primary to new primary node and wait for it to complete.

Assert replica gets all the docs. This should only be possible from flush operation before state switch to STARTED.

Sure, makes sense. This way it will fail more definitively without flush. Thanks @dreamer-89 , I will update the test

Asserting doc count on primary before relocation is not possible as without a refresh even on primary shard, it will have 0 docs, as we open searcher only after a refresh. So instead I am asserting on segrep stats that no replication event has taken place on replica shard before relocation

github-actions · 2023-03-15T00:03:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12438/
CommitID: f7dcfb1

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-15T00:50:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12441/
CommitID: ffd94b1

github-actions · 2023-03-15T01:02:17Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12442/
CommitID: f3fa803
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-03-15T01:36:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.index.SegmentReplicationPressureIT.testWritesRejected
      1 org.opensearch.index.SegmentReplicationPressureIT.testAddReplicaWhileWritesBlocked

URL: https://build.ci.opensearch.org/job/gradle-check/12445/
CommitID: f3fa803
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

dreamer-89

LGTM!
Let's also wait for @mch2 review.

dreamer-89 · 2023-03-15T00:41:36Z

...ernalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationRelocationIT.java

+        // assert
+        // will fail


nit: can be accomodated in a single line.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2023-03-15T05:06:19Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue
      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled
      1 org.opensearch.indices.replication.SegmentReplicationRelocationIT.testFlushAfterRelocation
      1 org.opensearch.index.SegmentReplicationPressureIT.testWritesRejected
      1 org.opensearch.index.SegmentReplicationPressureIT.testAddReplicaWhileWritesBlocked

URL: https://build.ci.opensearch.org/job/gradle-check/12453/
CommitID: fe5e715
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

…estRelocateWhileContinuouslyIndexingAndWaitingForRefresh (#6637) * Trigger Refresh on NRT Engine. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Revert changes made to PublishCheckpointAction in #6366 Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix failing unit test Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Force flush on new elected primary after relocation. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix failing unit test. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Remove unnecessary assertions Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Adding tests. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Address comments Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix indentation. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> --------- Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> (cherry picked from commit 1e5d913) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…estRelocateWhileContinuouslyIndexingAndWaitingForRefresh (#6637) (#6675) * Trigger Refresh on NRT Engine. * Revert changes made to PublishCheckpointAction in #6366 * Fix failing unit test * Force flush on new elected primary after relocation. * Fix failing unit test. * Remove unnecessary assertions * Adding tests. * Address comments * Fix indentation. --------- (cherry picked from commit 1e5d913) Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…estRelocateWhileContinuouslyIndexingAndWaitingForRefresh (opensearch-project#6637) * Trigger Refresh on NRT Engine. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Revert changes made to PublishCheckpointAction in opensearch-project#6366 Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix failing unit test Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Force flush on new elected primary after relocation. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix failing unit test. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Remove unnecessary assertions Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Adding tests. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Address comments Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix indentation. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> --------- Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Signed-off-by: Mingshi Liu <mingshl@amazon.com>

Trigger Refresh on NRT Engine.

f25361f

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and xuezhou25 as code owners March 11, 2023 23:06

Rishikesh1159 added the skip-changelog label Mar 11, 2023

mch2 mentioned this pull request Mar 11, 2023

[Segment Replication] Fix flaky test testRelocateWhileContinuouslyIndexingAndWaitingForRefresh #6619

Closed

6 tasks

Revert changes made to PublishCheckpointAction in opensearch-project#…

5407a68

…6366 Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Fix failing unit test

a5711a1

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Fix failing unit test.

d4379e3

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Remove unnecessary assertions

4e3272b

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 changed the title ~~[Segment Replication] Trigger Refresh on NRT Engine~~ [Segment Replication] Fix Flaky Test SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh Mar 14, 2023

mch2 reviewed Mar 14, 2023

View reviewed changes

Adding tests.

f7dcfb1

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 mentioned this pull request Mar 14, 2023

[BUG] org.opensearch.indices.replication.SegmentReplicationRelocationIT.testPrimaryRelocation is flaky #6665

Closed

dreamer-89 reviewed Mar 14, 2023

View reviewed changes

Rishikesh1159 and others added 2 commits March 15, 2023 00:18

Address comments

ffd94b1

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Merge branch 'main' into nrt-refresh

f3fa803

dreamer-89 approved these changes Mar 15, 2023

View reviewed changes

dreamer-89 mentioned this pull request Mar 15, 2023

[BUG] SegmentReplicationPressureIT flaky test #6671

Closed

Fix indentation.

fe5e715

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

mch2 approved these changes Mar 15, 2023

View reviewed changes

Rishikesh1159 added the backport 2.x Backport to 2.x branch label Mar 15, 2023

Rishikesh1159 merged commit 1e5d913 into opensearch-project:main Mar 15, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 15, 2023

[Backport 2.x] [Segment Replication] Fix Flaky Test SegmentReplicationRelocationIT #6675

Merged

Rishikesh1159 changed the title ~~[Segment Replication] Fix Flaky Test SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh~~ [Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT Mar 15, 2023

Rishikesh1159 mentioned this pull request Mar 15, 2023

[Segment Replication][BUG] Flaky Test SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh #6531

Closed

Rishikesh1159 mentioned this pull request Mar 19, 2023

[AUTOCUT] Gradle Check Failure on push to main #6613

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT #6637

[Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT #6637

Rishikesh1159 commented Mar 11, 2023 •

edited

Loading

github-actions bot commented Mar 11, 2023

codecov-commenter commented Mar 11, 2023 •

edited

Loading

github-actions bot commented Mar 12, 2023

dreamer-89 commented Mar 12, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 13, 2023

github-actions bot commented Mar 13, 2023

github-actions bot commented Mar 13, 2023

github-actions bot commented Mar 14, 2023

mch2 Mar 14, 2023

dreamer-89 Mar 14, 2023

Rishikesh1159 Mar 14, 2023

mch2 Mar 14, 2023

mch2 Mar 14, 2023

dreamer-89 Mar 14, 2023

dreamer-89 Mar 14, 2023 •

edited

Loading

Rishikesh1159 Mar 14, 2023

Rishikesh1159 Mar 15, 2023

github-actions bot commented Mar 15, 2023

github-actions bot commented Mar 15, 2023

github-actions bot commented Mar 15, 2023

github-actions bot commented Mar 15, 2023

dreamer-89 left a comment

dreamer-89 Mar 15, 2023

github-actions bot commented Mar 15, 2023

		// Verify if all docs are present in replica after flush, if new relocated primary doesn't flush after relocation the below assert
		// will fail

[Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT #6637

[Segment Replication] Fix Flaky in Test SegmentReplicationRelocationIT #6637

Conversation

Rishikesh1159 commented Mar 11, 2023 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Mar 11, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Mar 11, 2023 • edited Loading

Codecov Report

github-actions bot commented Mar 12, 2023

Gradle Check (Jenkins) Run Completed with:

dreamer-89 commented Mar 12, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 13, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 13, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 13, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreamer-89 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 15, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 15, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 15, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 15, 2023

Gradle Check (Jenkins) Run Completed with:

dreamer-89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 15, 2023

Gradle Check (Jenkins) Run Completed with:

Rishikesh1159 commented Mar 11, 2023 •

edited

Loading

codecov-commenter commented Mar 11, 2023 •

edited

Loading

dreamer-89 Mar 14, 2023 •

edited

Loading