Remove PROTO-serialization from IndexMetaData.Custom #32683
Conversation
Switches IndexMetaData.Custom to standard named serialization. In order to achieve this it also removes support for specifying custom index metadata on create index and put template requests.
imotov
added
>breaking
v7.0.0
:Distributed/Distributed
|
Pinging @elastic/es-distributed |
This change ruins plans that we have in CCR to use the index metadata to store a UUID (either the index UUID of the leader, or hopefully the history UUID of the leader). We would do this on create and follow requests, for example. What is the motivation for this change and breaking custom index metadata in this way? |
jasontedor
added
the
:Data Management/Indices APIs
|
@jasontedor I have removed it because I thought that it will not used as part of the templates of create index requests. Supporting custom parsing in templates creates additional complexity (for example, we need to know how to parse both Map and XContent and we parse the template in HL REST response). So, I was trying to avoid this unnecessary complexity mistakenly thinking that we are not going to have any users for it. |
|
We intend to use custom index metadata in both CCR and ILM. I would prefer that we proof out what our needs are there before we make any changes to custom index metadata. This feature was on its way out (deprecated!) but we are going to resurrect it. |
|
I am fine with resurrecting it, I am just not OK with resurrecting it based on PROTO serialization. I left it as is when I was converting the rest of the cluster state to NamedObject serialization because it wasn't used so we marked it as deprecated instead. So, I feel responsible for finishing PROTO off :) I will see if I can migrate it without significant loss of functionality, now that I know that it is needed by more than one efforts. |
Switches IndexMetaData.Custom to standard named serialization. Supersedes elastic#32683
Currently, if geo context is represented by something other than geo_point or an object with lat and lon fields, the parsing of it as a geo context can result in ignoring the context altogether, returning confusing errors such as number_format_exception or trying to parse the number specifying as long-encoded hash code. It would also fail if the geo_point was stored. This commit makes the mapping parsing more strict and will fail during mapping update or index creation if the geo context doesn't point to a geo_point field. Supersedes elastic#32683 Closes elastic#32202
This PR removes the deprecated `Custom` class in `IndexMetaData`, in favor
of a `Map<String, DiffableStringMap>` that is used to store custom index
metadata. As part of this, there is now no way to set this metadata in a
template or create index request (since it's only set by plugins, or dedicated
REST endpoints).
The `Map<String, DiffableStringMap>` is intended to be a namespaced `Map<String,
String>` (`DiffableStringMap` implements `Map<String, String>`, so the signature
is more like `Map<String, Map<String, String>>`). This is so we can do things
like:
``` java
Map<String, String> ccrMeta = indexMetaData.getCustom("ccr");
```
And then have complete control over the metadata. This also means any
plugin/feature that uses this has to manage its own BWC, as the map is just
serialized as a map. It also means that if metadata is put in the map that isn't
used (for instance, if a plugin were removed), it causes no failures the way
an unregistered `Setting` would.
The reason I use a custom `DiffableStringMap` here rather than a plain
`Map<String, String>` is so the map can be diffed with previous cluster state
updates for serialization.
Supersedes #32683
…32749) This PR removes the deprecated `Custom` class in `IndexMetaData`, in favor of a `Map<String, DiffableStringMap>` that is used to store custom index metadata. As part of this, there is now no way to set this metadata in a template or create index request (since it's only set by plugins, or dedicated REST endpoints). The `Map<String, DiffableStringMap>` is intended to be a namespaced `Map<String, String>` (`DiffableStringMap` implements `Map<String, String>`, so the signature is more like `Map<String, Map<String, String>>`). This is so we can do things like: ``` java Map<String, String> ccrMeta = indexMetaData.getCustom("ccr"); ``` And then have complete control over the metadata. This also means any plugin/feature that uses this has to manage its own BWC, as the map is just serialized as a map. It also means that if metadata is put in the map that isn't used (for instance, if a plugin were removed), it causes no failures the way an unregistered `Setting` would. The reason I use a custom `DiffableStringMap` here rather than a plain `Map<String, String>` is so the map can be diffed with previous cluster state updates for serialization. Supersedes elastic#32683
Switches IndexMetaData.Custom to standard named serialization. In
order to achieve this it also removes support for specifying custom
index metadata on create index and put template requests.
@dakrone as per our discussion