Skip to content

Documenting master_is_stable health API settings#87901

Merged
masseyke merged 9 commits intoelastic:mainfrom
masseyke:doc/health-api-settings
Jul 26, 2022
Merged

Documenting master_is_stable health API settings#87901
masseyke merged 9 commits intoelastic:mainfrom
masseyke:doc/health-api-settings

Conversation

@masseyke
Copy link
Copy Markdown
Member

@masseyke masseyke commented Jun 21, 2022

This adds documentation for the settings used in the master_is_stable part of the health API.
Preview URL: https://elasticsearch_87901.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/health-api-settings.html

@masseyke masseyke added >docs General docs changes :Distributed/Health Issues for the health report API v8.4.0 labels Jun 21, 2022
@elasticmachine elasticmachine added Team:Data Management (obsolete) DO NOT USE. This team no longer exists. Team:Docs Meta label for docs team labels Jun 21, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

Thanks Keith. Left some comments

++++

The following are the _expert-level_ settings available for configuring the
<<health-api, Health API>>. It is not recommended to change any of these
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say this configures an internal diagnostics service as opposed to the API.

What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe I should change it to "for configuring an internal diagnostics service that is exposed through the Health API"? To the user changes to these settings will only be visible via the health api right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this idea. Essentially these are internal diagnostics, they are now used in the health API but they might have more use cases, right?

What about calling this page "Health diagnostic settings" or "Internal health diagnostic settings" and only refer to the health API in the begging. Something like: "the outcome of this service is exposed via the Health API"?

I think this could open up different ways of phrasing the impact of the settings. For example:

This many changes or more will be reported as a problem in the Health API could be phrased as This many changes or more will be reported as unhealthy?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

++ I like it too and I like your suggestions @gmarouli !

==== Cluster level settings

`health.master_history.has_master_lookup_timeframe`::
(<<static-cluster-setting,Static>>) The amount of time the API looks back to see if the cluster has had
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say we're looking back to see if an instance observed any masters. We're not making cluster-level decisions here.

What do you think?


`master_history.max_age`::
(<<static-cluster-setting,Static>>) The maximum amount of time that the master history is kept in memory
for use in the Health API. Master node changes older than this time will not be considered in the Health
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
for use in the Health API. Master node changes older than this time will not be considered in the Health
. Master node changes older than this time will not be considered in the Health

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should potentially decouple these settings from the health API (I see it's mentioned again)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we don't say how these impact the end user, then no one would ever change them, right? That might not be such a bad thing though.

Copy link
Copy Markdown
Contributor

@gmarouli gmarouli left a comment

Choose a reason for hiding this comment

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

Nice work @masseyke , thank you for setting this page up, I will need soon too.

My only comment is in the existing discussion https://github.com/elastic/elasticsearch/pull/87901/files#r905907682.

@masseyke masseyke requested review from andreidan and gmarouli July 22, 2022 14:36
@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:06
Copy link
Copy Markdown
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

LGTM with a few suggestions
Thanks Keith

masseyke and others added 2 commits July 26, 2022 09:13
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
@masseyke masseyke requested a review from andreidan July 26, 2022 14:14
Copy link
Copy Markdown
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for iterating on this Keith

@masseyke masseyke merged commit e61bfcf into elastic:main Jul 26, 2022
@masseyke masseyke deleted the doc/health-api-settings branch July 26, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Health Issues for the health report API >docs General docs changes Team:Data Management (obsolete) DO NOT USE. This team no longer exists. Team:Docs Meta label for docs team v8.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants