Skip to content

[ML] Fix acceptable model snapshot versions in ML deprecation checker #81060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.ml;

import org.elasticsearch.Version;
import org.elasticsearch.common.Numbers;
import org.elasticsearch.common.hash.MurmurHash3;
import org.elasticsearch.common.settings.Setting;
Expand Down Expand Up @@ -44,6 +45,13 @@ public final class MachineLearningField {
License.OperationMode.PLATINUM
);

// Ideally this would be 7.0.0, but it has to be 6.4.0 because due to an oversight it's impossible
// for the Java code to distinguish the model states for versions 6.4.0 to 7.9.3 inclusive.
public static final Version MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION = Version.fromString("6.4.0");
// We tell the user we support model snapshots newer than 7.0.0 as that's the major version
// boundary, even though behind the scenes we have to support back to 6.4.0.
public static final Version MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION = Version.V_7_0_0;

private MachineLearningField() {}

public static String valuesToId(String... values) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void testMlDeprecationChecks() throws Exception {
assertThat(response.getMlSettingsIssues(), hasSize(1));
assertThat(
response.getMlSettingsIssues().get(0).getMessage(),
containsString("model snapshot [1] for job [deprecation_check_job] needs to be deleted or upgraded")
containsString("Delete model snapshot [1] or update it to 7.0.0 or greater")
);
assertThat(response.getMlSettingsIssues().get(0).getMeta(), equalTo(Map.of("job_id", jobId, "snapshot_id", "1")));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

package org.elasticsearch.xpack.deprecation;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentElasticsearchExtension;
Expand All @@ -27,6 +26,9 @@
import java.util.Map;
import java.util.Optional;

import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION;

public class MlDeprecationChecker implements DeprecationChecker {

static Optional<DeprecationIssue> checkDataFeedQuery(DatafeedConfig datafeedConfig, NamedXContentRegistry xContentRegistry) {
Expand Down Expand Up @@ -67,22 +69,23 @@ static Optional<DeprecationIssue> checkDataFeedAggregations(DatafeedConfig dataf
}

static Optional<DeprecationIssue> checkModelSnapshot(ModelSnapshot modelSnapshot) {
if (modelSnapshot.getMinVersion().before(Version.V_7_0_0)) {
if (modelSnapshot.getMinVersion().before(MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION)) {
StringBuilder details = new StringBuilder(
String.format(
Locale.ROOT,
"model snapshot [%s] for job [%s] supports minimum version [%s] and needs to be at least [%s].",
// Important: the Kibana upgrade assistant expects this to match the pattern /[Mm]odel snapshot/
// and if it doesn't then the expected "Fix" button won't appear for this deprecation.
"Model snapshot [%s] for job [%s] has an obsolete minimum version [%s].",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@masseyke and @debadair I've added one word compared to what you used in #79387: "Model snapshot" instead of just "Snapshot" at the beginning. Please let me know if you strongly object to this.

modelSnapshot.getSnapshotId(),
modelSnapshot.getJobId(),
modelSnapshot.getMinVersion(),
Version.V_7_0_0
modelSnapshot.getMinVersion()
)
);
if (modelSnapshot.getLatestRecordTimeStamp() != null) {
details.append(
String.format(
Locale.ROOT,
" The model snapshot's latest record timestamp is [%s]",
" The model snapshot's latest record timestamp is [%s].",
XContentElasticsearchExtension.DEFAULT_FORMATTER.format(modelSnapshot.getLatestRecordTimeStamp().toInstant())
)
);
Expand All @@ -92,9 +95,9 @@ static Optional<DeprecationIssue> checkModelSnapshot(ModelSnapshot modelSnapshot
DeprecationIssue.Level.CRITICAL,
String.format(
Locale.ROOT,
"model snapshot [%s] for job [%s] needs to be deleted or upgraded",
"Delete model snapshot [%s] or update it to %s or greater.",
modelSnapshot.getSnapshotId(),
modelSnapshot.getJobId()
MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION
),
"https://www.elastic.co/guide/en/elasticsearch/reference/master/ml-upgrade-job-model-snapshot.html",
details.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
import java.util.function.Predicate;

import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
import static org.elasticsearch.xpack.ml.job.task.OpenJobPersistentTasksExecutor.MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.ml.job.task.OpenJobPersistentTasksExecutor.MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.ml.job.task.OpenJobPersistentTasksExecutor.checkAssignmentState;

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,15 @@

import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION;
import static org.elasticsearch.xpack.core.ml.MlTasks.AWAITING_UPGRADE;
import static org.elasticsearch.xpack.core.ml.MlTasks.PERSISTENT_TASK_MASTER_NODE_TIMEOUT;
import static org.elasticsearch.xpack.ml.job.JobNodeSelector.AWAITING_LAZY_ASSIGNMENT;

public class OpenJobPersistentTasksExecutor extends AbstractJobPersistentTasksExecutor<OpenJobAction.JobParams> {

private static final Logger logger = LogManager.getLogger(OpenJobPersistentTasksExecutor.class);
// Ideally this would be 7.0.0, but it has to be 6.4.0 because due to an oversight it's impossible
// for the Java code to distinguish the model states for versions 6.4.0 to 7.9.3 inclusive.
public static final Version MIN_CHECKED_SUPPORTED_SNAPSHOT_VERSION = Version.fromString("6.4.0");
// We tell the user we support model snapshots newer than 7.0.0 as that's the major version
// boundary, even though behind the scenes we have to support back to 6.4.0.
public static final Version MIN_REPORTED_SUPPORTED_SNAPSHOT_VERSION = Version.V_7_0_0;

// Resuming a job with a running datafeed from its current snapshot was added in 7.11 and
// can only be done if the master node is on or after that version.
Expand Down