Render Mappings more Compact in GET /_cluster/state#83846
Render Mappings more Compact in GET /_cluster/state#83846original-brownbear wants to merge 2 commits intoelastic:mainfrom original-brownbear:more-compact-cs-rest-clean
Conversation
This deduplicates index mappings in the `/_cluster_state` response. This significantly reduces the response size for large cluster states, making this debug API easier to read for humans (because understanding whether or not mappings are duplicated across indices might be valuable) and more importantly less ressource hungry as far as serialization goes.
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @original-brownbear, I've created a changelog YAML for you. |
DaveCTurner
left a comment
There was a problem hiding this comment.
Generally +1 from me but we still need to consider whether to treat this as a breaking change or not. We could alternatively not change the default but start sending 429s to clients that request an unreasonably large response. Marking this as Request changes until these questions are resolved.
henningandersen
left a comment
There was a problem hiding this comment.
Preparing for our discussion today, I read through this and provided a couple of comments.
| if (mmd == null) { | ||
| builder.nullField(KEY_MAPPINGS); | ||
| } else { | ||
| builder.field(KEY_MAPPINGS, indexMetadata.mapping().getSha256()); |
There was a problem hiding this comment.
I would prefer to pick a new name for the SHA256. mappings_sha256 maybe or perhaps shared_mappings_id. Then we can also allow dumping both, which could provide a transitional path for clients.
|
|
||
| if (context == XContentContext.API) { | ||
| if (params.paramAsBoolean(MAPPINGS_BY_HASH_PARAM, true)) { | ||
| builder.startObject("mappings"); |
There was a problem hiding this comment.
Perhaps we can call this shared_mappings and then have a flag of the same name to the API as well as a cluster setting to disable dumping non-shared mappings?
DaveCTurner
left a comment
There was a problem hiding this comment.
Henning and I discussed this again today. We're not confident enough that nobody would be affected by this change in the default behaviour, and we think with chunked-encoding we can protect Elasticsearch against harm at least, so we would prefer to make the desired behaviour opt-in today, deprecate the old default, and remove it in the next major version.
I'll raise a proposal to do this with the breaking changes committee.
|
Pinging @elastic/es-distributed-obsolete (Team:Distributed (Obsolete)) |
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
|
I see that this was closed because it lacked consensus on the breaking change. Why not take this in steps:
What do you think? |
This deduplicates index mappings in the
/_cluster/stateresponse by grouping them by the sha256 hash of the mapping.This significantly reduces the response size for large cluster states,
making this debug API easier to read for humans (because understanding
whether or not mappings are duplicated across indices might be valuable)
and more importantly less ressource hungry as far as serialization goes.
Adds an undocumented parameter to get back to the old format if we actually have to in an unforeseen instance.
New rendering of an index:
and then mappings at the same level as "indices":
relates #77466