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

[Remote Store] Create empty lucene index during restore if remote segment store is empty #6375

Merged
merged 1 commit into from
Feb 20, 2023

Conversation

sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Feb 20, 2023

Description

  • Currently, restoring an index with no data in remote segment store fails.
  • Ideally, restore should create an empty index if there is no data in remote segment store.

Issues Resolved

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: Sachin Kale <kalsac@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #6375 (a9db818) into main (5b3b557) will increase coverage by 0.08%.
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    #6375      +/-   ##
============================================
+ Coverage     70.67%   70.76%   +0.08%     
- Complexity    58958    59039      +81     
============================================
  Files          4799     4799              
  Lines        282432   282434       +2     
  Branches      40716    40717       +1     
============================================
+ Hits         199622   199861     +239     
+ Misses        66428    66190     -238     
- Partials      16382    16383       +1     
Impacted Files Coverage Δ
...java/org/opensearch/index/shard/StoreRecovery.java 65.95% <0.00%> (-0.41%) ⬇️
...ava/org/opensearch/action/NoSuchNodeException.java 0.00% <0.00%> (-50.00%) ⬇️
...java/org/opensearch/threadpool/ThreadPoolInfo.java 56.25% <0.00%> (-37.50%) ⬇️
...pensearch/action/ingest/DeletePipelineRequest.java 31.25% <0.00%> (-37.50%) ⬇️
...r/src/main/java/org/opensearch/http/HttpUtils.java 33.33% <0.00%> (-33.34%) ⬇️
...nsearch/index/shard/IndexShardClosedException.java 66.66% <0.00%> (-33.34%) ⬇️
...rg/opensearch/index/shard/ShardSplittingQuery.java 57.25% <0.00%> (-20.62%) ⬇️
...pensearch/index/shard/SearchOperationListener.java 72.72% <0.00%> (-20.00%) ⬇️
...uery/functionscore/LinearDecayFunctionBuilder.java 32.00% <0.00%> (-20.00%) ⬇️
...ndex/query/functionscore/DecayFunctionBuilder.java 46.18% <0.00%> (-19.64%) ⬇️
... and 443 more

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

@sachinpkale
Copy link
Member Author

Running on local for a couple of hundred times before opening the PR for review.

@@ -457,6 +457,9 @@ private void recoverFromRemoteStore(IndexShard indexShard, Repository repository
// Download segments from remote segment store
indexShard.syncSegmentsFromRemoteSegmentStore(true);

if (store.directory().listAll().length == 0) {
store.createEmpty(indexShard.indexSettings().getIndexVersionCreated().luceneVersion);
Copy link
Member

@ashking94 ashking94 Feb 20, 2023

Choose a reason for hiding this comment

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

[Question] Just wondering since the creation of empty directory depends on the lucene version, do we need to handle anything for version upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should not change anything in the version compatibility as the index already exists. I think the confusion is due to PR title. We are not creating a new OpenSearch index but empty lucene index :). Let me change the title.

Copy link
Member

@ashking94 ashking94 left a comment

Choose a reason for hiding this comment

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

LGTM

@sachinpkale sachinpkale changed the title [Remote Store] Create empty index during restore if remote segment store is empty [Remote Store] Create empty lucene index during restore if remote segment store is empty Feb 20, 2023
@gbbafna gbbafna merged commit 1904158 into opensearch-project:main Feb 20, 2023
@gbbafna gbbafna added the backport 2.x Backport to 2.x branch label Feb 20, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 20, 2023
Signed-off-by: Sachin Kale <kalsac@amazon.com>
(cherry picked from commit 1904158)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gbbafna pushed a commit that referenced this pull request Feb 20, 2023
(cherry picked from commit 1904158)

Signed-off-by: Sachin Kale <kalsac@amazon.com>
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 skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants