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

[Remote Store] Simplify lock manager interface #8399

Conversation

sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Jul 3, 2023

Description

  • Currently, we have following for remote store lock manager (omitting details not in context of lock manager):
public interface RemoteStoreLockManager;

public class RemoteStoreMetadataLockManager implements RemoteStoreLockManager {
        private final RemoteDirectory lockDirectory;
}

public interface RemoteStoreCommitLevelLockManager;

public class RemoteSegmentStoreDirectory implements RemoteStoreCommitLevelLockManager {
        private final RemoteDirectory remoteDataDirectory;
        private final RemoteDirectory remoteMetadataDirectory;
        private final RemoteStoreLockManager mdLockManager;
}
  • This PR tries to simplify this structure in the following:
public interface RemoteStoreLockManager;

public class RemoteSegmentStoreDirectory implements RemoteStoreLockManager {
        private final RemoteDirectory remoteDataDirectory;
        private final RemoteDirectory remoteMetadataDirectory;
        private final RemoteDirectory lockDirectory;
}

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@sachinpkale sachinpkale force-pushed the interop-interface-changes branch from fe7cc08 to c4999b1 Compare July 3, 2023 18:02
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.RestoreSnapshotIT.testRestoreRemoteStoreIndicesWithoutRemoteTranslog
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication

@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (1f3a7a8) 70.91% compared to head (f68d52b) 71.00%.
Report is 847 commits behind head on main.

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

Files Patch % Lines
...earch/index/store/RemoteSegmentStoreDirectory.java 53.33% 10 Missing and 4 partials ⚠️
...in/java/org/opensearch/index/shard/IndexShard.java 50.00% 3 Missing ⚠️
...ensearch/index/store/lockmanager/FileLockInfo.java 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8399      +/-   ##
============================================
+ Coverage     70.91%   71.00%   +0.09%     
- Complexity    56926    56976      +50     
============================================
  Files          4758     4756       -2     
  Lines        269186   269176      -10     
  Branches      39399    39410      +11     
============================================
+ Hits         190896   191139     +243     
+ Misses        62179    61890     -289     
- Partials      16111    16147      +36     

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

@sachinpkale sachinpkale force-pushed the interop-interface-changes branch from 2a07d71 to f68d52b Compare July 4, 2023 08:29
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication

@sachinpkale sachinpkale requested a review from dbwiddis as a code owner July 4, 2023 10:09
@sachinpkale
Copy link
Member Author

Adding more tests to address CodeCov failure.

Sachin Kale added 8 commits July 5, 2023 09:04
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
@sachinpkale sachinpkale force-pushed the interop-interface-changes branch from f68d52b to 03b351d Compare July 5, 2023 04:33
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Aug 4, 2023
@opensearch-trigger-bot
Copy link
Contributor

This PR was closed because it has been stalled for 7 days with no activity.

@kotwanikunal
Copy link
Member

Apologies. This PR was auto closed without reaching a resolution from the maintainers.
Re-opening to move it forward.
Thanks for your contributions to OpenSearch!

@github-actions
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change 03b351d

Incompatible components

Skipped components

Compatible components

Compatible components: []

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Oct 5, 2023
@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Jan 14, 2024
@andrross
Copy link
Member

Looks like this has been abandoned. @sachinpkale Please reopen and rebase if you still want to commit this.

@andrross andrross closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants