Skip to content

Defer repo ops in searchable snapshot restore#54211

Merged
DaveCTurner merged 7 commits intoelastic:feature/searchable-snapshotsfrom
DaveCTurner:2020-03-25-reinstate-threadpool-assertion
Mar 31, 2020
Merged

Defer repo ops in searchable snapshot restore#54211
DaveCTurner merged 7 commits intoelastic:feature/searchable-snapshotsfrom
DaveCTurner:2020-03-25-reinstate-threadpool-assertion

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

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/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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 <tlrx.dev@gmail.com>
@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/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