Repository navigation
Period load rules should include the future by default #5869
Description
We have seen problem scenarios like the following:
- A user has load rules like: load 30 days, drop forever.
- The user loads some data from slightly in the future (maybe some clocks are running a bit fast or slow) using streaming ingestion. This creates a segment with an interval that is in the future.
- The coordinator disables the segment immediately upon noticing it (since it is not within the last 30 days).
- The Kafka tasks time out during handoff (because the segments are never loaded).
- And after that timeout, the data that was slightly in the future is still not available!
It would be better, I think, if period-based load rules included an unbounded time into the future too. I bet this lines up better with users' intuitive idea of what "retain 30 days" means. Of course it could be changeable with some optional parameters (or a different kind of rule?) but I think the default behavior should be the intuitive one.
NB: A workaround to this is to use rules like "load 30 days" followed by "drop 1000 years" followed by "load forever". That has the same effect as what I am suggesting for the new default behavior: we'll drop anything older than 30 days, but retain the last 30 days and also anything that is in the future.
Related, but different: #5868.
Activity
gianm commentedon Jul 1, 2018
Even though this isn't directly related to Kafka indexing, I gave it a Kafka tag since that's the kind of indexing most likely to be affected.
teeram commentedon Jul 4, 2018
Nice idea @gianm. Just wanted to mention that perhaps the
lateMessageRejectionPeriodandearlyMessageRejectionPeriodfound in theKafkaSupervisorIOConfigcould go away in favor of new load rules. Up to you guys of course, I just thought it might end up being a little confusing to have these configurable supervisor fields separate from load rules.gianm commentedon Jul 5, 2018
Hmm, I think there is still a use case for lateMessageRejectionPeriod / earlyMessageRejectionPeriod even if load / drop rule usage becomes more intelligent. Imagine having a 90 day load rule, but only wanting to load stream data if it's received within 24 hours. It could be useful if you need to do batch reprocessing on older data and don't want to worry about locks.
teeram commentedon Jul 6, 2018
Ah true, that's a good point.
QiuMM commentedon Sep 27, 2018
@gianm maybe add a new drop rule which will drop data older than a configured date, then if people we want to retain the last 30 days data, they can do like this: drop 30 days ago, load forever.
jihoonson commentedon Sep 28, 2018
@QiuMM would you elaborate more on your idea? I'm not sure how's it different from the
PeriodDropRule. Also, how does it fix this issue?jihoonson commentedon Sep 28, 2018
Is anyone working on this? If not, I think we can move this to 0.13.1.
QiuMM commentedon Sep 28, 2018
@jihoonson currently, we can edit a
PeriodDropRulelike this:{ "type" : "dropByPeriod", "period" : "P1D" }The period is from some time in the past to the current time, so this example will drop last one day data. I think
PeriodDropRuleis a impractical rule for it will always drop recent data. So people who want to retain the last 30 days data, they can not use thisPeriodDropRulebut have to do like this: load 30 days, drop forever. Then because ofdrop forevercause issues that @gianm have listed above.Maybe we can remove the
PeriodDropRulebecause I think few people would like to use it. Then add a new drop rule which will drop data older than a configured date. I also want a rule like this.jihoonson commentedon Sep 28, 2018
@QiuMM thank you for the explanation. Now I get it.
But, I'm afraid that it can introduce another issue. The purpose of load/drop rule is to control data retention which means how many data historicals should store locally. And as @gianm said, this problem mostly happens in stream ingestion which data size grows as time goes by. If dropRule works based on a fixed date, people might update the date for the dropRule periodically.
QiuMM commentedon Sep 29, 2018
@jihoonson I'm not sure why you think people need to update the date? If I just want to keep last 1 day data, then set the drop rule like this:
drop 1 day agoand no need to change it as time goes by,1 day agodata will always be dropped. Besides,1 day agofor current hour is different from1 day agofor next hour, the coordinator should realize this and operate automatically just like the current approach forLoadPeriodRule.jihoonson commentedon Sep 29, 2018
Ah, now I really get it. I thought you want to set a specific date for dropRule. Your idea makes sense to me. Probably it's easier to use than the combination of the current loadRule and dropRule.
QiuMM commentedon Sep 29, 2018
@jihoonson yes, that right. And by adding such drop rule, I think we don't have to change the current
LoadPeriodRule. I can help to add such drop rule for I also want it.jihoonson commentedon Sep 29, 2018
@QiuMM thanks! I think it makes more sense to add new rules like
LoadSinceorDropBeforerather than modifying existing rules. They would be more intuitive and easier to use than combining loadRules and dropRules to get the same effect. But I don't know what was the rationale for the current design of the load/drop rules. @gianm any idea?QiuMM commentedon Sep 29, 2018
In conclusion, I think current design of the load/drop rules is flexible and able to deal with almost all scenarios. But current
PeriodDropRuleis a impractical rule for it will always drop recent data. Then if people want toretain 30 daysdata, they can not use suchPeriodDropRulebut have to do like: load 30 days, drop forever. And because people have used thedrop foreverrule which would drop segments in the future, then below things occured:Then I think there are two ways to solve these things:
PeriodDropRuleto supportdrop before a period, then if people want toretain 30 daysdata, they can do like this: drop 30 days before, load forever.I prefer the second way and want to modify current
PeriodDropRulenot add a new one because the current one is very impractical, IMO no people would like to use such drop rule.jihoonson commentedon Sep 29, 2018
Yeah, I think it was the rationale of the current design, but it doesn't explain why
PeriodLoadRuledoesn't consider future data orPeriodDropRuleis designed in such a way. ForPeriodLoadRule, I think we didn't consider future data at that time. ForPeriodDropRule, I have no idea.I would say it's better to add a new rule because it's much easier. Modifying an existing rule in an incompatible way would take a long time, because we need to discuss about it and check its use cases as many as possible. Please note that modifying
PeriodLoadRuleto include future data is compatible with the current behavior. IfPeriodDropRuleis really not useful, we can deprecate and remove it in the future.But, I know there is a production use case that
PeriodDropRulemight be useful. They are ingesting data from a single Kafka stream into two different Druid dataSources. One stores the recent data of, let's say last month while another stores old data. In such cases, they can usePeriodDropRulefor the old dataSource to drop last month data to avoid the potential duplicate data issue.So, I would say adding new rules is more welcomed and having the current
PeriodDropRulewould be fine if there's no real problem. What do you think?QiuMM commentedon Sep 30, 2018
It's fine for me to keep current
PeriodDropRule. I'll open a PR to add such drop rule in a few days.jihoonson commentedon Sep 30, 2018
@QiuMM thanks!