-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Support choosing the downsampling method in data stream lifecycle #137023
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
Open
gmarouli
wants to merge
23
commits into
elastic:main
Choose a base branch
from
gmarouli:refactor-downsampling-dlm
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+871
−389
Open
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
64b01b5
Switch interval validation to accept `DateHistogramInterval`
gmarouli 345346d
Switch interval validation to accept `DateHistogramInterval`
gmarouli 097115d
Do not use DownsampleConfig in DataStreamLifecycle
gmarouli 3195327
Rename downsampling to downsampling rounds
gmarouli 1d383f6
Add downsampling method to the data stream lifecycle config
gmarouli 9107d76
Update rest endpoints
gmarouli bf80af5
Use the sampling method in the data stream lifecycle
gmarouli dd69548
Update docs/changelog/137023.yaml
gmarouli a328a4a
Update 137023.yaml
gmarouli ff2708c
Fix test
gmarouli 23823bf
merge with main
gmarouli 53e7079
Rename transport version
gmarouli 67099d3
Fix and simplify DataStreamLifecycleTests & DataStreamLifecycleTempla…
gmarouli 58a4994
Make random sampling method more dynamic
gmarouli a1d989c
Improvements based on review
gmarouli 5340685
merge with main
gmarouli 231506c
Add explanation about DLM using the source sampling method
gmarouli 8d4fa3d
Test template composition of data stream lifecycle.
gmarouli 3c10f8b
Fix test
gmarouli fcd8dd9
Remove unused constructor
gmarouli 67dd8ea
Merge branch 'main' into refactor-downsampling-dlm
gmarouli e23f767
Fix test adding sampling method when there are no rounds
gmarouli 4ca957f
Merge branch 'main' into refactor-downsampling-dlm
gmarouli 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 hidden or 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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 137023 | ||
| summary: Support choosing the downsampling method in data stream lifecycle | ||
| area: "Data streams" | ||
| type: enhancement | ||
| issues: [] |
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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.
What exactly is the reasoning behind this sampling method logic? Why are we specifying the sampling method from the source index metadata? Is there a reason we can't just always pass the
requestedDownsamplingMethod? I'm not saying it's wrong, it's just unexpected to me. Could you explain your thinking at least in a comment in the code and optionally here in the PR?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.
The goal of this is to make data stream lifecycle handle changes of the sampling method gracefully. Consider the following scenario.
The same thing applies for ILM, but in ILM a user can create a new policy with the new sampling method so it will only be applied to the newest backing indices.
Considering that we want data stream lifecycle to work seamlessly, we thought that using the downsampling method of the source index when available is closer to how the user is expecting it to work (considering the limitations).
Does this make sense?
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.
Yep, that definitely makes sense. Thanks for the explanation :). Could you put that explanation somewhere in this method?