From 8287f4008889e04fe17de249dabee72196a1c0d9 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Mon, 24 Mar 2025 10:31:49 -0400 Subject: [PATCH 1/2] Optimize MlConfigVersion#getMinMaxMlConfigVersion (#125432) --- .../xpack/core/ml/MlConfigVersion.java | 33 ++++++++++++------- .../action/GetTrainedModelsStatsAction.java | 8 +---- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java index 1b365bd96d834..5c8664510056f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.NavigableMap; import java.util.Set; @@ -280,17 +281,24 @@ public static MlConfigVersion getMaxMlConfigVersion(DiscoveryNodes nodes) { public static Tuple getMinMaxMlConfigVersion(DiscoveryNodes nodes) { MlConfigVersion minMlConfigVersion = MlConfigVersion.CURRENT; MlConfigVersion maxMlConfigVersion = MlConfigVersion.FIRST_ML_VERSION; + + // many nodes will have the same versions, so de-duplicate versions first so that we only have to parse unique versions + final Set versions = new HashSet<>(); for (DiscoveryNode node : nodes) { - try { - MlConfigVersion mlConfigVersion = getMlConfigVersionForNode(node); - if (mlConfigVersion.before(minMlConfigVersion)) { - minMlConfigVersion = mlConfigVersion; - } - if (mlConfigVersion.after(maxMlConfigVersion)) { - maxMlConfigVersion = mlConfigVersion; - } - } catch (IllegalStateException e) { - // This means we encountered a node that is after 8.10.0 but has the ML plugin disabled - ignore it + String mlConfigVerStr = node.getAttributes().get(ML_CONFIG_VERSION_NODE_ATTR); + if (mlConfigVerStr != null) { // ignore nodes that don't have an ML config version + versions.add(mlConfigVerStr); + } + } + + // of the unique versions, find the min and max + for (String version : versions) { + MlConfigVersion mlConfigVersion = fromString(version); + if (mlConfigVersion.before(minMlConfigVersion)) { + minMlConfigVersion = mlConfigVersion; + } + if (mlConfigVersion.after(maxMlConfigVersion)) { + maxMlConfigVersion = mlConfigVersion; } } return new Tuple<>(minMlConfigVersion, maxMlConfigVersion); @@ -304,6 +312,9 @@ public static MlConfigVersion getMlConfigVersionForNode(DiscoveryNode node) { return fromId(node.getPre811VersionId().orElseThrow(() -> new IllegalStateException("getting legacy version id not possible"))); } + // supports fromString below + private static final Pattern ML_VERSION_PATTERN = Pattern.compile("^(\\d+)\\.(\\d+)\\.(\\d+)(?:-\\w+)?$"); + // Parse an MlConfigVersion from a string. // Note that version "8.10.x" and "8.11.0" are silently converted to "10.0.0". // This is to support upgrade scenarios in pre-prod QA environments. @@ -319,7 +330,7 @@ public static MlConfigVersion fromString(String str) { if (str.startsWith("8.10.") || str.equals("8.11.0")) { return V_10; } - Matcher matcher = Pattern.compile("^(\\d+)\\.(\\d+)\\.(\\d+)(?:-\\w+)?$").matcher(str); + Matcher matcher = ML_VERSION_PATTERN.matcher(str); if (matcher.matches() == false) { throw new IllegalArgumentException("ML config version [" + str + "] not valid"); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java index 718714a2d20d1..fba53aadb7b49 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java @@ -91,12 +91,6 @@ public static class TrainedModelStats implements ToXContentObject, Writeable { private final AssignmentStats deploymentStats; private final int pipelineCount; - private static final IngestStats EMPTY_INGEST_STATS = new IngestStats( - IngestStats.Stats.IDENTITY, - Collections.emptyList(), - Collections.emptyMap() - ); - public TrainedModelStats( String modelId, TrainedModelSizeStats modelSizeStats, @@ -107,7 +101,7 @@ public TrainedModelStats( ) { this.modelId = Objects.requireNonNull(modelId); this.modelSizeStats = modelSizeStats; - this.ingestStats = ingestStats == null ? EMPTY_INGEST_STATS : ingestStats; + this.ingestStats = ingestStats == null ? IngestStats.IDENTITY : ingestStats; if (pipelineCount < 0) { throw new ElasticsearchException("[{}] must be a greater than or equal to 0", PIPELINE_COUNT.getPreferredName()); } From 18c50753e409cffafb52ca80c2a159929248598d Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Mon, 24 Mar 2025 13:29:10 -0400 Subject: [PATCH 2/2] Revert part of #125432 The logic is more complicated on 8.x than on main, so we need more of the code to remain as it originally was. --- .../xpack/core/ml/MlConfigVersion.java | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java index 5c8664510056f..dd1b35c2dcd0e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlConfigVersion.java @@ -25,7 +25,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.NavigableMap; import java.util.Set; @@ -281,24 +280,17 @@ public static MlConfigVersion getMaxMlConfigVersion(DiscoveryNodes nodes) { public static Tuple getMinMaxMlConfigVersion(DiscoveryNodes nodes) { MlConfigVersion minMlConfigVersion = MlConfigVersion.CURRENT; MlConfigVersion maxMlConfigVersion = MlConfigVersion.FIRST_ML_VERSION; - - // many nodes will have the same versions, so de-duplicate versions first so that we only have to parse unique versions - final Set versions = new HashSet<>(); for (DiscoveryNode node : nodes) { - String mlConfigVerStr = node.getAttributes().get(ML_CONFIG_VERSION_NODE_ATTR); - if (mlConfigVerStr != null) { // ignore nodes that don't have an ML config version - versions.add(mlConfigVerStr); - } - } - - // of the unique versions, find the min and max - for (String version : versions) { - MlConfigVersion mlConfigVersion = fromString(version); - if (mlConfigVersion.before(minMlConfigVersion)) { - minMlConfigVersion = mlConfigVersion; - } - if (mlConfigVersion.after(maxMlConfigVersion)) { - maxMlConfigVersion = mlConfigVersion; + try { + MlConfigVersion mlConfigVersion = getMlConfigVersionForNode(node); + if (mlConfigVersion.before(minMlConfigVersion)) { + minMlConfigVersion = mlConfigVersion; + } + if (mlConfigVersion.after(maxMlConfigVersion)) { + maxMlConfigVersion = mlConfigVersion; + } + } catch (IllegalStateException e) { + // This means we encountered a node that is after 8.10.0 but has the ML plugin disabled - ignore it } } return new Tuple<>(minMlConfigVersion, maxMlConfigVersion);