abhishekrb19 commented on code in PR #15015:
URL: https://github.com/apache/druid/pull/15015#discussion_r1332666287


##########
server/src/main/java/org/apache/druid/server/coordinator/rules/Rules.java:
##########
@@ -43,4 +48,40 @@ public static boolean eligibleForLoad(Period period, 
Interval interval, DateTime
   private Rules()
   {
   }
+
+  /**
+   * Validate rules. This method throws an exception if a rule contain an 
interval that
+   * will fully cover the next rules' interval in the list. Rules that will be 
evaluated at some point
+   * are considered to be legitimate.
+   * @param rules Datasource rules.
+   */
+  public static void validateRules(final List<Rule> rules)
+  {
+    if (rules == null) {
+      return;
+    }
+    final DateTime now = DateTimes.nowUtc();
+    for (int i = 0; i < rules.size() - 1; i++) {
+      final Rule currRule = rules.get(i);
+      final Rule nextRule = rules.get(i + 1);
+      final Interval currInterval = currRule.getEligibleInterval(now);
+      final Interval nextInterval = nextRule.getEligibleInterval(now);
+      if (currInterval.contains(nextInterval)) {
+        // If the current rule has an eternity interval, it covers everything 
following it.
+        // Or if the current rule still covers the next rule at the current 
interval boundaries, then the
+        // next rule will never fire at any time, so throw an exception.
+        if (Intervals.ETERNITY.equals(currInterval) ||
+            
(currRule.getEligibleInterval(currInterval.getStart()).contains(nextRule.getEligibleInterval(currInterval.getStart()))
+             && 
currRule.getEligibleInterval(currInterval.getEnd()).contains(nextRule.getEligibleInterval(currInterval.getEnd()))))
 {
+          throw InvalidInput.exception(
+              "Rule[%s] has an interval that contains interval for rule[%s]. 
The interval[%s] also covers interval[%s].",
+              currRule,
+              nextRule,
+              currInterval,
+              nextInterval

Review Comment:
   Re the use case above - I'd write the same rules as you've listed in the 
example. The first rule's interval 2020/2021 does _not_ contain the second 
rule's interval 2020/2022. So, I'm not sure why the validation check would flag 
this? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to