austingrant651-create commented on issue #19270: URL: https://github.com/apache/druid/issues/19270#issuecomment-4205329273
Thank you for providing the root cause analysis and the proposed fix for the Timeseries query hang. On Wed, Apr 8, 2026, 4:09 AM gammacomputer ***@***.***> wrote: > *gammacomputer* left a comment (apache/druid#19270) > <https://github.com/apache/druid/issues/19270#issuecomment-4205120455> > > Hello Gianm, > > Below is the fix proposed by the LLM, I hope it can help. > > Also it found that FilteredSegment maybe has the same pattern, this is not > covered in this fix: *FilteredSegment.as() also returns null for > TimeBoundaryInspector. However, a filter could theoretically narrow the > effective time range, so delegating the inspector there is less > straightforward -- the delegate's boundaries would be an upper bound, not > exact. This might still be acceptable (an overestimate is better than no > clipping at all), but it's a separate discussion.* > > Root Cause > > UnnestSegment.as(TimeBoundaryInspector.class) returns null. When > TimeseriesQueryRunnerFactory creates a runner (line 65 of > TimeseriesQueryRunnerFactory.java), it calls segment.as > (TimeBoundaryInspector.class) > to get the segment's time boundaries. Since UnnestSegment returns null for > this, CursorGranularizer.create() cannot clip the query interval. With a > wide interval (like ETERNITY, which is the default when no > WHERE __time filter is provided), granularity.getIterable() generates > billions of buckets for non-ALL granularities, and the engine spends all > its time iterating over empty buckets. > > Fix from commit 8a81601 > <https://github.com/apache/druid/commit/8a8160157172e14a3052597e2cff4950d8ede245> > (origin/main currently) > > The fix is a single addition to UnnestSegment.as() in > processing/src/main/java/org/apache/druid/segment/UnnestSegment.java: > > diff --git a/processing/src/main/java/org/apache/druid/segment/UnnestSegment.java b/processing/src/main/java/org/apache/druid/segment/UnnestSegment.java > index a82e60ab8b..5ae214d30c 100644 > --- a/processing/src/main/java/org/apache/druid/segment/UnnestSegment.java > +++ b/processing/src/main/java/org/apache/druid/segment/UnnestSegment.java > @@ -49,6 +49,9 @@ public class UnnestSegment extends WrappedSegment > return (T) new UnnestCursorFactory(delegate.as(CursorFactory.class), unnestColumn, filter); > } else if (TopNOptimizationInspector.class.equals(clazz)) { > return (T) new SimpleTopNOptimizationInspector(filter == null); > + } else if (TimeBoundaryInspector.class.equals(clazz)) { > + // Unnest does not alter the __time column, so the delegate's time boundaries are still valid. > + return delegate.as(clazz); > } > return null; > } > > This delegates TimeBoundaryInspector to the underlying segment. This is > safe because unnest does not modify the __time column -- it only expands > array values into additional rows that share the same > timestamp as the original row. With this change, CursorGranularizer clips > the interval to the actual data range, and the bucket iteration covers only > the relevant time span. > > Test > > Added UnnestTimeseriesQueryRunnerTest in > processing/src/test/java/org/apache/druid/query/timeseries/. The test runs > a timeseries query with DAY granularity over an unnest segment using the > ETERNITY interval. > It has a @test <https://github.com/test>(timeout = 10000) annotation so > it fails in 10 seconds if the fix is reverted, instead of hanging. The test > verifies that 2 rows across 2 days produce correct per-day counts (3 and 2 > unnested > values respectively). > > Testing > > Without the fix (fix temporarily reverted, test kept): > > docker run --rm -v "$(pwd)":/src -v druid-m2-cache:/root/.m2 -w /src \ > maven:3.9-eclipse-temurin-21 bash -c \ > "git config --global --add safe.directory /src && \ > mvn test -pl processing \ > -Dtest='UnnestTimeseriesQueryRunnerTest' \ > -DfailIfNoTests=false -B" > > Result: Tests run: 1, Failures: 0, Errors: 1, Skipped: 0 -- BUILD FAILURE > (timeout after 10 seconds, confirming the bug) > > With the fix (all unnest-related tests): > > docker run --rm -v "$(pwd)":/src -v druid-m2-cache:/root/.m2 -w /src \ > maven:3.9-eclipse-temurin-21 bash -c \ > "git config --global --add safe.directory /src && \ > mvn test -pl processing \ > -Dtest='UnnestCursorFactoryTest,UnnestColumnValueSelectorCursorTest,UnnestGroupByQueryRunnerTest,UnnestScanQueryRunnerTest,UnnestTopNQueryRunnerTest,UnnestTimeseriesQueryRunnerTest' \ > -DfailIfNoTests=false -B" > > Result: Tests run: 71, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS > > Here is the test file at > processing/src/test/java/org/apache/druid/query/timeseries/UnnestTimeseriesQueryRunnerTest.java > > /* > * Licensed to the Apache Software Foundation (ASF) under one > * or more contributor license agreements. See the NOTICE file > * distributed with this work for additional information > * regarding copyright ownership. The ASF licenses this file > * to you under the Apache License, Version 2.0 (the > * "License"); you may not use this file except in compliance > * with the License. You may obtain a copy of the License at > * > * http://www.apache.org/licenses/LICENSE-2.0 > * > * Unless required by applicable law or agreed to in writing, > * software distributed under the License is distributed on an > * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > * KIND, either express or implied. See the License for the > * specific language governing permissions and limitations > * under the License. > */ > > package org.apache.druid.query.timeseries; > > import com.google.common.collect.ImmutableList; > import com.google.common.collect.ImmutableMap; > import org.apache.druid.data.input.MapBasedInputRow; > import org.apache.druid.java.util.common.DateTimes; > import org.apache.druid.java.util.common.Intervals; > import org.apache.druid.java.util.common.granularity.Granularities; > import org.apache.druid.query.Druids; > import org.apache.druid.query.FinalizeResultsQueryRunner; > import org.apache.druid.query.QueryPlus; > import org.apache.druid.query.QueryRunner; > import org.apache.druid.query.QueryRunnerFactory; > import org.apache.druid.query.QueryRunnerTestHelper; > import org.apache.druid.query.Result; > import org.apache.druid.query.TableDataSource; > import org.apache.druid.query.UnnestDataSource; > import org.apache.druid.query.aggregation.CountAggregatorFactory; > import org.apache.druid.query.expression.TestExprMacroTable; > import org.apache.druid.segment.IncrementalIndexSegment; > import org.apache.druid.segment.Segment; > import org.apache.druid.segment.UnnestSegment; > import org.apache.druid.segment.column.ColumnType; > import org.apache.druid.segment.incremental.IncrementalIndex; > import org.apache.druid.segment.incremental.IncrementalIndexSchema; > import org.apache.druid.segment.incremental.OnheapIncrementalIndex; > import org.apache.druid.segment.virtual.ExpressionVirtualColumn; > import org.apache.druid.timeline.SegmentId; > import org.junit.Assert; > import org.junit.Test; > > import java.util.Arrays; > import java.util.List; > > /** > * Tests for timeseries queries over unnest datasources. > * Specifically verifies that timeseries queries with non-ALL granularity > * complete in reasonable time when the query interval is wide (e.g., ETERNITY), > * which requires proper TimeBoundaryInspector propagation through UnnestSegment. > */ > public class UnnestTimeseriesQueryRunnerTest > { > @Test(timeout = 10000) > public void testTimeseriesOverUnnestWithDayGranularityAndEternityInterval() throws Exception > { > final IncrementalIndex index = new OnheapIncrementalIndex.Builder() > .setIndexSchema( > new IncrementalIndexSchema.Builder() > .withMinTimestamp(DateTimes.of("2024-01-01T00:00:00Z").getMillis()) > .build() > ) > .setMaxRowCount(1000) > .build(); > > index.add( > new MapBasedInputRow( > DateTimes.of("2024-01-01T00:00:00Z").getMillis(), > ImmutableList.of("dim1"), > ImmutableMap.of("dim1", Arrays.asList("a", "b", "c")) > ) > ); > > index.add( > new MapBasedInputRow( > DateTimes.of("2024-01-02T00:00:00Z").getMillis(), > ImmutableList.of("dim1"), > ImmutableMap.of("dim1", Arrays.asList("d", "e")) > ) > ); > > final ExpressionVirtualColumn unnestColumn = new ExpressionVirtualColumn( > "unnested", > "\"dim1\"", > ColumnType.STRING, > TestExprMacroTable.INSTANCE > ); > > final Segment baseSegment = new IncrementalIndexSegment(index, SegmentId.dummy("ds")); > final Segment unnestSegment = new UnnestSegment(baseSegment, unnestColumn, null); > > final QueryRunnerFactory factory = new TimeseriesQueryRunnerFactory( > new TimeseriesQueryQueryToolChest(), > new TimeseriesQueryEngine(), > QueryRunnerTestHelper.NOOP_QUERYWATCHER > ); > > final QueryRunner<Result<TimeseriesResultValue>> runner = > new FinalizeResultsQueryRunner<>(factory.createRunner(unnestSegment), factory.getToolchest()); > > // Use ETERNITY interval with DAY granularity. Without the fix (UnnestSegment delegating > // TimeBoundaryInspector), CursorGranularizer cannot clip the interval, causing it to > // iterate over billions of empty day-sized buckets and effectively hang. > TimeseriesQuery query = Druids.newTimeseriesQueryBuilder() > .dataSource( > UnnestDataSource.create( > new TableDataSource("ds"), > unnestColumn, > null > ) > ) > .granularity(Granularities.DAY) > .intervals(ImmutableList.of(Intervals.ETERNITY)) > .aggregators(new CountAggregatorFactory("rows")) > .context(ImmutableMap.of(TimeseriesQuery.SKIP_EMPTY_BUCKETS, true)) > .build(); > > List<Result<TimeseriesResultValue>> results = runner.run(QueryPlus.wrap(query)).toList(); > > Assert.assertEquals("result size", 2, results.size()); > Assert.assertEquals("day 1 timestamp", DateTimes.of("2024-01-01"), results.get(0).getTimestamp()); > Assert.assertEquals("day 1 count", 3L, (long) results.get(0).getValue().getLongMetric("rows")); > Assert.assertEquals("day 2 timestamp", DateTimes.of("2024-01-02"), results.get(1).getTimestamp()); > Assert.assertEquals("day 2 count", 2L, (long) results.get(1).getValue().getLongMetric("rows")); > } > } > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/druid/issues/19270#issuecomment-4205120455>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/B74AFF5BAP5QG36HRME3YTT4UYJM5AVCNFSM6AAAAACXPT7LP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEMBVGEZDANBVGU> > . > You are receiving this because you are subscribed to this thread.Message > ID: ***@***.***> > -- 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]
