semantic_text: Add index metadata information for inference field mappers #107147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carlosdelest
changed the title
jimczi
approved these changes
Apr 5, 2024
server/src/main/java/org/elasticsearch/index/mapper/InferenceFieldMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
Outdated
Show resolved
Hide resolved
carlosdelest
added
>non-issue
:Search Foundations/Mapping
|
Pinging @elastic/es-search (Team:Search) |
benwtrent
approved these changes
Apr 8, 2024
There was a problem hiding this comment.
A minor concern. I think it all looks OK. My other concerns were about null being accidentally provided in the ctor as we have a ton of stuff that relies on the inference fields not being null, but instead empty, but it seems like we have adequate coverage and protection there. Just being paranoid :)
| String inferenceId = randomIdentifier(); | ||
| String[] inputFields = generateRandomStringArray(5, 10, false, false); | ||
| return new InferenceFieldMetadata(name, inferenceId, inputFields); | ||
| } |
There was a problem hiding this comment.
Could we have some tests covering the null cases that throw?
|
@elasticsearchmachine update branch |
Mikep86
approved these changes
Apr 9, 2024
server/src/main/java/org/elasticsearch/cluster/metadata/InferenceFieldMetadata.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataTests.java
Outdated
Show resolved
Hide resolved
original-brownbear
approved these changes
Apr 10, 2024
…metadata-changes # Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As prior work for
semantic_textnew field type, these changes add newIndexMetadatainformation for field types that need inference to be performed.As inference is done in the coordinator node to avoid re-doing it at shard / replica level, the coordinator needs to check for the need to perform inference for specific fields in an index.
Given that the coordinator node does not necessarily have mapping information for all indices (only for those that have shards in the node), the field inference information must be stored in the
IndexMetadataand broadcasted to all nodes.This change adds:
InferenceFieldMetadataclass that contains the information for fields that need inferenceInferenceFieldMapperinterface that will be implemented bysemantic_textand any other field types that require inference, so this information can be retrieved from the field mappingIndexMetadata