kfaraz commented on code in PR #15015:
URL: https://github.com/apache/druid/pull/15015#discussion_r1330986915
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodDropBeforeRule.java:
##########
@@ -63,4 +64,18 @@ public boolean appliesTo(Interval theInterval, DateTime
referenceTimestamp)
final DateTime periodAgo = referenceTimestamp.minus(period);
return theInterval.getEndMillis() <= periodAgo.getMillis();
}
+
+ @Override
+ public Interval getEligibleInterval(DateTime referenceTimestamp)
+ {
+ return new Interval(DateTimes.utc(Long.MIN_VALUE),
referenceTimestamp.minus(period));
Review Comment:
Maybe use `JodaUtils.MIN_INSTANT` instead of `Long.MIN_VALUE` for alignment
with `Intervals.ETERNITY`.
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/Rule.java:
##########
@@ -50,4 +50,12 @@ public interface Rule
boolean appliesTo(Interval interval, DateTime referenceTimestamp);
void run(DataSegment segment, SegmentActionHandler segmentHandler);
+
+ /**
+ * Return an eligible interval from the reference timestamp. Implemntations
Review Comment:
Typo:
```suggestion
* Return an eligible interval from the reference timestamp.
Implementations
```
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/Rule.java:
##########
@@ -50,4 +50,11 @@ public interface Rule
boolean appliesTo(Interval interval, DateTime referenceTimestamp);
void run(DataSegment segment, SegmentActionHandler segmentHandler);
+
+ /**
+ * Return an eligible interval from the reference timestamp. Implementations
+ * must return a valid interval based on the rule type.
+ * @param referenceTimestamp base timestamp
+ */
Review Comment:
```suggestion
/**
* Returns the interval eligible for this rule. The interval is computed
based on the
* {@code referenceTimestamp}, which is typically the current time when
the rule is
* being evaluated.
*/
```
##########
server/src/test/java/org/apache/druid/server/http/RulesResourceTest.java:
##########
@@ -151,6 +166,286 @@ public void testGetDatasourceRuleHistoryWithWrongCount()
EasyMock.verify(auditManager);
}
+ @Test
+ public void testSetDatasourceRulesWithEffectivelyNoRule()
+ {
+ EasyMock.expect(databaseRuleManager.overrideRule(EasyMock.anyObject(),
EasyMock.anyObject(), EasyMock.anyObject()))
+ .andReturn(true).times(2);
+ EasyMock.replay(databaseRuleManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final Response resp1 = rulesResource.setDatasourceRules("dataSource1",
null, null, null, EasyMock.createMock(HttpServletRequest.class));
+ Assert.assertEquals(200, resp1.getStatus());
+
+ final Response resp2 = rulesResource.setDatasourceRules("dataSource1", new
ArrayList<>(), null, null, EasyMock.createMock(HttpServletRequest.class));
+ Assert.assertEquals(200, resp2.getStatus());
+ EasyMock.verify(databaseRuleManager);
+ }
+
+ @Test
+ public void testSetDatasourceRulesWithInvalidLoadRules()
+ {
+ EasyMock.replay(auditManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final HttpServletRequest req =
EasyMock.createMock(HttpServletRequest.class);
+
+ final IntervalLoadRule loadInterval1 = new IntervalLoadRule(
+ Intervals.of("2023-07-29T01:00:00Z/2023-12-30T01:00:00Z"),
+ null,
+ null
+ );
+ final IntervalLoadRule loadInterval2 = new IntervalLoadRule(
+ Intervals.of("2023-09-29T01:00:00Z/2023-10-30T01:00:00Z"),
+ null,
+ null
+ );
+ final PeriodLoadRule loadPT1H = new PeriodLoadRule(new Period("PT1H"),
null, null, null);
+ final PeriodLoadRule loadP3M = new PeriodLoadRule(new Period("P3M"), null,
null, null);
+ final PeriodLoadRule loadP6M = new PeriodLoadRule(new Period("P6M"), null,
null, null);
+ final ForeverLoadRule loadForever = new ForeverLoadRule(null, null);
+
+ final List<Rule> rules = new ArrayList<>();
+ rules.add(loadP6M);
+ rules.add(loadP3M);
+ rules.add(loadForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadP6M, loadP3M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadP3M);
+ rules.add(loadForever);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadP6M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadForever);
+ rules.add(loadPT1H);
+ rules.add(loadInterval2);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadPT1H)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadPT1H);
+ rules.add(loadPT1H);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadPT1H, loadPT1H)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadInterval1);
+ rules.add(loadInterval2);
+ rules.add(loadP6M);
+ rules.add(loadForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadInterval1, loadInterval2)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadP6M);
+ rules.add(loadInterval1);
+ rules.add(loadForever);
+ rules.add(loadInterval2);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadInterval2)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+ }
+
+ @Test
+ public void testDatasourceRulesWithInvalidDropRules()
+ {
+ EasyMock.replay(auditManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final HttpServletRequest req =
EasyMock.createMock(HttpServletRequest.class);
+
+ final PeriodDropBeforeRule dropBeforeP3M = new PeriodDropBeforeRule(new
Period("P3M"));
+ final PeriodDropBeforeRule dropBeforeP6M = new PeriodDropBeforeRule(new
Period("P6M"));
+ final PeriodDropRule dropByP1M = new PeriodDropRule(new Period("P1M"),
true);
+ final PeriodDropRule dropByP2M = new PeriodDropRule(new Period("P2M"),
true);
+ final ForeverDropRule dropForever = new ForeverDropRule();
+
+ final List<Rule> rules = new ArrayList<>();
+ rules.add(dropBeforeP3M);
+ rules.add(dropBeforeP6M);
+ rules.add(dropByP1M);
+ rules.add(dropForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropBeforeP3M, dropBeforeP6M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(dropByP2M);
+ rules.add(dropByP1M);
+ rules.add(dropForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropByP2M, dropByP1M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(dropForever);
+ rules.add(dropByP1M);
+ rules.add(dropByP2M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropForever, dropByP1M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+ }
+
+ @Test
+ public void testDatasourceRulesWithInvalidBroadcastRules()
+ {
+ EasyMock.replay(auditManager);
Review Comment:
Testing Suggestion:
It might be simpler to add a `RulesTest` class and validate all the possible
scenarios there rather than validating all scenarios against `RulesResource`
itself.
Only a couple of cases validated against `RulesResource` should be enough, I
think.
Whether the tests are kept in `RulesTest` or `RulesResourceTest`, the
duplicate logic in the tests should be commoned out such that each test case
reads something like:
```
Test Case 1. A list of rules A, B, C is valid.
Test Case 2. A list of rules D, E, F throws so and so exception.
```
and so on.
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodBroadcastDistributionRule.java:
##########
@@ -65,6 +65,13 @@ public boolean appliesTo(Interval interval, DateTime
referenceTimestamp)
return Rules.eligibleForLoad(period, interval, referenceTimestamp,
includeFuture);
}
+ @Override
+ public Interval getEligibleInterval(DateTime referenceTimestamp)
+ {
+ return includeFuture ? new Interval(referenceTimestamp.minus(period),
referenceTimestamp.plus(period))
Review Comment:
When `includeFuture` is true, we include everything in the future, not just
upto the `period`.
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodDropRule.java:
##########
@@ -80,4 +80,19 @@ public boolean appliesTo(Interval theInterval, DateTime
referenceTimestamp)
return currInterval.contains(theInterval);
}
}
+
+ @Override
+ public Interval getEligibleInterval(DateTime referenceTimestamp)
+ {
+ return new Interval(referenceTimestamp.minus(period), referenceTimestamp);
Review Comment:
```suggestion
return new Interval(period, referenceTimestamp);
```
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java:
##########
@@ -85,6 +83,12 @@ public boolean appliesTo(Interval interval, DateTime
referenceTimestamp)
return Rules.eligibleForLoad(period, interval, referenceTimestamp,
includeFuture);
}
+ @Override
+ public Interval getEligibleInterval(DateTime referenceTimestamp)
+ {
+ return new Interval(referenceTimestamp.minus(period), referenceTimestamp);
Review Comment:
```suggestion
return new Interval(period, referenceTimestamp);
```
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodDropRule.java:
##########
@@ -80,4 +81,20 @@ public boolean appliesTo(Interval theInterval, DateTime
referenceTimestamp)
return currInterval.contains(theInterval);
}
}
+
+ @Override
+ public Interval getEligibleInterval(DateTime referenceTimestamp)
+ {
+ return includeFuture ? new Interval(referenceTimestamp.minus(period),
referenceTimestamp.plus(period))
Review Comment:
Same comment about `includeFuture`.
##########
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.
+ */
Review Comment:
Rephrased a bit, removed the `@param` tag as it is not adding any new info,
add `@throws` tag.
```suggestion
/**
* Validates the given list of retention rules for a datasource (or
cluster default).
* A rule is considered valid only if:
* <ul>
* <li>It will be evaluated at some point, i.e. its eligible interval is
not fully covered by the eligible interval of any preceding rule in the
list.</li>
* </ul>
* @throws DruidException with error code "invalidInput" if any of the
given rules is not valid.
*/
```
##########
server/src/test/java/org/apache/druid/server/http/RulesResourceTest.java:
##########
@@ -151,6 +166,286 @@ public void testGetDatasourceRuleHistoryWithWrongCount()
EasyMock.verify(auditManager);
}
+ @Test
+ public void testSetDatasourceRulesWithEffectivelyNoRule()
+ {
+ EasyMock.expect(databaseRuleManager.overrideRule(EasyMock.anyObject(),
EasyMock.anyObject(), EasyMock.anyObject()))
+ .andReturn(true).times(2);
+ EasyMock.replay(databaseRuleManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final Response resp1 = rulesResource.setDatasourceRules("dataSource1",
null, null, null, EasyMock.createMock(HttpServletRequest.class));
+ Assert.assertEquals(200, resp1.getStatus());
+
+ final Response resp2 = rulesResource.setDatasourceRules("dataSource1", new
ArrayList<>(), null, null, EasyMock.createMock(HttpServletRequest.class));
+ Assert.assertEquals(200, resp2.getStatus());
+ EasyMock.verify(databaseRuleManager);
+ }
+
+ @Test
+ public void testSetDatasourceRulesWithInvalidLoadRules()
+ {
+ EasyMock.replay(auditManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final HttpServletRequest req =
EasyMock.createMock(HttpServletRequest.class);
+
+ final IntervalLoadRule loadInterval1 = new IntervalLoadRule(
+ Intervals.of("2023-07-29T01:00:00Z/2023-12-30T01:00:00Z"),
+ null,
+ null
+ );
+ final IntervalLoadRule loadInterval2 = new IntervalLoadRule(
+ Intervals.of("2023-09-29T01:00:00Z/2023-10-30T01:00:00Z"),
+ null,
+ null
+ );
+ final PeriodLoadRule loadPT1H = new PeriodLoadRule(new Period("PT1H"),
null, null, null);
+ final PeriodLoadRule loadP3M = new PeriodLoadRule(new Period("P3M"), null,
null, null);
+ final PeriodLoadRule loadP6M = new PeriodLoadRule(new Period("P6M"), null,
null, null);
+ final ForeverLoadRule loadForever = new ForeverLoadRule(null, null);
+
+ final List<Rule> rules = new ArrayList<>();
+ rules.add(loadP6M);
+ rules.add(loadP3M);
+ rules.add(loadForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadP6M, loadP3M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadP3M);
+ rules.add(loadForever);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadP6M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadForever);
+ rules.add(loadPT1H);
+ rules.add(loadInterval2);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadPT1H)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadPT1H);
+ rules.add(loadPT1H);
+ rules.add(loadP6M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadPT1H, loadPT1H)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadInterval1);
+ rules.add(loadInterval2);
+ rules.add(loadP6M);
+ rules.add(loadForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadInterval1, loadInterval2)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(loadP6M);
+ rules.add(loadInterval1);
+ rules.add(loadForever);
+ rules.add(loadInterval2);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", loadForever, loadInterval2)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+ }
+
+ @Test
+ public void testDatasourceRulesWithInvalidDropRules()
+ {
+ EasyMock.replay(auditManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final HttpServletRequest req =
EasyMock.createMock(HttpServletRequest.class);
+
+ final PeriodDropBeforeRule dropBeforeP3M = new PeriodDropBeforeRule(new
Period("P3M"));
+ final PeriodDropBeforeRule dropBeforeP6M = new PeriodDropBeforeRule(new
Period("P6M"));
+ final PeriodDropRule dropByP1M = new PeriodDropRule(new Period("P1M"),
true);
+ final PeriodDropRule dropByP2M = new PeriodDropRule(new Period("P2M"),
true);
+ final ForeverDropRule dropForever = new ForeverDropRule();
+
+ final List<Rule> rules = new ArrayList<>();
+ rules.add(dropBeforeP3M);
+ rules.add(dropBeforeP6M);
+ rules.add(dropByP1M);
+ rules.add(dropForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropBeforeP3M, dropBeforeP6M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(dropByP2M);
+ rules.add(dropByP1M);
+ rules.add(dropForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropByP2M, dropByP1M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
+ rules.add(dropForever);
+ rules.add(dropByP1M);
+ rules.add(dropByP2M);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", dropForever, dropByP1M)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+ }
+
+ @Test
+ public void testDatasourceRulesWithInvalidBroadcastRules()
+ {
+ EasyMock.replay(auditManager);
+
+ final RulesResource rulesResource = new RulesResource(databaseRuleManager,
auditManager);
+ final HttpServletRequest req =
EasyMock.createMock(HttpServletRequest.class);
+
+ final ForeverBroadcastDistributionRule broadcastForever = new
ForeverBroadcastDistributionRule();
+ final PeriodBroadcastDistributionRule broadcastPeriodPT1H = new
PeriodBroadcastDistributionRule(new Period("PT1H"), true);
+ final PeriodBroadcastDistributionRule broadcastPeriodPT2H = new
PeriodBroadcastDistributionRule(new Period("PT2H"), true);
+ final IntervalBroadcastDistributionRule broadcastInterval1 = new
IntervalBroadcastDistributionRule(
+ Intervals.of("2000-09-29T01:00:00Z/2050-10-30T01:00:00Z")
+ );
+ final IntervalBroadcastDistributionRule broadcastInterval2 = new
IntervalBroadcastDistributionRule(
+ Intervals.of("2010-09-29T01:00:00Z/2020-10-30T01:00:00Z")
+ );
+
+ final List<Rule> rules = new ArrayList<>();
+ rules.add(broadcastInterval1);
+ rules.add(broadcastInterval2);
+ rules.add(broadcastForever);
+
+ DruidExceptionMatcher.invalidInput().expectMessageContains(
+ StringUtils.format("Rule[%s] has an interval that contains interval
for rule[%s].", broadcastInterval1, broadcastInterval2)
+ ).assertThrowsAndMatches(() ->
rulesResource.setDatasourceRules("dataSource1", rules, null, null, req));
+
+ rules.clear();
Review Comment:
It would be nicer to have this in a separate test case rather than clearing
the list of rules. See other comment.
##########
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:
Yes, I agree. Rule #3 would never be applied on any segment irrespective of
the rule types.
While evaluating retention rules, as soon as a rule applies to a segment, we
break and don't look at the other rules.
Also, in general, I am not entirely sure of the validation logic here.
- For one, the set thing suggested by @LakshSingla makes sense. But we would
have to ensure that the `fully covered` check is done correctly.
- Additive logic (cleaner to implement): Say your set contains intervals
`A [4, 10)` and `B [15, 20)` and a new interval `C [8, 17)` comes in. Adding
this new interval C should merge all of A, B and C into one interval `[4, 17)`.
- Subtractive logic: To check if an interval has any part which is not
fully covered by the preceding intervals, remove all parts of it that overlap
with intervals already encountered. If you are left with an empty interval, the
rule is not valid.
- Secondly, all of this is easy to follow when we are dealing with
`ForeverXyzRule` or `IntervalXyzRule` (rule with fixed interval). For
`PeriodXyzRule`, I am not entirely sure of the boundaries that would need to be
checked. The proposed change does compute the `eligibleInterval` at both start
and end of the `currInterval` but I need to give it some more thought.
- There could even be other types of rules in the future, but I guess we
need not worry about them now.
--
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]
