Skip to content

Defer repo ops in searchable snapshot restore #54211

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

Conversation

DaveCTurner
Copy link
Contributor

A searchable snapshots Directory requires a BlobContainer and a
BlobStoreIndexShardSnapshot, which require us to read some data from the
repository. Today we construct these objects on the cluster applier thread,
blocking that thread on remote operations.

This commit defers their construction until the restore process starts, so that
they can happen on a more appropriate thread. It also reinstates the assertion
that constructing the blob container only occurs on the snapshot or generic
threadpool, and adds an assertion that blobs are only accessed on snapshot or
generic or search threads too.

A searchable snapshots `Directory` requires a `BlobContainer` and a
`BlobStoreIndexShardSnapshot`, which require us to read some data from the
repository. Today we construct these objects on the cluster applier thread,
blocking that thread on remote operations.

This commit defers their construction until the restore process starts, so that
they can happen on a more appropriate thread. It also reinstates the assertion
that constructing the blob container only occurs on the snapshot or generic
threadpool, and adds an assertion that blobs are only accessed on snapshot or
generic or search threads too.
@DaveCTurner DaveCTurner added >enhancement :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Mar 25, 2020
@DaveCTurner DaveCTurner requested a review from tlrx March 25, 2020 17:05
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@DaveCTurner DaveCTurner removed the WIP label Mar 31, 2020
@DaveCTurner DaveCTurner requested a review from tlrx March 31, 2020 11:19
Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM


// Today processExistingRecoveries considers all shards and constructs a shard store snapshot on this thread, this needs
// addressing. TODO NORELEASE
|| threadName.contains('[' + ThreadPool.Names.FETCH_SHARD_STORE + ']')
Copy link
Member

Choose a reason for hiding this comment

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

👍

…arch/index/store/SearchableSnapshotDirectory.java

Co-Authored-By: Tanguy Leroux <[email protected]>
@DaveCTurner DaveCTurner merged commit 773e7e3 into elastic:feature/searchable-snapshots Mar 31, 2020
@DaveCTurner DaveCTurner deleted the 2020-03-25-reinstate-threadpool-assertion branch March 31, 2020 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants