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

Fixed a bug to prevent updating index.knn setting after index creation #2348

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

anntians
Copy link
Contributor

@anntians anntians commented Dec 21, 2024

Description

This change prevents customers to update index.knn setting after creating an index. Previously, there was a loophole to migrate a live k-nn index from hot to UW by closing the index, setting index.knn to false, opening the index, and migrating to UW. However, migrating k-nn indices to UW nodes is not supported. Thus, this PR updates index.knn to be FINAL and immutable after index creation.

Related Issues

Resolves #2334

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

@anntians anntians marked this pull request as ready for review December 21, 2024 01:33
Copy link
Member

@kotwanikunal kotwanikunal left a comment

Choose a reason for hiding this comment

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

LGTM.

@kotwanikunal
Copy link
Member

@shatejas / @navneet1v / @Vikasht34 - Can you please review this one?

@navneet1v navneet1v merged commit a875eb8 into opensearch-project:main Jan 9, 2025
60 checks passed
@navneet1v navneet1v added backport 2.x Bug Fixes Changes to a system or product designed to handle a programming bug/glitch labels Jan 9, 2025
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

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

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

@navneet1v
Copy link
Collaborator

@anntians can you please port this change for 2.x branch and create a new PR

anntians added a commit to anntians/k-NN that referenced this pull request Jan 10, 2025
opensearch-project#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
@anntians
Copy link
Contributor Author

@anntians can you please port this change for 2.x branch and create a new PR

Sure thing, linked is the backport PR: #2379

anntians added a commit to anntians/k-NN that referenced this pull request Jan 10, 2025
opensearch-project#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
Signed-off-by: AnnTian Shao <[email protected]>
anntians added a commit to anntians/k-NN that referenced this pull request Jan 10, 2025
opensearch-project#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
anntians added a commit to anntians/k-NN that referenced this pull request Jan 10, 2025
opensearch-project#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
anntians added a commit to anntians/k-NN that referenced this pull request Jan 10, 2025
opensearch-project#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
Signed-off-by: AnnTian Shao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Bug Fixes Changes to a system or product designed to handle a programming bug/glitch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]Block updation of knn.enabled flag.
5 participants