Skip to content

Commit

Permalink
[ML] Use results retention time for deleting system annotations (elas…
Browse files Browse the repository at this point in the history 
…tic#76096)

In elastic#75617 a new setting, system_annotations_retention_days, was
added to control how long system annotations are retained for.
We now feel that this setting is redundant and that system
annotations should be retained for the same period as results.
This is intuitive and defensible, as system annotations can be
considered a type of result.

Followup to elastic#75617
  • Loading branch information
@droberts195
droberts195 authored Aug 4, 2021
1 parent 858cc32 commit 7ac5ea3
Show file tree
Hide file tree
Showing 19 changed files with 54 additions and 189 deletions.
25 changes: 3 additions & 22 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/Job.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public class Job implements ToXContentObject {
public static final ParseField DAILY_MODEL_SNAPSHOT_RETENTION_AFTER_DAYS =
new ParseField("daily_model_snapshot_retention_after_days");
public static final ParseField RESULTS_RETENTION_DAYS = new ParseField("results_retention_days");
public static final ParseField SYSTEM_ANNOTATIONS_RETENTION_DAYS = new ParseField("system_annotations_retention_days");
public static final ParseField MODEL_SNAPSHOT_ID = new ParseField("model_snapshot_id");
public static final ParseField RESULTS_INDEX_NAME = new ParseField("results_index_name");
public static final ParseField DELETING = new ParseField("deleting");
Expand Down Expand Up @@ -84,7 +83,6 @@ public class Job implements ToXContentObject {
PARSER.declareString((builder, val) -> builder.setBackgroundPersistInterval(
TimeValue.parseTimeValue(val, BACKGROUND_PERSIST_INTERVAL.getPreferredName())), BACKGROUND_PERSIST_INTERVAL);
PARSER.declareLong(Builder::setResultsRetentionDays, RESULTS_RETENTION_DAYS);
PARSER.declareLong(Builder::setSystemAnnotationsRetentionDays, SYSTEM_ANNOTATIONS_RETENTION_DAYS);
PARSER.declareLong(Builder::setModelSnapshotRetentionDays, MODEL_SNAPSHOT_RETENTION_DAYS);
PARSER.declareLong(Builder::setDailyModelSnapshotRetentionAfterDays, DAILY_MODEL_SNAPSHOT_RETENTION_AFTER_DAYS);
PARSER.declareField(Builder::setCustomSettings, (p, c) -> p.mapOrdered(), CUSTOM_SETTINGS, ValueType.OBJECT);
Expand All @@ -110,7 +108,6 @@ public class Job implements ToXContentObject {
private final Long modelSnapshotRetentionDays;
private final Long dailyModelSnapshotRetentionAfterDays;
private final Long resultsRetentionDays;
private final Long systemAnnotationsRetentionDays;
private final Map<String, Object> customSettings;
private final String modelSnapshotId;
private final String resultsIndexName;
Expand All @@ -122,7 +119,7 @@ private Job(String jobId, String jobType, List<String> groups, String descriptio
AnalysisConfig analysisConfig, AnalysisLimits analysisLimits, DataDescription dataDescription,
ModelPlotConfig modelPlotConfig, Long renormalizationWindowDays, TimeValue backgroundPersistInterval,
Long modelSnapshotRetentionDays, Long dailyModelSnapshotRetentionAfterDays, Long resultsRetentionDays,
Long systemAnnotationsRetentionDays, Map<String, Object> customSettings, String modelSnapshotId, String resultsIndexName,
Map<String, Object> customSettings, String modelSnapshotId, String resultsIndexName,
Boolean deleting, Boolean allowLazyOpen) {

this.jobId = jobId;
Expand All @@ -140,7 +137,6 @@ private Job(String jobId, String jobType, List<String> groups, String descriptio
this.modelSnapshotRetentionDays = modelSnapshotRetentionDays;
this.dailyModelSnapshotRetentionAfterDays = dailyModelSnapshotRetentionAfterDays;
this.resultsRetentionDays = resultsRetentionDays;
this.systemAnnotationsRetentionDays = systemAnnotationsRetentionDays;
this.customSettings = customSettings == null ? null : Collections.unmodifiableMap(customSettings);
this.modelSnapshotId = modelSnapshotId;
this.resultsIndexName = resultsIndexName;
Expand Down Expand Up @@ -266,10 +262,6 @@ public Long getResultsRetentionDays() {
return resultsRetentionDays;
}

public Long getSystemAnnotationsRetentionDays() {
return systemAnnotationsRetentionDays;
}

public Map<String, Object> getCustomSettings() {
return customSettings;
}
Expand Down Expand Up @@ -332,9 +324,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (resultsRetentionDays != null) {
builder.field(RESULTS_RETENTION_DAYS.getPreferredName(), resultsRetentionDays);
}
if (systemAnnotationsRetentionDays != null) {
builder.field(SYSTEM_ANNOTATIONS_RETENTION_DAYS.getPreferredName(), systemAnnotationsRetentionDays);
}
if (customSettings != null) {
builder.field(CUSTOM_SETTINGS.getPreferredName(), customSettings);
}
Expand Down Expand Up @@ -383,7 +372,6 @@ public boolean equals(Object other) {
&& Objects.equals(this.customSettings, that.customSettings)
&& Objects.equals(this.modelSnapshotId, that.modelSnapshotId)
&& Objects.equals(this.resultsIndexName, that.resultsIndexName)
&& Objects.equals(this.systemAnnotationsRetentionDays, that.systemAnnotationsRetentionDays)
&& Objects.equals(this.deleting, that.deleting)
&& Objects.equals(this.allowLazyOpen, that.allowLazyOpen);
}
Expand All @@ -393,7 +381,7 @@ public int hashCode() {
return Objects.hash(jobId, jobType, groups, description, createTime, finishedTime,
analysisConfig, analysisLimits, dataDescription, modelPlotConfig, renormalizationWindowDays,
backgroundPersistInterval, modelSnapshotRetentionDays, dailyModelSnapshotRetentionAfterDays, resultsRetentionDays,
systemAnnotationsRetentionDays, customSettings, modelSnapshotId, resultsIndexName, deleting, allowLazyOpen);
customSettings, modelSnapshotId, resultsIndexName, deleting, allowLazyOpen);
}

@Override
Expand Down Expand Up @@ -422,7 +410,6 @@ public static class Builder {
private Long modelSnapshotRetentionDays;
private Long dailyModelSnapshotRetentionAfterDays;
private Long resultsRetentionDays;
private Long systemAnnotationsRetentionDays;
private Map<String, Object> customSettings;
private String modelSnapshotId;
private String resultsIndexName;
Expand Down Expand Up @@ -452,7 +439,6 @@ public Builder(Job job) {
this.modelSnapshotRetentionDays = job.getModelSnapshotRetentionDays();
this.dailyModelSnapshotRetentionAfterDays = job.getDailyModelSnapshotRetentionAfterDays();
this.resultsRetentionDays = job.getResultsRetentionDays();
this.systemAnnotationsRetentionDays = job.getSystemAnnotationsRetentionDays();
this.customSettings = job.getCustomSettings() == null ? null : new LinkedHashMap<>(job.getCustomSettings());
this.modelSnapshotId = job.getModelSnapshotId();
this.resultsIndexName = job.getResultsIndexNameNoPrefix();
Expand Down Expand Up @@ -544,11 +530,6 @@ public Builder setResultsRetentionDays(Long resultsRetentionDays) {
return this;
}

public Builder setSystemAnnotationsRetentionDays(Long systemAnnotationsRetentionDays) {
this.systemAnnotationsRetentionDays = systemAnnotationsRetentionDays;
return this;
}

public Builder setModelSnapshotId(String modelSnapshotId) {
this.modelSnapshotId = modelSnapshotId;
return this;
Expand Down Expand Up @@ -581,7 +562,7 @@ public Job build() {
id, jobType, groups, description, createTime, finishedTime,
analysisConfig, analysisLimits, dataDescription, modelPlotConfig, renormalizationWindowDays,
backgroundPersistInterval, modelSnapshotRetentionDays, dailyModelSnapshotRetentionAfterDays, resultsRetentionDays,
systemAnnotationsRetentionDays, customSettings, modelSnapshotId, resultsIndexName, deleting, allowLazyOpen);
customSettings, modelSnapshotId, resultsIndexName, deleting, allowLazyOpen);
}
}
}
30 changes: 3 additions & 27 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/JobUpdate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class JobUpdate implements ToXContentObject {
TimeValue.parseTimeValue(val, Job.BACKGROUND_PERSIST_INTERVAL.getPreferredName())), Job.BACKGROUND_PERSIST_INTERVAL);
PARSER.declareLong(Builder::setRenormalizationWindowDays, Job.RENORMALIZATION_WINDOW_DAYS);
PARSER.declareLong(Builder::setResultsRetentionDays, Job.RESULTS_RETENTION_DAYS);
PARSER.declareLong(Builder::setSystemAnnotationsRetentionDays, Job.SYSTEM_ANNOTATIONS_RETENTION_DAYS);
PARSER.declareLong(Builder::setModelSnapshotRetentionDays, Job.MODEL_SNAPSHOT_RETENTION_DAYS);
PARSER.declareLong(Builder::setDailyModelSnapshotRetentionAfterDays, Job.DAILY_MODEL_SNAPSHOT_RETENTION_AFTER_DAYS);
PARSER.declareStringArray(Builder::setCategorizationFilters, AnalysisConfig.CATEGORIZATION_FILTERS);
Expand All @@ -63,7 +62,6 @@ public class JobUpdate implements ToXContentObject {
private final Long modelSnapshotRetentionDays;
private final Long dailyModelSnapshotRetentionAfterDays;
private final Long resultsRetentionDays;
private final Long systemAnnotationsRetentionDays;
private final List<String> categorizationFilters;
private final PerPartitionCategorizationConfig perPartitionCategorizationConfig;
private final Map<String, Object> customSettings;
Expand All @@ -74,7 +72,7 @@ private JobUpdate(String jobId, @Nullable List<String> groups, @Nullable String
@Nullable List<DetectorUpdate> detectorUpdates, @Nullable ModelPlotConfig modelPlotConfig,
@Nullable AnalysisLimits analysisLimits, @Nullable TimeValue backgroundPersistInterval,
@Nullable Long renormalizationWindowDays, @Nullable Long resultsRetentionDays,
@Nullable Long systemAnnotationsRetentionDays, @Nullable Long modelSnapshotRetentionDays,
@Nullable Long modelSnapshotRetentionDays,
@Nullable Long dailyModelSnapshotRetentionAfterDays, @Nullable List<String> categorizationFilters,
@Nullable PerPartitionCategorizationConfig perPartitionCategorizationConfig,
@Nullable Map<String, Object> customSettings, @Nullable Boolean allowLazyOpen, @Nullable TimeValue modelPruneWindow) {
Expand All @@ -89,7 +87,6 @@ private JobUpdate(String jobId, @Nullable List<String> groups, @Nullable String
this.modelSnapshotRetentionDays = modelSnapshotRetentionDays;
this.dailyModelSnapshotRetentionAfterDays = dailyModelSnapshotRetentionAfterDays;
this.resultsRetentionDays = resultsRetentionDays;
this.systemAnnotationsRetentionDays = systemAnnotationsRetentionDays;
this.categorizationFilters = categorizationFilters;
this.perPartitionCategorizationConfig = perPartitionCategorizationConfig;
this.customSettings = customSettings;
Expand Down Expand Up @@ -137,10 +134,6 @@ public Long getResultsRetentionDays() {
return resultsRetentionDays;
}

public Long getSystemAnnotationsRetentionDays() {
return systemAnnotationsRetentionDays;
}

public List<String> getCategorizationFilters() {
return categorizationFilters;
}
Expand Down Expand Up @@ -195,9 +188,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (resultsRetentionDays != null) {
builder.field(Job.RESULTS_RETENTION_DAYS.getPreferredName(), resultsRetentionDays);
}
if (systemAnnotationsRetentionDays != null) {
builder.field(Job.SYSTEM_ANNOTATIONS_RETENTION_DAYS.getPreferredName(), systemAnnotationsRetentionDays);
}
if (categorizationFilters != null) {
builder.field(AnalysisConfig.CATEGORIZATION_FILTERS.getPreferredName(), categorizationFilters);
}
Expand Down Expand Up @@ -240,7 +230,6 @@ public boolean equals(Object other) {
&& Objects.equals(this.modelSnapshotRetentionDays, that.modelSnapshotRetentionDays)
&& Objects.equals(this.dailyModelSnapshotRetentionAfterDays, that.dailyModelSnapshotRetentionAfterDays)
&& Objects.equals(this.resultsRetentionDays, that.resultsRetentionDays)
&& Objects.equals(this.systemAnnotationsRetentionDays, that.systemAnnotationsRetentionDays)
&& Objects.equals(this.categorizationFilters, that.categorizationFilters)
&& Objects.equals(this.perPartitionCategorizationConfig, that.perPartitionCategorizationConfig)
&& Objects.equals(this.customSettings, that.customSettings)
Expand All @@ -252,7 +241,7 @@ public boolean equals(Object other) {
public int hashCode() {
return Objects.hash(jobId, groups, description, detectorUpdates, modelPlotConfig, analysisLimits, renormalizationWindowDays,
backgroundPersistInterval, modelSnapshotRetentionDays, dailyModelSnapshotRetentionAfterDays, resultsRetentionDays,
systemAnnotationsRetentionDays, categorizationFilters, perPartitionCategorizationConfig, customSettings, allowLazyOpen,
categorizationFilters, perPartitionCategorizationConfig, customSettings, allowLazyOpen,
modelPruneWindow);
}

Expand Down Expand Up @@ -348,7 +337,6 @@ public static class Builder {
private Long modelSnapshotRetentionDays;
private Long dailyModelSnapshotRetentionAfterDays;
private Long resultsRetentionDays;
private Long systemAnnotationsRetentionDays;
private List<String> categorizationFilters;
private PerPartitionCategorizationConfig perPartitionCategorizationConfig;
private Map<String, Object> customSettings;
Expand Down Expand Up @@ -484,18 +472,6 @@ public Builder setResultsRetentionDays(Long resultsRetentionDays) {
return this;
}

/**
* Advanced configuration option. The number of days for which job annotations are retained
*
* Updates the {@link Job#systemAnnotationsRetentionDays} setting
*
* @param systemAnnotationsRetentionDays number of days to keep results.
*/
public Builder setSystemAnnotationsRetentionDays(Long systemAnnotationsRetentionDays) {
this.systemAnnotationsRetentionDays = systemAnnotationsRetentionDays;
return this;
}

/**
* Sets the categorization filters on the {@link Job}
*
Expand Down Expand Up @@ -546,7 +522,7 @@ public Builder setModelPruneWindow(TimeValue modelPruneWindow) {

public JobUpdate build() {
return new JobUpdate(jobId, groups, description, detectorUpdates, modelPlotConfig, analysisLimits, backgroundPersistInterval,
renormalizationWindowDays, resultsRetentionDays, systemAnnotationsRetentionDays, modelSnapshotRetentionDays,
renormalizationWindowDays, resultsRetentionDays, modelSnapshotRetentionDays,
dailyModelSnapshotRetentionAfterDays, categorizationFilters, perPartitionCategorizationConfig, customSettings,
allowLazyOpen, modelPruneWindow);
}
Expand Down
3 changes: 0 additions & 3 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ public static Job.Builder createRandomizedJobBuilder() {
if (randomBoolean()) {
builder.setResultsRetentionDays(randomNonNegativeLong());
}
if (randomBoolean()) {
builder.setSystemAnnotationsRetentionDays(randomNonNegativeLong());
}
if (randomBoolean()) {
builder.setCustomSettings(Collections.singletonMap(randomAlphaOfLength(10), randomAlphaOfLength(10)));
}
Expand Down
3 changes: 0 additions & 3 deletions .../rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobUpdateTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ public static JobUpdate createRandom(String jobId) {
if (randomBoolean()) {
update.setResultsRetentionDays(randomNonNegativeLong());
}
if (randomBoolean()) {
update.setSystemAnnotationsRetentionDays(randomNonNegativeLong());
}
if (randomBoolean()) {
update.setCategorizationFilters(Arrays.asList(generateRandomStringArray(10, 10, false)));
}
Expand Down
4 changes: 0 additions & 4 deletions docs/reference/ml/anomaly-detection/apis/put-job.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,10 +379,6 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=results-index-name]
(Optional, long)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=results-retention-days]

`system_annotations_retention_days`::
(Optional, long)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=system-annotations-retention-days]

[[ml-put-job-example]]
== {api-examples-title}

Expand Down
7 changes: 1 addition & 6 deletions docs/reference/ml/anomaly-detection/apis/update-job.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,6 @@ close the job, then reopen the job and restart the {dfeed} for the changes to ta
(long)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=results-retention-days]

`system_annotations_retention_days`::
(long)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=system-annotations-retention-days]


[[ml-update-job-example]]
== {api-examples-title}
Expand All @@ -260,8 +256,7 @@ POST _ml/anomaly_detectors/low_request_rate/_update
"renormalization_window_days": 30,
"background_persist_interval": "2h",
"model_snapshot_retention_days": 7,
"results_retention_days": 60,
"system_annotations_retention_days": 60
"results_retention_days": 60
}
--------------------------------------------------
// TEST[skip:setup:Kibana sample data]
Expand Down
14 changes: 3 additions & 11 deletions docs/reference/ml/ml-shared.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1430,19 +1430,11 @@ retained. Age is calculated relative to the timestamp of the latest bucket
result. If this property has a non-null value, once per day at 00:30 (server
time), results that are the specified number of days older than the latest
bucket result are deleted from {es}. The default value is null, which means all
results are retained.
results are retained. Annotations generated by the system also count as results
for retention purposes; they are deleted after the same number of days as
results. Annotations added by users are retained forever.
end::results-retention-days[]

tag::system-annotations-retention-days[]
Advanced configuration option. The period of time (in days) that automatically
created annotations are retained. Age is calculated relative to the timestamp of
the latest bucket result. If this property has a non-null value, once per day at
00:30 (server time), annotations that are the specified number of days older
than the latest bucket result are deleted from {es}. The default value is null,
which means all annotations are retained.
User created annotations are never deleted automatically.
end::system-annotations-retention-days[]

tag::retain[]
If `true`, this snapshot will not be deleted during automatic cleanup of
snapshots older than `model_snapshot_retention_days`. However, this snapshot
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugin/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ def v7compatibilityNotSupportedTests = {
'rollup/start_job/Test start job twice',
'service_accounts/10_basic/Test service account tokens', // https://github.com/elastic/elasticsearch/pull/75200

// temporarily muted awaiting backport of https://github.com/elastic/elasticsearch/pull/76097
'ml/delete_expired_data/Test delete expired data with body parameters',
'ml/delete_expired_data/Test delete expired data with no body',
'ml/delete_expired_data/Test delete expired data with path parameters',
'ml/delete_expired_data/Test delete expired data with unknown job id',
'ml/jobs_crud/Test update job',

// a type field was added to cat.ml_trained_models #73660, this is a backwards compatible change.
// still this is a cat api, and we don't support them with rest api compatibility. (the test would be very hard to transform too)
'ml/trained_model_cat_apis/Test cat trained models'
Expand Down
Loading

0 comments on commit 7ac5ea3

Please sign in to comment.