From a589f00a3c9d95234d262c18772bc4431d6c5fe2 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Tue, 26 Sep 2023 15:41:46 +0100 Subject: [PATCH] [ML] Decouple ML template versioning from product version The ML index templates used to be versioned using the product version. This won't work in serverless, so needs to be changed. Most plugins have a separate version constant for their index templates that started at 1 and is incremented manually when any template changes. For ML, we've got used to _not_ having to worry about index template versions, only mappings versions. To try to continue this approach as far as possible, the new strategy for ML index template versions is: - Start with 10000000. This is because 8.11.0's ID is 8110099, and whatever new scheme we use must generate numbers bigger than this. - Add on the mappings version constants for all index mappings for which we use index templates. This means that incrementing the mappings version is still sufficient to cause the templates to be reinstalled. It's only necessary to increment the base template version if something changes in a template that's _not_ in the mappings, such as priority. This should be very rare. So the risk of forgetting to update the template versions when updating mappings is removed. --- .../ml/notifications/NotificationsIndex.java | 2 +- .../xpack/core/template/TemplateUtils.java | 5 +-- .../xpack/ml/MachineLearning.java | 7 +++- .../xpack/ml/MlIndexTemplateRegistry.java | 34 ++++++++++++++----- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/notifications/NotificationsIndex.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/notifications/NotificationsIndex.java index 06a923cd9d275..08f493fd29523 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/notifications/NotificationsIndex.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/notifications/NotificationsIndex.java @@ -17,7 +17,7 @@ public final class NotificationsIndex { private static final String RESOURCE_PATH = "/ml/"; private static final String MAPPINGS_VERSION_VARIABLE = "xpack.ml.version"; - private static final int NOTIFICATIONS_INDEX_MAPPINGS_VERSION = 1; + public static final int NOTIFICATIONS_INDEX_MAPPINGS_VERSION = 1; private NotificationsIndex() {} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/TemplateUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/TemplateUtils.java index 895305d9372b8..ad27607e47c5e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/TemplateUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/TemplateUtils.java @@ -117,14 +117,15 @@ public static String replaceVariable(String input, String variable, String value * Checks if a versioned template exists, and if it exists checks if the version is greater than or equal to the current version. * @param templateName Name of the index template * @param state Cluster state + * @param currentVersion The current version to check against */ - public static boolean checkTemplateExistsAndVersionIsGTECurrentVersion(String templateName, ClusterState state) { + public static boolean checkTemplateExistsAndVersionIsGTECurrentVersion(String templateName, ClusterState state, long currentVersion) { ComposableIndexTemplate templateMetadata = state.metadata().templatesV2().get(templateName); if (templateMetadata == null) { return false; } - return templateMetadata.version() != null && templateMetadata.version() >= Version.CURRENT.id; + return templateMetadata.version() != null && templateMetadata.version() >= currentVersion; } /** diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index d527f998a1ee8..c8e4fd488394a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -1752,7 +1752,12 @@ public static boolean criticalTemplatesInstalled(ClusterState clusterState) { // installs it if necessary List templateNames = List.of(STATE_INDEX_PREFIX, AnomalyDetectorsIndex.jobResultsIndexPrefix()); for (String templateName : templateNames) { - allPresent = allPresent && TemplateUtils.checkTemplateExistsAndVersionIsGTECurrentVersion(templateName, clusterState); + allPresent = allPresent + && TemplateUtils.checkTemplateExistsAndVersionIsGTECurrentVersion( + templateName, + clusterState, + MlIndexTemplateRegistry.ML_INDEX_TEMPLATE_VERSION + ); } return allPresent; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlIndexTemplateRegistry.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlIndexTemplateRegistry.java index 0d1ba0d3fece1..91e738bf2183b 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlIndexTemplateRegistry.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlIndexTemplateRegistry.java @@ -6,7 +6,6 @@ */ package org.elasticsearch.xpack.ml; -import org.elasticsearch.Version; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.service.ClusterService; @@ -29,6 +28,23 @@ public class MlIndexTemplateRegistry extends IndexTemplateRegistry { + /** + * The template version starts from 10000000 because up until 8.11.0 we + * used version IDs for template versioning, so the first detached + * version number needs to be higher than the version ID of 8.11.0. + * We add on the mappings version of each of the templates that has + * mappings. This will cause _all_ templates to get installed when the + * mappings for any single template change. However, this is better + * than the risk of potentially updating mappings without updating the + * template versions and hence not reinstalling the templates. Note that + * the state index has no mappings - its template basically just says + * this - hence there's no mappings version for the state index. Please + * add a comment with a reason each time the base number is incremented. + * 10000001: TODO - reason + */ + public static final int ML_INDEX_TEMPLATE_VERSION = 10000000 + AnomalyDetectorsIndex.RESULTS_INDEX_MAPPINGS_VERSION + + NotificationsIndex.NOTIFICATIONS_INDEX_MAPPINGS_VERSION + MlStatsIndex.STATS_INDEX_MAPPINGS_VERSION; + private static final String ROOT_RESOURCE_PATH = "/ml/"; private static final String ANOMALY_DETECTION_PATH = ROOT_RESOURCE_PATH + "anomalydetection/"; private static final String VERSION_PATTERN = "xpack.ml.version"; @@ -42,7 +58,7 @@ public class MlIndexTemplateRegistry extends IndexTemplateRegistry { private IndexTemplateConfig stateTemplate() { Map variables = new HashMap<>(); - variables.put(VERSION_ID_PATTERN, String.valueOf(Version.CURRENT.id)); + variables.put(VERSION_ID_PATTERN, String.valueOf(ML_INDEX_TEMPLATE_VERSION)); // In serverless a different version of "state_index_template.json" is shipped that won't substitute the ILM policy variable variables.put(INDEX_LIFECYCLE_NAME, ML_SIZE_BASED_ILM_POLICY_NAME); variables.put(INDEX_LIFECYCLE_ROLLOVER_ALIAS, AnomalyDetectorsIndex.jobStateIndexWriteAlias()); @@ -50,7 +66,7 @@ private IndexTemplateConfig stateTemplate() { return new IndexTemplateConfig( AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX, ANOMALY_DETECTION_PATH + "state_index_template.json", - Version.CURRENT.id, + ML_INDEX_TEMPLATE_VERSION, VERSION_PATTERN, variables ); @@ -58,13 +74,13 @@ private IndexTemplateConfig stateTemplate() { private static IndexTemplateConfig anomalyDetectionResultsTemplate() { Map variables = new HashMap<>(); - variables.put(VERSION_ID_PATTERN, String.valueOf(Version.CURRENT.id)); + variables.put(VERSION_ID_PATTERN, String.valueOf(ML_INDEX_TEMPLATE_VERSION)); variables.put("xpack.ml.anomalydetection.results.mappings", AnomalyDetectorsIndex.resultsMapping()); return new IndexTemplateConfig( AnomalyDetectorsIndex.jobResultsIndexPrefix(), ANOMALY_DETECTION_PATH + "results_index_template.json", - Version.CURRENT.id, + ML_INDEX_TEMPLATE_VERSION, VERSION_PATTERN, variables ); @@ -72,13 +88,13 @@ private static IndexTemplateConfig anomalyDetectionResultsTemplate() { private static IndexTemplateConfig notificationsTemplate() { Map variables = new HashMap<>(); - variables.put(VERSION_ID_PATTERN, String.valueOf(Version.CURRENT.id)); + variables.put(VERSION_ID_PATTERN, String.valueOf(ML_INDEX_TEMPLATE_VERSION)); variables.put("xpack.ml.notifications.mappings", NotificationsIndex.mapping()); return new IndexTemplateConfig( NotificationsIndex.NOTIFICATIONS_INDEX, ROOT_RESOURCE_PATH + "notifications_index_template.json", - Version.CURRENT.id, + ML_INDEX_TEMPLATE_VERSION, VERSION_PATTERN, variables ); @@ -86,7 +102,7 @@ private static IndexTemplateConfig notificationsTemplate() { private IndexTemplateConfig statsTemplate() { Map variables = new HashMap<>(); - variables.put(VERSION_ID_PATTERN, String.valueOf(Version.CURRENT.id)); + variables.put(VERSION_ID_PATTERN, String.valueOf(ML_INDEX_TEMPLATE_VERSION)); variables.put("xpack.ml.stats.mappings", MlStatsIndex.mapping()); // In serverless a different version of "stats_index_template.json" is shipped that won't substitute the ILM policy variable variables.put(INDEX_LIFECYCLE_NAME, ML_SIZE_BASED_ILM_POLICY_NAME); @@ -95,7 +111,7 @@ private IndexTemplateConfig statsTemplate() { return new IndexTemplateConfig( MlStatsIndex.TEMPLATE_NAME, ROOT_RESOURCE_PATH + "stats_index_template.json", - Version.CURRENT.id, + ML_INDEX_TEMPLATE_VERSION, VERSION_PATTERN, variables );