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 store type settings ordering for searchable snapshots #7297

Merged

Conversation

kotwanikunal
Copy link
Member

@kotwanikunal kotwanikunal commented Apr 24, 2023

Description

  • Restoring searchable snapshots with indices backed up with the setting index.store.type leads to wrong ordering for store type settings, causing the restore to be of the original configuration set on the index at backup which can be fs / a local restored index instead. (Sample code which does that with benchmarks here)
  • This PR fixes the ordering of the settings to enable searchable snapshot index restore with the appropriate store type, irrespective of the original store type configuration for the index.

Issues Resolved

  • N/A

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.

@kotwanikunal kotwanikunal changed the title Fix storage type settings ordering for searchable snapshots Fix store type settings ordering for searchable snapshots Apr 24, 2023
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
@@ -334,6 +342,7 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo
Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, Integer.toString(numReplicasIndex))
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1")
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey())
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this setting for ensuring store type is set to the correct value at restore.
Verified that assertRemoteSnapshotIndexSettings fails if the ordering within RestoreService is not flipped.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023

Codecov Report

Merging #7297 (7421127) into main (785a67a) will increase coverage by 0.00%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##               main    #7297   +/-   ##
=========================================
  Coverage     70.65%   70.66%           
- Complexity    59515    59545   +30     
=========================================
  Files          4862     4862           
  Lines        285544   285544           
  Branches      41153    41153           
=========================================
+ Hits         201749   201774   +25     
- Misses        67182    67205   +23     
+ Partials      16613    16565   -48     
Impacted Files Coverage Δ
.../java/org/opensearch/snapshots/RestoreService.java 56.28% <0.00%> (ø)

... and 511 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

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

@andrross andrross merged commit 72ed13b into opensearch-project:main Apr 25, 2023
@andrross
Copy link
Member

Flaky test documented here: #6124

opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 25, 2023
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 72ed13b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 25, 2023
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 72ed13b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Apr 25, 2023
… (#7298)

(cherry picked from commit 72ed13b)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Apr 25, 2023
… (#7299)

(cherry picked from commit 72ed13b)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
austintlee pushed a commit to austintlee/OpenSearch that referenced this pull request Apr 28, 2023
@kotwanikunal kotwanikunal deleted the fix-index-setting-bug branch May 12, 2023 19:51
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…search-project#7297)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants