PeriodGranularity wonkiness with late-month origins

[gianm]

See https://groups.google.com/d/topic/druid-user/BiR9p-KJP8Y/discussion.

Seems to be because 2015-03-30 truncates to 2015-02-28T22, which increments to 2015-03-28T22, which is less than 2015-03-30. And the cursor-making stuff assumes that truncate -> next is going to result in something at least as high as what it started with (which is a reasonable assumption imo).

I think the bug is that PeriodGranularity's "next" should not just add the period blindly, it should check to make sure that the incremented result is actually aligned as closely as possible with the origin, and adjust if needed.

Steps to reproduce:

1. Index some data for 2015-03-29/P1D at segmentGranularity=day, queryGranularity=none
2. Do a topN with intervals/granularity like:

{
    "queryType": "topN",
    "dataSource": "xxx",
    "intervals": [
        "2015-03-29/p1d"
    ],
    "granularity": {
        "type": "period",
        "period": "P1M",
"timeZone":"UTC",
        "origin": "2014-11-30T22"
    },
    ... other topN stuff ...
}

historicals will log an exception like this:

java.lang.IllegalArgumentException: The end instant must be greater or equal to the start
        at org.joda.time.base.AbstractInterval.checkInterval(AbstractInterval.java:63) ~[joda-time-2.8.2.jar:2.8.2]
        at org.joda.time.base.BaseInterval.<init>(BaseInterval.java:73) ~[joda-time-2.8.2.jar:2.8.2]
        at org.joda.time.Interval.<init>(Interval.java:82) ~[joda-time-2.8.2.jar:2.8.2]
        at io.druid.segment.QueryableIndexStorageAdapter.makeCursors(QueryableIndexStorageAdapter.java:175) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.topn.TopNQueryEngine.query(TopNQueryEngine.java:75) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.topn.TopNQueryRunnerFactory$1.run(TopNQueryRunnerFactory.java:74) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.ReferenceCountingSegmentQueryRunner.run(ReferenceCountingSegmentQueryRunner.java:49) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.MetricsEmittingQueryRunner$1.accumulate(MetricsEmittingQueryRunner.java:118) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at com.metamx.common.guava.Sequences.toList(Sequences.java:113) ~[java-util-0.27.4.jar:?]
        at io.druid.query.BySegmentQueryRunner.run(BySegmentQueryRunner.java:54) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.MetricsEmittingQueryRunner$1.accumulate(MetricsEmittingQueryRunner.java:118) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.spec.SpecificSegmentQueryRunner$2$1.call(SpecificSegmentQueryRunner.java:85) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.spec.SpecificSegmentQueryRunner.doNamed(SpecificSegmentQueryRunner.java:169) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.spec.SpecificSegmentQueryRunner.access$400(SpecificSegmentQueryRunner.java:39) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.spec.SpecificSegmentQueryRunner$2.doItNamed(SpecificSegmentQueryRunner.java:160) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.spec.SpecificSegmentQueryRunner$2.accumulate(SpecificSegmentQueryRunner.java:78) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.CPUTimeMetricQueryRunner$1.accumulate(CPUTimeMetricQueryRunner.java:83) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at com.metamx.common.guava.Sequences$1.accumulate(Sequences.java:90) ~[java-util-0.27.4.jar:?]
        at com.metamx.common.guava.Sequences.toList(Sequences.java:113) ~[java-util-0.27.4.jar:?]
        at io.druid.query.ChainedExecutionQueryRunner$1$1$1.call(ChainedExecutionQueryRunner.java:130) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at io.druid.query.ChainedExecutionQueryRunner$1$1$1.call(ChainedExecutionQueryRunner.java:120) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) ~[?:1.8.0_66]
        at io.druid.query.PrioritizedExecutorService$PrioritizedListenableFutureTask.run(PrioritizedExecutorService.java:222) ~[druid-processing-0.8.3-iap1.jar:0.8.3-iap1]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) ~[?:1.8.0_66]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) ~[?:1.8.0_66]
        at java.lang.Thread.run(Thread.java:745) [?:1.8.0_66]

[gianm]

0.8.3-iap1 does have some changes to QueryableIndexStorageAdapter, that line is https://github.com/implydata/druid/blob/druid-0.8.3-iap1/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java#L175

i.e. this part

    long minDataTimestamp = getMinTime().getMillis();
    long maxDataTimestamp = getMaxTime().getMillis();
    final Interval dataInterval = new Interval(
        minDataTimestamp,
        gran.next(gran.truncate(maxDataTimestamp)) // <<<<<
    );

[xvrl]

@gianm since you have a fix, do you mind submitting a PR?

[gianm]

@xvrl ah, I don't have a fix, I was just pointing out that the line numbers in the stack trace in my report doesn't match base Druid (we pulled an unrelated patch to that file back from master and that moved some lines around).

[josh-pritchard-fcx]

@gianm This is still an issue any chance this might work its way into a release? This impacts me greatly and I have to go to more work/less efficient queries to work around it. Basically any query using an origin of greater than the 28th of the month binning on monthly period breaks if it's over months that don't contain that day.

For example a query with a granularity spec of

"granularity": { "type": "period", "period": "P1M", "timeZone": "+00:00", "origin": "2017-07-31T00:00:00Z" }

and intervals of ["2017-07-31T00:00:00.000Z/2018-07-31T00:00:00.000Z"] throws the above error because there are months that don't have the 31st in them.

[remizyaka]

Have the same issue with granularity period P1M when origin is at the end of the month. Any chance it will be fixed?

[tarpdalton]

Having the same issue trying to aggregate by monthly periods. I want to offset the periods by timezone offset so that counts are accurate for the timezone.

  "granularity": {
    "type": "period",
    "period": "P1M",
    "timeZone": "UTC",
    "origin": "1970-01-01T00:00:00+01:00"
  },

Is there a work around or another way to solve the problem?

EDIT:

the bug still exists but my problem can mostly be solved by using timeZone in the granularity.

  "granularity": {
    "type": "period",
    "period": "P1M",
    "timeZone": "GB"
  },

[github-actions]

This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

[github-actions]

This issue has been closed due to lack of activity. If you think that
is incorrect, or the issue requires additional review, you can revive the issue at
any time.