-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[ML] More advanced model snapshot retention options #56125
Merged
droberts195
merged 8 commits into
elastic:master
from
droberts195:daily_model_snapshot_retention
May 5, 2020
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
8d03124
[ML] More advanced model snapshot retention options
droberts195 3e7bf0e
[DOCS] Updates retention descriptions
lcawl ee14fd3
Update docs/reference/ml/ml-shared.asciidoc
droberts195 7951a77
Fixing tests
droberts195 394be6d
Merge branch 'master' into daily_model_snapshot_retention
droberts195 3201549
Use a dedicated class instead of Tuple
droberts195 0bb92d9
Merge branch 'master' into daily_model_snapshot_retention
droberts195 c3f6a84
Make members final
droberts195 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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 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 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 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 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 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 |
---|---|---|
|
@@ -361,6 +361,18 @@ example, it can contain custom URL information as shown in | |
{ml-docs}/ml-configuring-url.html[Adding custom URLs to {ml} results]. | ||
end::custom-settings[] | ||
|
||
tag::daily-model-snapshot-retention-after-days[] | ||
Advanced configuration option. Specifies a number of days between 0 and the | ||
value of `model_snapshot_retention_days`. After this period of time, only the first | ||
model snapshot per day is retained for this job. Age is calculated relative to | ||
the timestamp of the newest model snapshot. For new jobs, the default value is | ||
`1`, which means that all snapshots are retained for one day. Older snapshots | ||
are thinned out such that only one per day is retained. For jobs that were | ||
created before this setting was available, the default value matches the | ||
`model_snapshot_retention_days` value, which preserves the original behavior | ||
and no thinning out of model snapshots occurs. | ||
end::daily-model-snapshot-retention-after-days[] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docs should be updated for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added that change in a commit. |
||
|
||
tag::data-description[] | ||
The data description defines the format of the input data when you send data to | ||
the job by using the <<ml-post-data,post data>> API. Note that when configure | ||
|
@@ -997,8 +1009,8 @@ end::model-snapshot-id[] | |
tag::model-snapshot-retention-days[] | ||
Advanced configuration option. The period of time (in days) that model snapshots | ||
are retained. Age is calculated relative to the timestamp of the newest model | ||
snapshot. The default value is `1`, which means snapshots that are one day | ||
(twenty-four hours) older than the newest snapshot are deleted. | ||
snapshot. The default value is `10`, which means snapshots that are ten days | ||
older than the newest snapshot are deleted. | ||
end::model-snapshot-retention-days[] | ||
|
||
tag::model-timestamp[] | ||
|
This file contains 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 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 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 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 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.
Is this sentence needed? If yes, it's unclear to me what this age affects.
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.
I think something is needed. Not sure how best to phrase it though.
Suppose you have the following model snapshots:
And suppose
model_snapshot_retention_days
is 4 anddaily_model_snapshot_retention_after_days
is 2.Then we will delete all snapshots older than Friday 9:04 am (= Tuesday 9:04 am - 4 days). So we will delete snapshots 7 and 8 by this rule. And for snapshots at or after Friday 9:04 am (= Tuesday 9:04 am - 4 days) but before Sunday 9:04 am (= Tuesday 9:04 am - 2 days) we will keep the first per 24 hour period. So we will keep snapshots 6 and 4 by this rule and delete snapshots 5 and 3.
You can see that this is all highly dependent on the timestamp of the latest model snapshot, because everything is measured relative to its timestamp. So this is what "Age is calculated relative to the timestamp of the newest model snapshot" was trying to say. Can you recommend a good way to explain it? I will probably do this docs change in a followup PR though if the PR build goes green soon as I need to get the main code change merged before the 7.8 branch is split.
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.
I've created elastic/stack-docs#1047 to update the Model snapshots (https://www.elastic.co/guide/en/machine-learning/current/ml-model-snapshots.html) page and can revisit the API details thereafter.
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.
Thanks Lisa!