Skip to content

Upgrade the mappings on .tasks index if needed - #5377

Closed
pbruski wants to merge 1 commit into
opensearch-project:mainfrom
pbruski:main
Closed

Upgrade the mappings on .tasks index if needed#5377
pbruski wants to merge 1 commit into
opensearch-project:mainfrom
pbruski:main

Conversation

@pbruski

@pbruski pbruski commented Nov 24, 2022

Copy link
Copy Markdown

Signed-off-by: Przemek Bruski pbruskispam@op.pl

Description

Upgrade the mappings on .tasks index if needed. This fixes an regression introduced in #1732

Issues Resolved

#5376

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: Przemek Bruski <pbruskispam@op.pl>
@pbruski
pbruski requested review from a team and reta as code owners November 24, 2022 16:41
@pbruski pbruski changed the title Upgrade the mappings on .tasks index if needed (#5376) Upgrade the mappings on .tasks index if needed Nov 24, 2022
@github-actions

Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

@martin-gaievski

Copy link
Copy Markdown
Member

Is it possible to add test around this scenario so we more confident that it will not break in future releases?

@pbruski

pbruski commented Nov 26, 2022

Copy link
Copy Markdown
Author

Is it possible to add test around this scenario so we more confident that it will not break in future releases?

How I'd do it?

  1. I think the mapping update could be done regardless of the version.
  2. Guarding against a change in mapping without corresponding changes in versions could be done via a commit hook.
  3. Ultimately, the only real solution to this problem is a test suite that does an upgrade of OpenSearch in a baseline version to the current/master version and runs integration tests on it.

@dblock dblock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this is a regression, let's definitely figure out a way to add a test.

You'll also need to fix DCO with git commit --amend -s, thanks!

Comment thread CHANGELOG.md
- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
- Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
- Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299))
- after upgrade to OpenSearch 2.0, finished tasks no longer get stored in .tasks index ([#5376](https://github.com/opensearch-project/OpenSearch/issues/5376))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's make this look like the other lines, please?

"Fixed storing finished tasks in .tasks index"

@dblock

dblock commented Nov 28, 2022

Copy link
Copy Markdown
Member

Is it possible to add test around this scenario so we more confident that it will not break in future releases?

How I'd do it?

  1. I think the mapping update could be done regardless of the version.
  2. Guarding against a change in mapping without corresponding changes in versions could be done via a commit hook.
  3. Ultimately, the only real solution to this problem is a test suite that does an upgrade of OpenSearch in a baseline version to the current/master version and runs integration tests on it.

We have bcw tests exactly for this scenario! I've never written one myself, but if you can't figure it out we'll find someone to help. Start here: https://opensearch.org/blog/technical-post/2021/11/bwc-testing-for-opensearch/

@andrross

Copy link
Copy Markdown
Member

@pbruski Thanks for the contribution here! I've gone ahead and created #5412 which takes your fix here as well as modifies the existing compatibility test to catch these types of problems in the future.

@andrross

Copy link
Copy Markdown
Member

Closing as this has been resolved in #5412

Thanks again @pbruski !

@andrross andrross closed this Nov 30, 2022
@pbruski

pbruski commented Dec 2, 2022

Copy link
Copy Markdown
Author

@andrross Thanks for taking this over - honestly, I have no idea when I'd find the time to dig into the test infra and write that test...

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