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 1.x compatibility bug with stored Tasks #5412

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

andrross
Copy link
Member

When the new 'cancelled' field was introduced it was a miss not to increment the version number on the mapping definitions for the .tasks index. This commit fixes that oversight, as well as modifies the existing backward compatiblity test to ensure that it will catch future mistakes like this one.

Closes #5376

Signed-off-by: Andrew Ross [email protected]

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.

When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <[email protected]>
@andrross andrross changed the title Fix 1.x compatiblity bug with stored Tasks Fix 1.x compatibility bug with stored Tasks Nov 30, 2022
@andrross andrross marked this pull request as ready for review November 30, 2022 18:57
@andrross andrross requested review from a team and reta as code owners November 30, 2022 18:57
*/
private static void createAndVerifyStoredTask() throws Exception {
// Use update by query to create an async task
final Request updateByQueryRequest = new Request("POST", "/test_index_old/_update_by_query");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, _reindex was replaced with _update_by_query, is there some particular reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it is done twice, once for the old cluster and again for the updated cluster. Reindex creates a new index so you can't make the exact same request twice (unless you closed or deleted the new index). Update by query is simpler because it takes no parameters and doesn't create any new resources :)

Copy link
Member Author

Choose a reason for hiding this comment

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

(Previously this step was only being done on the old cluster, so there was no problem. My change does this test for both the old and updated clusters.)

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.index.IndexServiceTests.testAsyncTranslogTrimTaskOnClosedIndex

Copy link
Member

@dreamer-89 dreamer-89 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @andrross for this quick fix.

@dreamer-89
Copy link
Member

      1 org.opensearch.index.IndexServiceTests.testAsyncTranslogTrimTaskOnClosedIndex

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.index.IndexServiceTests.testAsyncTranslogTrimTaskOnClosedIndex

Created #5414 to track this.

@andrross andrross merged commit 4616dfa into opensearch-project:main Nov 30, 2022
@andrross andrross added the backport 2.x Backport to 2.x branch label Nov 30, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

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

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
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-5412-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4616dfac5382062a5149c799554dda4ff601369f
# Push it to GitHub
git push --set-upstream origin backport/backport-5412-to-2.x
# Go back to the original working tree
popd
# 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-5412-to-2.x.

andrross added a commit to andrross/OpenSearch that referenced this pull request Nov 30, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <[email protected]>
andrross added a commit to andrross/OpenSearch that referenced this pull request Nov 30, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <[email protected]>
andrross added a commit that referenced this pull request Nov 30, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes #5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <[email protected]>
@dblock
Copy link
Member

dblock commented Dec 1, 2022

👏

@andrross andrross added the backport 2.4 Backport to 2.4 branch label Dec 3, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.4 failed:

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

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.4 2.4
# Navigate to the new working tree
pushd ../.worktrees/backport-2.4
# Create a new branch
git switch --create backport/backport-5412-to-2.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4616dfac5382062a5149c799554dda4ff601369f
# Push it to GitHub
git push --set-upstream origin backport/backport-5412-to-2.4
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.4

Then, create a pull request where the base branch is 2.4 and the compare/head branch is backport/backport-5412-to-2.4.

andrross added a commit to andrross/OpenSearch that referenced this pull request Dec 3, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
andrross added a commit to andrross/OpenSearch that referenced this pull request Dec 3, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <[email protected]>
andrross added a commit that referenced this pull request Dec 3, 2022
When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes #5376

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <[email protected]>

Signed-off-by: Andrew Ross <[email protected]>
@andrross andrross deleted the task-mapping-fix branch December 16, 2022 18:35
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 backport 2.4 Backport to 2.4 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] after upgrade to OpenSearch 2.0, finished tasks no longer get stored in .tasks index
4 participants