Skip to content

Conversation

lukewhiting
Copy link
Contributor

Fixes #104233

This PR allows the PUT _watcher/settings endpoint to accept shard allocation settings while also adding restrictions to prevent users modifying or removing the index.routing.allocation.include._tier_preference setting on the .watches index. It also modifies the GET _watcher/settings to show the newly allowed settings.

By controlling shard allocation, You also control which nodes in the cluster actually execute watches. The relevant documentation is also updated to show this new way of adjusting which nodes Watcher runs on.

I chose this approach over removing the reference in the documentation as that felt like a breaking change / removal of previous functionality.

Now accepts index.routing.allocation.* settings but denies changing
the allocation setting that keeps watches on data nodes
Now returns index.routing.allocation.* settings explicitly filters out
the `index.routing.allocation.include._tier_preference` setting
@lukewhiting lukewhiting added >bug >docs General docs changes :Data Management/Watcher auto-backport Automatically create backport pull requests when merged v9.0.0 v8.17.0 labels Oct 21, 2024
@lukewhiting lukewhiting requested a review from masseyke October 21, 2024 15:31
Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team labels Oct 21, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_PREFIX
);

public static final Set<String> EXPLICITLY_DENIED_SETTINGS = Set.of(
Copy link
Member

Choose a reason for hiding this comment

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

I assume you're denying this one because we always want the .watches shards in the hot tier? That makes me wonder what other possible values there are for these afix settings, and if we ought to just explicitly allow the index.routing.allocation.exclude.role, index.routing.allocation.include.role, and index.routing.allocation.require.role ones and disallow anything else to avoid causing some unforeseen problems. You've probably looked at all these settings more than I have -- what do you think?

Copy link
Contributor Author

@lukewhiting lukewhiting Oct 22, 2024

Choose a reason for hiding this comment

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

So I based this restriction on what the default setting for the .watcher index are. I assumed we put it in the hot tier for good reason so locked off the ability to change that. I may be wrong in that assumption.

Locking down the index.routing.allocation.*.* settings further is difficult / undesirable. index.routing.allocation.include.role isn't actually a setting as such but a convention for a wildcard setting. Anything after index.routing.allocation.include. is assumed to be an arbitrary node attribute hence my leaving it as allowing any setting with that prefix as we have no way of knowing what attributes the end user might have set on their nodes and how they would like to use those for scheduling watches.

For instance index.routing.allocation.include.sandwich_filling=turkey is a valid setting for this if the user chose to use a bread based node attributes scheme to group their cluster members :-)

Copy link
Member

@masseyke masseyke left a comment

Choose a reason for hiding this comment

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

LGTM!

@lukewhiting lukewhiting merged commit 36c45c1 into elastic:main Oct 22, 2024
17 checks passed
lukewhiting added a commit to lukewhiting/elasticsearch that referenced this pull request Oct 22, 2024
* Update settings endpoint modified

Now accepts index.routing.allocation.* settings but denies changing
the allocation setting that keeps watches on data nodes

* Get settings endpoint modified

Now returns index.routing.allocation.* settings explicitly filters out
the `index.routing.allocation.include._tier_preference` setting

* Tests for modified endpoints

* Update docs
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

@lukewhiting lukewhiting deleted the 104233-watcher-index-allocation branch October 22, 2024 14:41
elasticsearchmachine pushed a commit that referenced this pull request Oct 22, 2024
* Update settings endpoint modified

Now accepts index.routing.allocation.* settings but denies changing
the allocation setting that keeps watches on data nodes

* Get settings endpoint modified

Now returns index.routing.allocation.* settings explicitly filters out
the `index.routing.allocation.include._tier_preference` setting

* Tests for modified endpoints

* Update docs
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
* Update settings endpoint modified

Now accepts index.routing.allocation.* settings but denies changing
the allocation setting that keeps watches on data nodes

* Get settings endpoint modified

Now returns index.routing.allocation.* settings explicitly filters out
the `index.routing.allocation.include._tier_preference` setting

* Tests for modified endpoints

* Update docs
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
* Update settings endpoint modified

Now accepts index.routing.allocation.* settings but denies changing
the allocation setting that keeps watches on data nodes

* Get settings endpoint modified

Now returns index.routing.allocation.* settings explicitly filters out
the `index.routing.allocation.include._tier_preference` setting

* Tests for modified endpoints

* Update docs
@herrBez
Copy link

herrBez commented Mar 17, 2025

@lukewhiting
Copy link
Contributor Author

@herrBez The documentation has now been updated and will go live with the next release :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >bug :Data Management/Watcher >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot update watcher index settings to set index.routing.allocation.include.role as described in the docs

4 participants