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

RestClusterGetSettingsAction Requests the Full Metadata from Master #82342

Closed
Tracked by #77466
original-brownbear opened this issue Jan 7, 2022 · 1 comment · Fixed by #86405
Closed
Tracked by #77466

RestClusterGetSettingsAction Requests the Full Metadata from Master #82342

original-brownbear opened this issue Jan 7, 2022 · 1 comment · Fixed by #86405
Assignees
Labels
>bug :Core/Infra/Settings Settings infrastructure and APIs Team:Core/Infra Meta label for core/infra team

Comments

@original-brownbear
Copy link
Member

original-brownbear commented Jan 7, 2022

Similar issue as solved in #78829.

This API is surprisingly expensive because it requests the full metadata via get cluster state request from master when called only to return just the settings from it to the user. This can mean requesting tens or hundreds of MB from master in a large cluster unexpectedly.

We should adjust this to not require fetching the full cluster state. One option would be to add a dedicate master node action for returning just the settings.

Relates #77466

@original-brownbear original-brownbear added >bug :Core/Infra/Settings Settings infrastructure and APIs labels Jan 7, 2022
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 7, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@grcevski grcevski self-assigned this Mar 30, 2022
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue May 1, 2022
Today there are request and response objects for the
get-cluster-settings action but the request is unused and the response
is only used in the REST layer. This commit removes the unused request
and renames the response to reflect that it's not a transport-layer
response. It also tidies a few things up in this area, removing the
unused `ActionResponse` superclass, making its fields final, and
replacing the overly-general `RestBuilderListener` with a regular
`RestToXContentListener` in the REST action.

Relates elastic#82342 because to resolve that issue we will want to introduce
transport-layer request/response classes, and the classes involved in
this commit are in the way of that change.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue May 2, 2022
Today there are request and response objects for the
get-cluster-settings action but the request is unused and the response
is only used in the REST layer. This commit removes the unused request
and renames the response to reflect that it's not a transport-layer
response. It also tidies a few things up in this area, removing the
unused `ActionResponse` superclass, making its fields final, and
replacing the overly-general `RestBuilderListener` with a regular
`RestToXContentListener` in the REST action.

Relates elastic#82342 because to resolve that issue we will want to introduce
transport-layer request/response classes, and the classes involved in
this commit are in the way of that change.
elasticsearchmachine pushed a commit that referenced this issue May 2, 2022
Today there are request and response objects for the
get-cluster-settings action but the request is unused and the response
is only used in the REST layer. This commit removes the unused request
and renames the response to reflect that it's not a transport-layer
response. It also tidies a few things up in this area, removing the
unused `ActionResponse` superclass, making its fields final, and
replacing the overly-general `RestBuilderListener` with a regular
`RestToXContentListener` in the REST action.

Relates #82342 because to resolve that issue we will want to introduce
transport-layer request/response classes, and the classes involved in
this commit are in the way of that change.
grcevski pushed a commit to grcevski/elasticsearch that referenced this issue May 3, 2022
The existing API to retrieve the cluster settings relies on
pulling in the full cluster state, which can be very expensive.

This change adds a dedicated cluster settings API, avoiding
serializing the full cluster state.

Closes elastic#82342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Settings Settings infrastructure and APIs Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants