Skip to content

Introduce MapperMergeContext#104512

Merged
elasticsearchmachine merged 5 commits intoelastic:mainfrom
felixbarny:mapper-merge-context
Jan 18, 2024
Merged

Introduce MapperMergeContext#104512
elasticsearchmachine merged 5 commits intoelastic:mainfrom
felixbarny:mapper-merge-context

Conversation

@felixbarny
Copy link
Copy Markdown
Member

Currently, this is basically a noop.
This is in preparation for #102936, in an effort to reduce the mechanical changes.

Currently, this is basically a noop.
This is in preparation for elastic#102936,
in an effort to reduce the mechanical changes.
@felixbarny felixbarny added >non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types v8.13.0 labels Jan 18, 2024
@felixbarny felixbarny requested a review from javanna January 18, 2024 10:35
@elasticsearchmachine elasticsearchmachine added Team:Search Meta label for search team external-contributor Pull request authored by a developer outside the Elasticsearch team labels Jan 18, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left two small comments, LGTM otherwise

Comment thread server/src/main/java/org/elasticsearch/index/mapper/MapperMergeContext.java Outdated
@felixbarny felixbarny added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 18, 2024
Comment thread server/src/main/java/org/elasticsearch/index/mapper/MapperMergeContext.java Outdated
@felixbarny
Copy link
Copy Markdown
Member Author

@elasticmachine update branch

@elasticsearchmachine elasticsearchmachine merged commit f84c015 into elastic:main Jan 18, 2024
@felixbarny felixbarny deleted the mapper-merge-context branch January 18, 2024 14:10
elasticsearchmachine pushed a commit that referenced this pull request Jan 23, 2024
This is extracting a piece of independent functionality from
#96235

This is the result of this discussion:
#96235 (comment)

The mapping merge executed on the master node is the only authoritative
place where we have a consistent view of the cluster state as it's
guaranteed to be executed on a single thread on the master node.
Therefore, there can be no race conditions and we exactly know how many
fields are left before hitting the field limit.

Therefore, this is the best place to add as many fields as possible
without going over the limit if
`index.mapping.total_fields.ignore_dynamic_beyond_limit` is set to
`true` in the index settings.

This change makes it possible to limit the number of fields added during
mapping merger. However, that feature isn't actually used outside of
tests.

In #96235, the feature to
limit fields during the merge process is used to guarantee that the
cluster state update never fails due to dynamic mapping updates that
exceed the field limit in case `ignore_dynamic_beyond_limit` is enabled.

This works because after each dynamic mapping update, the indexing
request gets retried (there's no change to that logic in this or related
PRs) so that the document parsing takes the new mapping into account.

This only applies to dynamic mapping updates aka auto-updates. When
explicitly trying to update the mapping with too many fields, the put
mapping operation will still fail, even if `ignore_dynamic_beyond_limit`
is enabled.

Depends on: - [x] #104512 -
[x] #104578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) external-contributor Pull request authored by a developer outside the Elasticsearch team >non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants