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

Add extension point for custom TranslogDeletionPolicy in EnginePlugin. #1404

Merged
merged 4 commits into from
Oct 22, 2021
Merged

Add extension point for custom TranslogDeletionPolicy in EnginePlugin. #1404

merged 4 commits into from
Oct 22, 2021

Conversation

adnapibar
Copy link
Contributor

@adnapibar adnapibar commented Oct 21, 2021

Description

This change adds a method that can be used to provide a custom TranslogDeletionPolicy. It also removes the deprecated settings for translog pruning based on retention leases. The removal of deprecated settings and CCR specific logic will be done as part of a separate PR.

Issues Resolved

Resolves #1260

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.

Signed-off-by: Rabi Panda adnapibar@gmail.com

This change adds a method that can be used to provide a custom TranslogDeletionPolicy. It also removes the deprecated settings for translog pruning based on retention leases.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
@adnapibar adnapibar requested a review from nknize October 21, 2021 07:17
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success d521adf

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed d521adf

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success d521adf

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

This is looking good. Just a few important things:

  1. We need to retain "read only" bwc support for the CCR setting until OpenSearch 3.0
  2. Let's keep this PR simple to just adding the TranslogDeletionPolicy extension point and address retention lease setting removal in a separate PR

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: Rabi Panda <adnapibar@gmail.com>
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 912504d

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 912504d

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 912504d

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

nice!

@nknize
Copy link
Collaborator

nknize commented Oct 21, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 912504d
Log 772

Reports 772

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 1c39d8c

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 1c39d8c

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 1c39d8c

@adnapibar
Copy link
Contributor Author

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 1c39d8c
Log 775

Reports 775

Copy link

@krishna-ggk krishna-ggk left a comment

Choose a reason for hiding this comment

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

Took a first pass, LGTM and thanks for the tests :)

Will take another pass with all other related PRs.

@nknize nknize merged commit 2ebd0e0 into opensearch-project:main Oct 22, 2021
@nknize nknize added pending backport Identifies an issue or PR that still needs to be backported distributed framework labels Oct 22, 2021
@nknize nknize added enhancement Enhancement or improvement to existing feature or request v1.2.0 Issues related to version 1.2.0 v2.0.0 Version 2.0.0 labels Oct 22, 2021
@nknize
Copy link
Collaborator

nknize commented Oct 22, 2021

Merged as is to let this bake in random testing. Let's iterate any changes, additions, bugs in separate PRs.

nknize pushed a commit that referenced this pull request Oct 26, 2021
#1404)

This commit adds a method that can be used to provide a custom TranslogDeletionPolicy
from within plugins that implement the EnginePlugin interface. This enables plugins to
provide a custom deletion policy with the current limitation that only one plugin can
override the policy. An exception will be thrown if more than one plugin overrides the
policy.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
adnapibar added a commit that referenced this pull request Oct 29, 2021
…in EnginePlugin (#1404) (#1424)

* Add extension point for custom TranslogDeletionPolicy in EnginePlugin. (#1404)

This commit adds a method that can be used to provide a custom TranslogDeletionPolicy from within plugins that implement the EnginePlugin interface. This enables plugins to provide a custom deletion policy with the current limitation that only one plugin can override the policy. An exception will be thrown if more than one plugin overrides the policy.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Close first engine instance before creating second (#1457)

When creating the second instance of an InternalEngine using the same translog config of the default InternalEngine instance, the second instance will attempt to delete all the existing translog files. I found
a deterministic test failure when running with the seed `E3E6AAD95ABD299B`. As opposed to creating a second engine instance with a different translog location, just close the first one before creating the second.

Signed-off-by: Andrew Ross <andrross@amazon.com>

Co-authored-by: Andrew Ross <andrross@amazon.com>
saikaranam-amazon pushed a commit to opensearch-project/cross-cluster-replication that referenced this pull request Nov 3, 2021
… retention leases (#209)

* Provide a custom TranslogDeletionPolicy for translog pruning based on retention leases.

This commit overrides the new getCustomTranslogDeletionPolicyFactory method of the EnginePlugin to provide a custom TranslogDeletionPolicy. The extension is added as part of OpenSearch 1.2 (opensearch-project/OpenSearch#1404).

Signed-off-by: Rabi Panda <pandarab@amazon.com>
@dblock dblock removed the pending backport Identifies an issue or PR that still needs to be backported label Dec 6, 2021
@adnapibar adnapibar deleted the translog-deletion-plugin branch November 15, 2022 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed framework enhancement Enhancement or improvement to existing feature or request v1.2.0 Issues related to version 1.2.0 v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pluggable extension point for TranslogDeletionPolicy and move CCR specific logic to plugin.
7 participants