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

[Backport 2.x] [Searchable Snapshots] Remove virtual file and fix duplicate clone #6347

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 1cdff3b from #6345.

A "virtual file" in this context is a small file whose entire contents
is stored in the snapshot metadata as opposed to in a discrete file in
the remote repository. OnDemandVirtualFileSnapshotIndexInput was a
complicated wrapper that ultimately returned a ByteArrayIndexInput
wrapping the file contents pulled from the metadata data. This change
simplifies things a lot and just creates the ByteArrayIndexInput
directly.

The other change (which led to removal of the virtual file) is to remove
a duplicate clone() call of the index input. The file cache design calls
for keeping the "origin" IndexInput instance in the cache and always
[returning clones][1]. OnDemandBlockIndexInput was incorrectly
duplicating this clone operation.

[1]: https://github.com/opensearch-project/OpenSearch/blob/0ca51a774211184835c4825dfeff38b23198352e/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java#L105

Signed-off-by: Andrew Ross <[email protected]>
(cherry picked from commit 1cdff3b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #6347 (790c682) into 2.x (c1c26e4) will increase coverage by 0.10%.
The diff coverage is 50.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              @@
##                2.x    #6347      +/-   ##
============================================
+ Coverage     70.37%   70.47%   +0.10%     
- Complexity    59200    59246      +46     
============================================
  Files          4796     4795       -1     
  Lines        284156   284124      -32     
  Branches      41312    41311       -1     
============================================
+ Hits         199965   200233     +268     
+ Misses        67492    67209     -283     
+ Partials      16699    16682      -17     
Impacted Files Coverage Δ
...tore/remote/directory/RemoteSnapshotDirectory.java 3.12% <0.00%> (ø)
...arch/index/store/remote/utils/TransferManager.java 3.03% <ø> (ø)
...dex/store/remote/file/OnDemandBlockIndexInput.java 73.01% <100.00%> (ø)
.../java/org/opensearch/node/NodeClosedException.java 50.00% <0.00%> (-50.00%) ⬇️
...opensearch/persistent/PersistentTasksExecutor.java 22.22% <0.00%> (-44.45%) ⬇️
...regations/metrics/AbstractHyperLogLogPlusPlus.java 51.72% <0.00%> (-37.94%) ⬇️
...opensearch/index/reindex/BulkByScrollResponse.java 48.38% <0.00%> (-37.91%) ⬇️
...ensearch/client/indices/DetailAnalyzeResponse.java 20.54% <0.00%> (-34.25%) ⬇️
...min/cluster/snapshots/get/GetSnapshotsRequest.java 52.63% <0.00%> (-31.58%) ⬇️
...opensearch/snapshots/SnapshotRestoreException.java 25.00% <0.00%> (-25.00%) ⬇️
... and 491 more

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

@andrross andrross merged commit 5a76075 into 2.x Feb 16, 2023
@andrross andrross deleted the backport/backport-6345-to-2.x branch February 16, 2023 22:19
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.

2 participants