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

Searchable snapshot rolling restart #66369

Conversation

henningandersen
Copy link
Contributor

@henningandersen henningandersen commented Dec 15, 2020

Add a new boolean setting
xpack.searchable.snapshot.allocate_on_rolling_restart
that indicates the behavior of searchable snapshots when the generic
cluster.routing.allocation.enable setting is set to primaries.
Defaults to false, meaning that when cluster.routing.allocation.enable=primaries,
searchable snapshots are not allocated.This ensures existing rolling restart procedures
do not reallocate searchable snapshot shards to other nodes.

Add a new setting
`xpack.searchable.snapshot.allocation.enable.primaries`
that indicates the behavior of searchable snapshots when the generic
`cluster.routing.allocation.enable` setting is set to `primaries`.
Valid values are NONE and PRIMARIES (defer to generic decider). Defaults
to NONE since this ensures existing rolling restart procedures do not
reallocate searchable snapshot shards to other nodes.
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Dec 15, 2020
@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.

Looks good, I left a few thoughts mostly on UX. We also need to adjust the docs to match 😁 (can be a followup if you'd prefer)

internalCluster().restartNode(indexNode);
}

assertBusy(() -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need to assertBusy() here, if this fails even once then that's a fail.

*
*/
public static final Setting<EnableAllocationDecider.Allocation> SEARCHABLE_SNAPSHOTS_ALLOCATION_ENABLE_PRIMARIES_SETTING =
new Setting<>("xpack.searchable.snapshot.allocation.enable.primaries", EnableAllocationDecider.Allocation.NONE.toString(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this means we have these options:

xpack.searchable.snapshot.allocation.enable.primaries: none
xpack.searchable.snapshot.allocation.enable.primaries: primaries

The repetition of primaries seems odd to me. I see that this is a bit nicer than a straight boolean, given the relationship to the other allocation decider, but how about these options instead?

xpack.searchable.snapshot.allocation.enable: none
xpack.searchable.snapshot.allocation.enable: all

Copy link
Contributor

Choose a reason for hiding this comment

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

I misunderstood the precise meaning of this setting, sorry, it defaults to none!

We discussed this in another channel and decided a boolean called xpack.searchable.snapshot.allocate_on_rolling_restart which defaults to false.

primariesAllocation
);
} else {
return allocation.decision(Decision.YES, NAME, "decider relies on generic enable decider");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this message is wrong, the decider return YES here because xpack.searchable.snapshot.allocation.enable.primaries: primaries permits it.

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

@henningandersen
Copy link
Contributor Author

Thanks David.

@henningandersen henningandersen merged commit dd9b913 into elastic:master Dec 15, 2020
henningandersen added a commit that referenced this pull request Dec 15, 2020
Add a new boolean setting
`xpack.searchable.snapshot.allocate_on_rolling_restart`
that indicates the behavior of searchable snapshots when the generic
`cluster.routing.allocation.enable` setting is set to `primaries`.
Defaults to false, meaning that when `cluster.routing.allocation.enable=primaries`,
searchable snapshots are not allocated.This ensures existing rolling restart procedures
do not reallocate searchable snapshot shards to other nodes.
arteam added a commit to arteam/elasticsearch that referenced this pull request Oct 7, 2024
…art` setting

The setting was created as an escape-hatch in case elastic#66369 had some unintended side-effects. It has been deprecated since elastic#84959 (8.2.0).
arteam added a commit that referenced this pull request Nov 14, 2024
…art` setting (#114202)

The setting was created as an escape-hatch in case #66369 had some unintended side-effects. It has been deprecated since #84959 (8.2.0).

---------

Co-authored-by: David Turner <[email protected]>
salvatore-campagna pushed a commit to salvatore-campagna/elasticsearch that referenced this pull request Nov 18, 2024
…art` setting (elastic#114202)

The setting was created as an escape-hatch in case elastic#66369 had some unintended side-effects. It has been deprecated since elastic#84959 (8.2.0).

---------

Co-authored-by: David Turner <[email protected]>
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
…art` setting (elastic#114202)

The setting was created as an escape-hatch in case elastic#66369 had some unintended side-effects. It has been deprecated since elastic#84959 (8.2.0).

---------

Co-authored-by: David Turner <[email protected]>
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 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants