Make mappings immutable. #15313
Make mappings immutable. #15313
Conversation
jpountz
added
>enhancement
:Search/Mapping
| @@ -565,18 +585,20 @@ public MultiFields build(FieldMapper.Builder mainFieldBuilder, BuilderContext co | |||
| } | |||
|
|
|||
| private final ContentPath.Type pathType; | |||
| private volatile ImmutableOpenMap<String, FieldMapper> mappers; | |||
| private final ImmutableOpenMap<String, FieldMapper> mappers; | |||
|
LGTM but I've been reviewing code for about 5 hours now and I'm starting to have some trouble focussing so I'm probably not to be trusted. |
|
No worries @nik9000, I've seen you been a very active reviewer recently indeed! |
| // to be not indexed on another type | ||
| // so we use IndexOptions.DOCS here but this will result | ||
| // in a mapping merge later | ||
| options = IndexOptions.DOCS; |
There was a problem hiding this comment.
I read the comment...but I still dont' understand...why can't we just throw an error?
|
+1, I left some minor comments. |
Today mappings are mutable because of two APIs: - Mapper.merge, which expects changes to be performed in-place - IncludeInAll, which allows to change whether values should be put in the `_all` field in place. This commit changes both APIs to return a modified copy instead of modifying in place so that mappings can be immutable. For now, only the type-level object is immutable, but in the future we can imagine making them immutable at the index-level so that mapping updates could be completely atomic at the index level. Close elastic#9365
jpountz
force-pushed
the
fix/immutable_mappings
branch
from
f8063f5
to
50eeafa
Compare
|
I just pushed a commit to address review comments. I'll merge soon. |
| conflicts.add("Cannot update excludes setting for [_source]"); | ||
| } | ||
| if (conflicts.isEmpty() == false) { | ||
| throw new IllegalArgumentException("Can't merge because of conflicts: " + conflicts); | ||
| } |
There was a problem hiding this comment.
It looks like this change also caused later modifications to includes and excludes to get denied. Was that intentional?
I wanted to add an exclude for an existing field mapping in the index, so that new documents indexed in the future will no longer store the raw (binary) input data as part of _source in the index. I will have to rebuild the index now (which will take two days to complete).
I would have expected that includes and excludes can be modified after the fact, but the definition will (obviously) only have an impact on newly indexed items.
Today mappings are mutable because of two APIs:
_allfield in place.This commit changes both APIs to return a modified copy instead of modifying in
place so that mappings can be immutable. For now, only the type-level object is
immutable, but in the future we can imagine making them immutable at the
index-level so that mapping updates could be completely atomic at the index
level.
Close #9365