Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Jun 17, 2021

Today a searchable snapshot index inherits the index.shard.check_on_startup index setting from the snapshot it is mounted from. But this setting can require some expensive processing as documented in #74233, so we decided in #73147 to default the value of this setting to false but allow the user to override this in the mount request.

Closes #73147

Note to reviewer: I don't see this as a breaking change neither I think it should deserve a mention in the Mount API as the setting is "expert only", so I did not update documentation. Let me know if you think I should.

@tlrx tlrx added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.14.0 labels Jun 17, 2021
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Jun 17, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Thanks Tanguy, I left a couple of comments on the tests.

@tlrx tlrx requested a review from DaveCTurner June 17, 2021 14:09
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx merged commit 82796f4 into elastic:master Jun 18, 2021
@tlrx tlrx deleted the default-check_on_startup branch June 18, 2021 07:13
@tlrx
Copy link
Member Author

tlrx commented Jun 18, 2021

Thanks David!

tlrx added a commit to tlrx/elasticsearch that referenced this pull request Jun 18, 2021
elastic#74235)

Today a searchable snapshot index inherits the 
index.shard.check_on_startup index setting from 
the snapshot it is mounted from. But this setting 
can require some expensive processing as 
documented in elastic#74233, so we decided in elastic#73147 
to default the value of this setting to false but 
allow the user to override this in the mount request.

Closes elastic#73147
@DaveCTurner
Copy link
Contributor

I just realised we missed a bit of tidy-up here, with this change we no longer need to set index.shard.check_on_startup: false in the MountSnapshotStep.

@tlrx
Copy link
Member Author

tlrx commented Jun 18, 2021

I also noticed that we need to always set at least to false in test because the test framework randomly insert this setting too.

I'll open a follow up

@ywangd
Copy link
Member

ywangd commented Jun 18, 2021

I raised #74282 since testCreateAndRestorePartialSearchableSnapshot failed on my PR branch and reproducible locally. Also muted the test.

tlrx added a commit that referenced this pull request Jun 18, 2021
…#74283)

Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

Relates #74235

This commit also fixes a failing test where some leftover code
was overriding the setting value.

Closes #74282
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Jun 18, 2021
Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

Backport of elastic#74283
Relates elastic#74235
tlrx added a commit that referenced this pull request Jun 18, 2021
…apshots (#74279)

Today a searchable snapshot index inherits the
index.shard.check_on_startup index setting from
the snapshot it is mounted from. But this setting
can require some expensive processing as
documented in #74233, so we decided in #73147
to default the value of this setting to false but
allow the user to override this in the mount request.

Backport of #74235
Closes #73147
tlrx added a commit that referenced this pull request Jun 18, 2021
…#74288)

Now the Mount API overrides the index.shard.check_on_startup
setting value to false, there's no need for MountSnapshotStep
to set it explicitly.

Backport of #74283
Relates #74235
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 >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.14.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Index shard corruption check does not play well with partially mounted searchable snapshots

5 participants