Add period drop before rule #6415
Conversation
|
Does the coordinator console need adjusted to account for this rule? I haven't looked recently, but the coordinator has some hard-coded strings in its console |
|
@nishantmonu51 thanks for your review, I'll add more commits to do that, and it seems I also need to open a PR in druid-console repo to make some corresponding adjustments. |
|
BTW, why not embed the |
|
Have adjusted the coordinator console and tested it in a test cluster. |
| @@ -151,6 +155,10 @@ function makeDropByPeriod(rule) { | |||
| return "<span class='rule_label'>period</span><input type='text' name='period' " + "value='" + rule.period + "'/>"; | |||
| } | |||
|
|
|||
| function makeDropBeforeByPeriod(rule) { | |||
| return "<span class='rule_label'>period</span><input type='text' name='period' " + "value='" + rule.period + "'/>"; | |||
There was a problem hiding this comment.
would it make sense to put something more descriptive so it is clear this period is different than the one above it?
There was a problem hiding this comment.
This period is a property of dropBeforeByPeriod rule which representing ISO-8601 Periods, they are distinguished by the rule's name. Below is the UI:
I think there is no need to change this period to something others.
| * `type` - this should always be "dropBeforeByPeriod" | ||
| * `period` - A JSON Object representing ISO-8601 Periods | ||
|
|
||
| The interval of a segment will be compared against the specified period. The period is from some time in the past to the current time. The rule matches if the interval before the period. If you just want to retain recent data, you can use this rule to drop the old data that before a specified period. |
There was a problem hiding this comment.
Just so I'm clear, this is an alternative to doing a load by period followed by a drop forever?
Is this intended to be used in cases where the default load rule is generally used, but you want to configure the maximum age on a per-datasource basis? If so can that be added to the docs?
There was a problem hiding this comment.
I get the situation now (from the issue description). Form this description it is not clear what scenarios it is desired to use this rule, can either a link to the issue or more detail on when this is appropriate be added here?
There was a problem hiding this comment.
If you just want to retain recent data, you can use this rule to drop the old data that before a specified period.
Hmm @drcrallen do you think the quote is not enough to explain the desired use case of this rule? Would you please elaborate more on what details you want? Maybe it's better to add like this rule is effectively dropForever + loadByPeriod (includeFuture = true)?
There was a problem hiding this comment.
Maybe like below
If you just want to retain recent data, you can use this rule to drop the old data that
before a specified period and add a loadForever rule to follow it.
Notes, dropBeforeByPeriod + loadForever is equivalent to loadByPeriod(includeFuture = true) + dropForever
?
|
The latest change looks good to me. @drcrallen do you have more comments? |
|
@jihoonson @drcrallen if there are no more comments, I think it can be merged now. Then I'll update #6414 and finally open a PR in druid-console to support #6414 and #6415. |
|
@QiuMM merged. Thanks! |
Add a new drop rule which has been discussed in #5869.