Moving RecoveryState.Index to a top-level class and renaming #3075

kartg · 2022-04-25T23:20:17Z

Signed-off-by: Kartik Ganesh gkart@amazon.com

Description

This class is a building block of replication and will be re-used between peer recovery and segment replication. Thus, the inner class has been extracted to a top-level class and moved to the replication.common package. It has been renamed to ReplicationLuceneIndex to better reflect what it represents. It has two dependent inner classes from RecoveryState that have also been moved along with it - these remain inner classes since they are not currently used anywhere else. The RecoveryFilesDetails class has been renamed to FilesDetails and the FileDetail class has been renamed to FileMetadata.

This is a part of the process of merging our feature branch - feature/segment-replication - back into main by re-PRing our changes from the feature branch.
The breakdown of the plan to merge segment-replication to main is detailed in #2355
Segment replication design proposal - #2229

Issues Resolved

[List any issues this PR will resolve]

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

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.

opensearch-ci-bot · 2022-04-25T23:53:20Z

❌ Gradle Check failure 45ed12a
Log 4791

Reports 4791

kartg · 2022-04-26T00:47:33Z

2 flaky failures

testDelayWithALargeAmountOfShards - [BUG] ClusterRerouteIT.testDelayWithALargeAmountOfShards (random test failure) #1561
testFilterCacheStats - [CI] flaky test failure - o.o.indices.stats.IndexStatsIT.testFilterCacheStats #2501

2> REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.indices.stats.IndexStatsIT.testFilterCacheStats" -Dtests.seed=42436B7D286B68CF -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-CL -Dtests.timezone=Europe/Athens -Druntime.java=17
  2> java.lang.AssertionError: shard [index][0] on node [node_s3] has pending operations:
     --> RetentionLeaseBackgroundSyncAction.Request{retentionLeases=RetentionLeases{primaryTerm=1, version=424, leases={peer_recovery/Z1sBF3ItSfeggg1JubKagg=RetentionLease{id='peer_recovery/Z1sBF3ItSfeggg1JubKagg', retainingSequenceNumber=0, timestamp=1650930091233, source='peer recovery'}}}, shardId=[index][0], timeout=1m, index='index', waitForActiveShards=0}
    	at org.opensearch.index.shard.IndexShardOperationPermits.acquire(IndexShardOperationPermits.java:248)
    	at org.opensearch.index.shard.IndexShard.acquirePrimaryOperationPermit(IndexShard.java:3146)
    	at org.opensearch.action.support.replication.TransportReplicationAction.acquirePrimaryOperationPermit(TransportReplicationAction.java:1116)
    	at org.opensearch.action.support.replication.TransportReplicationAction$AsyncPrimaryAction.doRun(TransportReplicationAction.java:433)
    	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:50)
    	at org.opensearch.action.support.replication.TransportReplicationAction.handlePrimaryRequest(TransportReplicationAction.java:377)
    	at org.opensearch.transport.RequestHandlerRegistry.processMessageReceived(RequestHandlerRegistry.java:98)
    	at org.opensearch.transport.TransportService$8.doRun(TransportService.java:937)
    	at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:792)
    	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:50)
    	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    	at java.base/java.lang.Thread.run(Thread.java:833)
        at __randomizedtesting.SeedInfo.seed([42436B7D286B68CF:277C963898636FB6]:0)
        at org.opensearch.test.InternalTestCluster.lambda$assertNoPendingIndexOperations$12(InternalTestCluster.java:1302)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1049)
        at org.opensearch.test.InternalTestCluster.assertNoPendingIndexOperations(InternalTestCluster.java:1289)
        at org.opensearch.test.InternalTestCluster.beforeIndexDeletion(InternalTestCluster.java:1254)
        at org.opensearch.test.OpenSearchIntegTestCase.beforeIndexDeletion(OpenSearchIntegTestCase.java:630)
        at org.opensearch.test.OpenSearchIntegTestCase.afterInternal(OpenSearchIntegTestCase.java:600)
        at org.opensearch.test.OpenSearchIntegTestCase.cleanUpCluster(OpenSearchIntegTestCase.java:2235)
        at jdk.internal.reflect.GeneratedMethodAccessor23.invoke(Unknown Source)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:996)
        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:44)
        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:833)

kartg · 2022-04-26T00:47:46Z

start gradle check

opensearch-ci-bot · 2022-04-26T01:19:19Z

✅ Gradle Check success 45ed12a
Log 4793

Reports 4793

nknize

Looks like a straightforward, nice, rote refactor. I left some change requests for javadocs and making a few classes final and/or private. During these refactors I'd like for us to start marking internal classes accordingly (using @opensearch.internal) and using the final modifier liberally. Since OpenSearch has an API for "external" plugins to extend we need to start putting lines of demarcation around what we allow to be extended vs internal.

server/src/main/java/org/opensearch/indices/replication/common/ReplicationLuceneIndex.java

.idea/inspectionProfiles/Project_Default.xml

server/src/main/java/org/opensearch/indices/replication/common/ReplicationLuceneIndex.java

opensearch-ci-bot · 2022-04-26T16:59:16Z

❌ Gradle Check failure cab9d23
Log 4805

Reports 4805

opensearch-ci-bot · 2022-04-26T17:07:51Z

❌ Gradle Check failure 529b676
Log 4807

Reports 4807

kartg · 2022-04-26T17:41:52Z

org.opensearch.rest.action.cat.RestRecoveryActionTests > testRestRecoveryAction FAILED
    org.mockito.exceptions.base.MockitoException: 
    Cannot mock/spy class org.opensearch.indices.replication.common.ReplicationLuceneIndex
    Mockito cannot mock/spy because :
     - final class
        at __randomizedtesting.SeedInfo.seed([C5B852CFA5D7AA08:DE39A66EA4C701BB]:0)
        at app//org.opensearch.rest.action.cat.RestRecoveryActionTests.testRestRecoveryAction(RestRecoveryActionTests.java:95)

The easier solution is to just remove the final modifier on the class, but I think we can avoid the mocking in the test class by setting up test data. I'll work on this

opensearch-ci-bot · 2022-04-26T21:28:21Z

❌ Gradle Check failure 5f974bb
Log 4815

Reports 4815

kartg · 2022-04-26T21:54:45Z

More mocking that needs fixing.

org.opensearch.indices.recovery.RecoverySourceHandlerTests > testHandleCorruptedIndexOnSendSendFiles FAILED
    org.mockito.exceptions.base.MockitoException: 
    Cannot mock/spy class org.opensearch.indices.replication.common.ReplicationLuceneIndex
    Mockito cannot mock/spy because :
     - final class
        at __randomizedtesting.SeedInfo.seed([BC4C40C00E65B32E:669D16FD4440A01D]:0)
        at app//org.opensearch.indices.recovery.RecoverySourceHandlerTests.testHandleCorruptedIndexOnSendSendFiles(RecoverySourceHandlerTests.java:526)

opensearch-ci-bot · 2022-04-26T22:59:51Z

❌ Gradle Check failure 426bf62
Log 4817

Reports 4817

kartg · 2022-04-27T16:48:54Z

> Task :server:javadoc
/var/jenkins/workspace/OpenSearch_CI/PR_Checks/Gradle_Check/search/server/src/main/java/org/opensearch/indices/replication/common/ReplicationLuceneIndex.java:37: error: unknown tag: opensearch.internal
 * @opensearch.internal
   ^

opensearch-ci-bot · 2022-05-04T00:51:59Z

✅ Gradle Check success 9bec21260568a7eb1e32a97d3645677981971a4f
Log 4978

Reports 4978

This class is a building block of replication and will be re-used between peer recovery and segment replication. Thus, the inner class has been extracted to a top-level class and moved to the replication.common package. It has been renamed to ReplicationLuceneIndex to better reflect what it represents. It has two dependent inner classes from RecoveryState that have also been moved along with it - these remain inner classes since they are not currently used anywhere else. The RecoveryFilesDetails class has been renamed to FilesDetails and the FileDetail class has been renamed to FileMetadata. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Instead, the test now populates dummy data. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

The class has been marked final, so it can no longer be mocked. Instead, the test class sets up the lucene index class by adding the smae file metadata that is set up for the store. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

opensearch-ci-bot · 2022-05-04T22:40:30Z

❌ Gradle Check failure 2077d76
Log 5018

Reports 5018

opensearch-ci-bot · 2022-05-04T23:01:37Z

✅ Gradle Check success 23f8822b6af5786875b4e88f814c9c2c38c10795
Log 5022

Reports 5022

This reverts commit 2077d76. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

opensearch-ci-bot · 2022-05-05T06:32:03Z

✅ Gradle Check success 51b9c81
Log 5031

Reports 5031

Changes incorporated

opensearch-trigger-bot · 2022-07-19T18:49:44Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3075-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 44573baff2b486336b5a133c286af142bc2bdc56
# Push it to GitHub
git push --set-upstream origin backport/backport-3075-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3075-to-2.x.

…n RecoveryState class Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

…top-level class and renaming (#3971) * Backport #3075 to 2.x branch and resolve conflict in RecoveryState class Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Co-authored-by: Kartik Ganesh <gkart@amazon.com>

kartg requested review from a team and reta as code owners April 25, 2022 23:20

kartg mentioned this pull request Apr 25, 2022

[Segment Replication] Merge to main tracking - Refactoring of existing classes to support segrep. #2926

Closed

5 tasks

nknize previously requested changes Apr 26, 2022

View reviewed changes

kartg requested a review from nknize May 4, 2022 00:58

kotwanikunal approved these changes May 4, 2022

View reviewed changes

kartg added 6 commits May 4, 2022 15:10

Incorporate PR comments

db9976e

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Revert Project_Default.xml

253ad37

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Update REST Action test to no longer mock a final class

2d737c0

Instead, the test now populates dummy data. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

Configure @opensearch.internal as custom Javadoc tag

2077d76

Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg force-pushed the segrep-merge-to-main branch from 9bec212 to 2077d76 Compare May 4, 2022 22:10

Revert "Configure @opensearch.internal as custom Javadoc tag"

51b9c81

This reverts commit 2077d76. Signed-off-by: Kartik Ganesh <gkart@amazon.com>

kartg force-pushed the segrep-merge-to-main branch from 23f8822 to 51b9c81 Compare May 5, 2022 05:59

kartg merged commit 44573ba into opensearch-project:main May 5, 2022

Rishikesh1159 added the backport 2.x Backport to 2.x branch label Jul 19, 2022

Rishikesh1159 pushed a commit to Rishikesh1159/OpenSearch that referenced this pull request Jul 21, 2022

Backport opensearch-project#3075 to 2.x branch and resolve conflict i…

f3d9799

…n RecoveryState class Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 mentioned this pull request Jul 21, 2022

[Backport 2.x] [Segment Replication] Moving RecoveryState.Index to a top-level class and renaming #3971

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving RecoveryState.Index to a top-level class and renaming #3075

Moving RecoveryState.Index to a top-level class and renaming #3075

kartg commented Apr 25, 2022 •

edited

Loading

opensearch-ci-bot commented Apr 25, 2022

kartg commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

nknize left a comment •

edited

Loading

opensearch-ci-bot commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 27, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 5, 2022

opensearch-trigger-bot bot commented Jul 19, 2022

Moving RecoveryState.Index to a top-level class and renaming #3075

Moving RecoveryState.Index to a top-level class and renaming #3075

Conversation

kartg commented Apr 25, 2022 • edited Loading

Description

Issues Resolved

Check List

opensearch-ci-bot commented Apr 25, 2022

kartg commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

nknize left a comment • edited Loading

Choose a reason for hiding this comment

opensearch-ci-bot commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 26, 2022

opensearch-ci-bot commented Apr 26, 2022

kartg commented Apr 27, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 5, 2022

opensearch-trigger-bot bot commented Jul 19, 2022

kartg commented Apr 25, 2022 •

edited

Loading

nknize left a comment •

edited

Loading