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.

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 RemoteStoreRefreshListenerTests testTrackerData #11256 #11803

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

ashking94
Copy link
Member

@ashking94 ashking94 commented Jan 8, 2024

Description

This closes #11256. The flakiness in the RemoteStoreRefreshListenerTests.testTrackerData test is happening due to RemoteSegmentStoreDirectory.init() method being called on account of simultaneous refreshes from test. In the test setup, we create a full fledged IndexShard and use it's remote store classes for mimicing/mocking certain test behaviours. Now, the refreshes of the underlying IndexShard and the manually invoked test's afterRefreshes were happening simultaneously causing the shared RemoteSegmentStoreDirectory object's segmentsUploadedToRemoteStore map to get cleared. The changes now ensures that the concurrency does not happen any more.

Related Issues

Resolves #11256

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

@ashking94
Copy link
Member Author

@opensearch-project/opensearch-core Please review the fix for flaky test. It is just a test fix. Since the refresh happens in background. This particular case was happening due to refresh not yet successful while the assertion kicked in.

I am running this test with fix on loop overnight. I will confirm once the test has passed for over 10K iterations and then you merge this PR.

Copy link
Contributor

github-actions bot commented Jan 8, 2024

Compatibility status:

Checks if related components are compatible with change 16cec03

Incompatible components

Incompatible components: [https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/k-nn.git]

Copy link
Contributor

github-actions bot commented Jan 8, 2024

❕ Gradle check result for 4aa121d: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteIndexPrimaryRelocationIT.testPrimaryRelocationWhileIndexing

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c132db9) 71.33% compared to head (4aa121d) 71.35%.

❗ Current head 4aa121d differs from pull request most recent head 16cec03. Consider uploading reports for the commit 16cec03 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #11803      +/-   ##
============================================
+ Coverage     71.33%   71.35%   +0.01%     
+ Complexity    59300    59271      -29     
============================================
  Files          4921     4911      -10     
  Lines        278989   278667     -322     
  Branches      40543    40528      -15     
============================================
- Hits         199014   198836     -178     
+ Misses        63444    63310     -134     
+ Partials      16531    16521      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ashking94
Copy link
Member Author

Saw the test failing at 965th iteration with below assertion failure stack trace -

java.lang.AssertionError: expected:<3> but was:<2>
	at __randomizedtesting.SeedInfo.seed([B6586DD698EE379F:F50CE868C1E60D9D]:0)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.opensearch.index.shard.RemoteStoreRefreshListenerTests.assertNoLag(RemoteStoreRefreshListenerTests.java:437)
	at org.opensearch.index.shard.RemoteStoreRefreshListenerTests.lambda$testTrackerData$12(RemoteStoreRefreshListenerTests.java:421)
	at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1089)
	at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1062)
	at org.opensearch.index.shard.RemoteStoreRefreshListenerTests.testTrackerData(RemoteStoreRefreshListenerTests.java:421)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:938)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:974)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:988)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.apache.lucene.tests.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:48)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at org.apache.lucene.tests.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
	at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
	at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:817)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:468)
	at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:947)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:832)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:883)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:894)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.apache.lucene.tests.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
	at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
	at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.apache.lucene.tests.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
	at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
	at org.apache.lucene.tests.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
	at java.base/java.lang.Thread.run(Thread.java:1583)
	Suppressed: java.lang.AssertionError: expected:<2> but was:<1>
		at org.junit.Assert.fail(Assert.java:89)
		at org.junit.Assert.failNotEquals(Assert.java:835)
		at org.junit.Assert.assertEquals(Assert.java:647)
		at org.junit.Assert.assertEquals(Assert.java:633)
		at org.opensearch.index.shard.RemoteStoreRefreshListenerTests.assertNoLag(RemoteStoreRefreshListenerTests.java:437)
		at org.opensearch.index.shard.RemoteStoreRefreshListenerTests.lambda$testTrackerData$12(RemoteStoreRefreshListenerTests.java:421)
		at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1077)
		... 40 more
	.. (atleast 10 times more)

The reason for this is due to exception while uploading metadata for the refresh -

[2024-01-08T19:57:21,329][INFO ][o.o.i.t.RemoteFsTranslog ] [testTrackerData] [index][0] Downloaded data from remote translog till maxSeqNo = -1
[2024-01-08T19:57:21,392][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=RECOVERING engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,393][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=RECOVERING engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,393][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=RECOVERING engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,394][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=RECOVERING engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,419][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=STARTED engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,420][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Skipped syncing segments with primaryMode=false indexShardState=STARTED engineType=InternalEngine recoverySourceType=EMPTY_STORE primary=true
[2024-01-08T19:57:21,423][INFO ][o.o.i.s.RemoteStoreRefreshListener] [testTrackerData] [index][0] Scheduled retry with didRefresh=true
[2024-01-08T19:57:22,384][WARN ][o.o.i.s.RemoteStoreRefreshListener] [org.opensearch.index.shard.RemoteStoreRefreshListenerTests] [index][0] Exception in post new segment upload actions
java.nio.file.NoSuchFileException: segments_3
	at org.opensearch.index.store.RemoteSegmentStoreDirectory.uploadMetadata(RemoteSegmentStoreDirectory.java:617) ~[main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener.uploadMetadata(RemoteStoreRefreshListener.java:363) ~[main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener$1.onResponse(RemoteStoreRefreshListener.java:226) [main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener$1.onResponse(RemoteStoreRefreshListener.java:220) [main/:?]
	at org.opensearch.action.LatchedActionListener.onResponse(LatchedActionListener.java:58) [main/:?]
	at org.opensearch.core.action.ActionListener$4.onResponse(ActionListener.java:182) [opensearch-core-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
	at org.opensearch.action.support.GroupedActionListener.onResponse(GroupedActionListener.java:81) [main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener.lambda$uploadNewSegments$3(RemoteStoreRefreshListener.java:391) [main/:?]
	at org.opensearch.core.action.ActionListener$1.onResponse(ActionListener.java:82) [opensearch-core-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
	at org.opensearch.index.store.RemoteSegmentStoreDirectory.copyFrom(RemoteSegmentStoreDirectory.java:466) [main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener.uploadNewSegments(RemoteStoreRefreshListener.java:401) [main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener.syncSegments(RemoteStoreRefreshListener.java:254) [main/:?]
	at org.opensearch.index.shard.RemoteStoreRefreshListener.performAfterRefreshWithPermit(RemoteStoreRefreshListener.java:152) [main/:?]
	at org.opensearch.index.shard.ReleasableRetryableRefreshListener.runAfterRefreshWithPermit(ReleasableRetryableRefreshListener.java:160) [main/:?]
	at org.opensearch.index.shard.ReleasableRetryableRefreshListener.lambda$scheduleRetry$2(ReleasableRetryableRefreshListener.java:126) [main/:?]
	at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:852) [main/:3.0.0-SNAPSHOT]
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) [?:?]
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) [?:?]
	at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
[2024-01-08T19:57:22,394][INFO ][o.o.i.s.RemoteStoreRefreshListener] [org.opensearch.index.shard.RemoteStoreRefreshListenerTests] [index][0] Scheduled retry with didRefresh=true

Copy link
Contributor

❌ Gradle check result for 73fac30: FAILURE

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?

@ashking94
Copy link
Member Author

❌ Gradle check result for 73fac30: FAILURE

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?

org.opensearch.qa.verify_version_constants.VerifyVersionConstantsIT.testLuceneVersionConstant
The failure is mostly due to Lucene version upgrade.

Copy link
Contributor

❌ Gradle check result for 16cec03: FAILURE

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?

@ashking94
Copy link
Member Author

❌ Gradle check result for 16cec03: FAILURE

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?

Flaky tests -

@ashking94 ashking94 changed the title Fix flaky test #11256 Fix flaky RemoteStoreRefreshListenerTests testTrackerData #11256 Jan 16, 2024
@ashking94
Copy link
Member Author

ashking94 commented Jan 16, 2024

Have run the test for 2K+ iterations and no failures have occurred so far.
Update - Have run for 5K iterations and seen no failure. Good to be reviewed and merged.

Copy link
Contributor

❕ Gradle check result for 16cec03: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteIndexPrimaryRelocationIT.testPrimaryRelocationWhileIndexing

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@ashking94
Copy link
Member Author

testPrimaryRelocationWhileIndexing

Flaky test - #9191

@gbbafna gbbafna merged commit 517f091 into opensearch-project:main Jan 17, 2024
37 checks passed
@gbbafna gbbafna added the backport 2.x Backport to 2.x branch label Jan 17, 2024
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 17, 2024
Signed-off-by: Ashish Singh <[email protected]>
(cherry picked from commit 517f091)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
sachinpkale pushed a commit to sachinpkale/OpenSearch that referenced this pull request Feb 5, 2024
sachinpkale pushed a commit that referenced this pull request Feb 5, 2024
(cherry picked from commit 517f091)

Signed-off-by: Ashish Singh <[email protected]>
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>
peteralfonsi pushed a commit to peteralfonsi/OpenSearch that referenced this pull request Mar 1, 2024
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch bug Something isn't working flaky-test Random test failure that succeeds on second run skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] org.opensearch.index.shard.RemoteStoreRefreshListenerTests.testTrackerData is flaky
3 participants