Remove default mapping #44945
Remove default mapping #44945
Conversation
romseygeek
added
:Search Foundations/Mapping
|
Pinging @elastic/es-search |
There was a problem hiding this comment.
Thanks @romseygeek, this is looking good to me! I spotted one other reference to _default_ here: https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java#L126
To check I'm on the same page with the upgrade plan, this is my understanding of what we will recommend to users upgrading from 7.x -> 8.x:
- There may be some indices created in 6.x that still contain default mappings. However, these will need to be reindexed before upgrading to 8.x, so we don't need any special logic to handle these mappings during the upgrade.
- If there are any index templates from 6.x that haven't been updated, users should make sure to re-add them with no default mapping (and no type).
| @@ -727,8 +724,7 @@ static IndexMetaData validateResize(ClusterState state, String sourceIndex, | |||
| throw new IllegalStateException("index " + sourceIndex + " must be read-only to resize index. use \"index.blocks.write=true\""); | |||
| } | |||
|
|
|||
| if ((targetIndexMappingsTypes.size() > 1 || | |||
| (targetIndexMappingsTypes.isEmpty() || targetIndexMappingsTypes.contains(MapperService.DEFAULT_MAPPING)) == false)) { | |||
| if ((targetIndexMappingsTypes.size() != 0)) { | |||
There was a problem hiding this comment.
Minor comment: extra parentheses here, and could be targetIndexMappingsTypes.size() > 0.
| // first, simulate: just call merge and ignore the result | ||
| existingMapper.merge(newMapper.mapping()); | ||
| } | ||
| newMapper = mapperService.parse(request.type(), mappingUpdateSource); |
There was a problem hiding this comment.
This could be combined with the declaration: DocumentMapper newMapper = ...
| if (mappings.containsKey(MapperService.DEFAULT_MAPPING)) { | ||
| MappingMetaData defaultMapping = mappings.get(MapperService.DEFAULT_MAPPING); | ||
| for (ObjectCursor<MappingMetaData> cursor : mappings.values()) { | ||
| cursor.value.updateDefaultMapping(defaultMapping); |
There was a problem hiding this comment.
I think we can nuke the updateDefaultMapping method now that this block is deleted.
|
Thanks for the review @jtibshirani (and apologies for the gap between updates). I made some updates based on your feedback.
Right
Yes - I'll check to see if we're emitting deprecation warnings here already |
There was a problem hiding this comment.
This looks good to me. I see just one more reference to the default mapping that would be good to remove before merging: https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java#L126
|
@elasticmachine test this please |
|
@elasticmachine please test this |
|
@elasticmachine test this please |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
Default mappings cannot be created in 7x indexes, so we no longer need to keep
support for them in master.