Add small optimizations to PUT _component_template API
#135644
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -293,25 +293,26 @@ public ProjectMetadata execute(ProjectMetadata currentProject) throws Exception | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Add the given component template to the project. If {@code create} is true, we will fail if there exists a component template with | ||
| * the same name. If a component template with the same name exists, but the content is identical, no change will be made. | ||
| * This method will perform all necessary validation but assumes that the component template has already been normalized (see | ||
| * {@link #normalizeComponentTemplate(ComponentTemplate)}. | ||
| */ | ||
| public ProjectMetadata addComponentTemplate( | ||
| final ProjectMetadata project, | ||
| final boolean create, | ||
| final String name, | ||
| final ComponentTemplate template | ||
| ) throws IOException { | ||
| final ComponentTemplate existingTemplate = project.componentTemplates().get(name); | ||
| if (existingTemplate != null) { | ||
| if (create) { | ||
| throw new IllegalArgumentException("component template [" + name + "] already exists"); | ||
| } | ||
| if (template.contentEquals(existingTemplate)) { | ||
| return project; | ||
| } | ||
| } | ||
|
|
||
| // Collect all the composable (index) templates that use this component template, we'll use | ||
|
|
@@ -324,9 +325,9 @@ public ProjectMetadata addComponentTemplate( | |
| .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); | ||
|
|
||
| // if we're updating a component template, let's check if it's part of any V2 template that will yield the CT update invalid | ||
| if (create == false && template.template().settings() != null) { | ||
| // if the CT is specifying the `index.hidden` setting it cannot be part of any global template | ||
| if (IndexMetadata.INDEX_HIDDEN_SETTING.exists(template.template().settings())) { | ||
| List<String> globalTemplatesThatUseThisComponent = new ArrayList<>(); | ||
| for (Map.Entry<String, ComposableIndexTemplate> entry : templatesUsingComponent.entrySet()) { | ||
| ComposableIndexTemplate templateV2 = entry.getValue(); | ||
|
|
@@ -350,47 +351,27 @@ public ProjectMetadata addComponentTemplate( | |
| } | ||
| } | ||
|
|
||
| final Long now = instantSource.instant().toEpochMilli(); | ||
| final Long createdDateMillis = existingTemplate == null ? now : existingTemplate.createdDateMillis().orElse(null); | ||
| final ComponentTemplate finalComponentTemplate = new ComponentTemplate( | ||
| template.template(), | ||
| template.version(), | ||
| template.metadata(), | ||
| template.deprecated(), | ||
| createdDateMillis, | ||
| now | ||
| ); | ||
|
|
||
| // These two validation checks are only scoped to the component template itself (and don't depend on any other entities in the | ||
| // cluster state) and could thus be done in the transport action. However, since we're parsing mappings here, we shouldn't be doing | ||
| // it directly on the transport thread. Instead, we should fork to a different threadpool (management/generic). | ||
| validateTemplate(finalComponentTemplate.template().settings(), finalComponentTemplate.template().mappings(), indicesService); | ||
| validate(name, finalComponentTemplate.template(), List.of(), null); | ||
|
Comment on lines
+366
to
370
There was a problem hiding this comment. I experimented with moving these two validations to the transport action, but since the Distributed Coordination team advised running them on the management or generic threadpool, I decided to leave it as future work. In terms of overall processing, we wouldn't save much. The only/main win would be that we avoid adding a cluster state update task in case the validation fails. Also worth noting is that we potentially do more (heavier) mappings validation in |
||
|
|
||
| ProjectMetadata projectWithComponentTemplateAdded = ProjectMetadata.builder(project).put(name, finalComponentTemplate).build(); | ||
| // Validate all composable index templates that use this component template | ||
| if (templatesUsingComponent.isEmpty() == false) { | ||
| IllegalArgumentException validationFailure = null; | ||
| for (Map.Entry<String, ComposableIndexTemplate> entry : templatesUsingComponent.entrySet()) { | ||
| final String composableTemplateName = entry.getKey(); | ||
| final ComposableIndexTemplate composableTemplate = entry.getValue(); | ||
|
|
@@ -424,10 +405,42 @@ public ProjectMetadata addComponentTemplate( | |
| .addWarningHeaderIfDataRetentionNotEffective(globalRetentionSettings.get(false), false); | ||
| } | ||
|
|
||
| logger.info("{} component template [{}]", existingTemplate == null ? "adding" : "updating", name); | ||
| return projectWithComponentTemplateAdded; | ||
| } | ||
|
|
||
| /** | ||
| * Normalize the given component template by trying to normalize settings and wrapping mappings if necessary. Returns the same instance | ||
| * if nothing needs to be done. | ||
| */ | ||
| public ComponentTemplate normalizeComponentTemplate(final ComponentTemplate componentTemplate) throws IOException { | ||
| Template template = componentTemplate.template(); | ||
| // Normalize the index settings if necessary | ||
| Settings prefixedSettings = null; | ||
| if (template.settings() != null) { | ||
| prefixedSettings = template.settings().maybeNormalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX); | ||
| } | ||
| // TODO: theoretically, we could avoid parsing the mappings once by combining this wrapping with the mapping validation later on, | ||
| // but that refactoring will be non-trivial as we currently don't seem to have methods available to merge already-parsed mappings; | ||
| // we only allow merging mappings from CompressedXContent. | ||
| CompressedXContent wrappedMappings = MetadataIndexTemplateService.wrapMappingsIfNecessary(template.mappings(), xContentRegistry); | ||
|
|
||
| // No need to build a new component template if we didn't change anything. | ||
| // We can check for reference equality since `maybeNormalizePrefix` and `wrapMappingsIfNecessary` return the same instance if | ||
| // nothing needs to be done. | ||
| if (prefixedSettings == template.settings() && wrappedMappings == template.mappings()) { | ||
| return componentTemplate; | ||
| } | ||
| return new ComponentTemplate( | ||
| Template.builder(template).settings(prefixedSettings).mappings(wrappedMappings).build(), | ||
| componentTemplate.version(), | ||
| componentTemplate.metadata(), | ||
| componentTemplate.deprecated(), | ||
| componentTemplate.createdDateMillis().orElse(null), | ||
| componentTemplate.modifiedDateMillis().orElse(null) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Mappings in templates don't have to include <code>_doc</code>, so update the mappings to include this single type if necessary | ||
| * | ||
|
|
@@ -2009,7 +2022,7 @@ private static void validateCompositeTemplate( | |
| } | ||
|
|
||
| public static void validateTemplate(Settings validateSettings, CompressedXContent mappings, IndicesService indicesService) | ||
| throws IOException { | ||
| // Hard to validate settings if they're non-existent, so used empty ones if none were provided | ||
| Settings settings = validateSettings; | ||
| if (settings == null) { | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the added lines in this class are solely from moving the tests over from
MetadataIndexTemplateServiceTests.