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

[Backport 2.x] [Point in time] Backport point in time changes #4616

Merged
merged 14 commits into from
Oct 6, 2022

Conversation

bharath-techie
Copy link
Contributor

Description

Backport point in time changes

Issues Resolved

#1147

Original PRs

Adding create pit service layer changes #3921
Adding delete pit service layer changes #3949
Add service layer changes for list all PITs #4016
Add Point In Time Node Stats API ServiceLayer Changes #4030
Add changes to Point in time segments API service layer #4105
Add changes for Create PIT and Delete PIT rest layer and rest high level client #4064
Added RestLayer Changes for PIT stats #4217
Added rest layer changes for List all PITs and PIT segments #4388

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: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
…vel client

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
@bharath-techie bharath-techie requested review from a team and reta as code owners September 28, 2022 11:52
/**
* Create point in time for one or more indices
*/
void createPit(CreatePitRequest createPITRequest, ActionListener<CreatePitResponse> listener);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Posting @reta's comment here - #4406 (comment)

@bharath-techie from the API perspective, this is beaking change. Since we backport to 2.x (no breaking changes), we probably should mark this methods as default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reta by marking default we can remove implementations in case of roll back ? is that the idea ?

@bharath-techie
Copy link
Contributor Author

@reta @Bukhtawar please review.

Closed the old backport PR because it had issues like DCO. So made this fresh PR.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
if (in.getVersion().onOrAfter(Version.V_2_3_0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the only place we are doing serde and relying on versioning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And in rest high level tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Bukhtawar Bukhtawar merged commit 2c0b4e7 into opensearch-project:2.x Oct 6, 2022
reta added a commit to reta/OpenSearch that referenced this pull request Oct 6, 2022
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
reta added a commit to reta/OpenSearch that referenced this pull request Oct 6, 2022
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
reta added a commit to reta/OpenSearch that referenced this pull request Oct 6, 2022
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
reta added a commit to reta/OpenSearch that referenced this pull request Oct 6, 2022
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
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.

Documenting for posterity: this change caused bwc failures because the version check wasn't set correctly in the original PR.

For future reference (and maybe we need to make sure this is documented in CONRTIBUTING.md) make sure the version checks are set to the intended target version in the initial PR. BWC tests will temporarily fail after the initial merge to main but only until the backport PRs are merged. If you wait, an additional follow up PR will be needed (which could be forgotten as it was here) whereas if you target the correct version in the original PR it will rectify itself on backport.

reta added a commit to reta/OpenSearch that referenced this pull request Oct 6, 2022
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants