This is an automated email from the ASF dual-hosted git repository. xiangfu0 pushed a commit to branch xiangfu0/data-3221-11-metadata-only-pruning in repository https://gitbox.apache.org/repos/asf/pinot.git
commit 6f1c69123ae827a25b77e154250e2b35e89bd212 Author: Xiang Fu <[email protected]> AuthorDate: Tue Sep 8 16:07:46 2026 -0700 DATA-3221 (13): prune segments from column metadata instead of materializing them `ColumnValueSegmentPruner` asks each segment for a column's data source and then reads nothing from it but the metadata — data type, min/max, partition function. Reaching that metadata through `getDataSource` makes a segment that builds its columns lazily construct the whole index container, every index reader for the column, for a segment it is about to discard. That runs in `ValueBasedSegmentPruner#prune`, a serial loop over every segment the server holds, on the query thread. On an external table under lazy column materialization it puts a Parquet footer parse there too: on a server holding 44,780 segments the pruner alone accounted for the bulk of a query that timed out at 300 s, for a filter that matched 1,839 segments. `IndexSegment#getDataSourceMetadata(String, Schema)` names what the caller actually wants, defaulting to today's behaviour so no implementation has to change. `ImmutableSegmentImpl` answers it from column metadata: `ImmutableDataSourceMetadata` already delegates to `ColumnMetadata` and holds no readers, so it needs nothing built. A column the segment does not have still falls through to the data source, where the schema-driven default and virtual columns are created. The pruner keeps its per-segment data-source cache for mutable segments, whose metadata is not derivable without the data source. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../query/pruner/ColumnValueSegmentPruner.java | 29 ++++++++++++-------- .../immutable/ImmutableSegmentImpl.java | 15 ++++++++++ .../index/datasource/ImmutableDataSource.java | 6 ++++ .../immutable/ImmutableSegmentImplTest.java | 32 ++++++++++++++++++++++ .../org/apache/pinot/segment/spi/IndexSegment.java | 13 +++++++++ 5 files changed, 83 insertions(+), 12 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ColumnValueSegmentPruner.java b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ColumnValueSegmentPruner.java index 5d469ee4c9b..5c69d0eabcd 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ColumnValueSegmentPruner.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ColumnValueSegmentPruner.java @@ -88,10 +88,7 @@ public class ColumnValueSegmentPruner extends ValueBasedSegmentPruner { private boolean pruneEqPredicate(IndexSegment segment, EqPredicate eqPredicate, Map<String, DataSource> dataSourceCache, ValueCache valueCache, QueryContext query) { String column = eqPredicate.getLhs().getIdentifier(); - DataSource dataSource = segment instanceof ImmutableSegment ? segment.getDataSource(column, query.getSchema()) - : dataSourceCache.computeIfAbsent(column, col -> segment.getDataSource(column, query.getSchema())); - assert dataSource != null; - DataSourceMetadata dataSourceMetadata = dataSource.getDataSourceMetadata(); + DataSourceMetadata dataSourceMetadata = getDataSourceMetadata(segment, column, dataSourceCache, query); ValueCache.CachedValue cachedValue = valueCache.get(eqPredicate, dataSourceMetadata.getDataType()); // Check min/max value if (!checkMinMaxRange(dataSourceMetadata, cachedValue.getComparableValue())) { @@ -120,10 +117,7 @@ public class ColumnValueSegmentPruner extends ValueBasedSegmentPruner { return false; } String column = inPredicate.getLhs().getIdentifier(); - DataSource dataSource = segment instanceof ImmutableSegment ? segment.getDataSource(column, query.getSchema()) - : dataSourceCache.computeIfAbsent(column, col -> segment.getDataSource(column, query.getSchema())); - assert dataSource != null; - DataSourceMetadata dataSourceMetadata = dataSource.getDataSourceMetadata(); + DataSourceMetadata dataSourceMetadata = getDataSourceMetadata(segment, column, dataSourceCache, query); List<ValueCache.CachedValue> cachedValues = valueCache.get(inPredicate, dataSourceMetadata.getDataType()); // Check min/max value for (ValueCache.CachedValue value : cachedValues) { @@ -140,10 +134,7 @@ public class ColumnValueSegmentPruner extends ValueBasedSegmentPruner { private boolean pruneRangePredicate(IndexSegment segment, RangePredicate rangePredicate, Map<String, DataSource> dataSourceCache, QueryContext query) { String column = rangePredicate.getLhs().getIdentifier(); - DataSource dataSource = segment instanceof ImmutableSegment ? segment.getDataSource(column, query.getSchema()) - : dataSourceCache.computeIfAbsent(column, col -> segment.getDataSource(column, query.getSchema())); - assert dataSource != null; - DataSourceMetadata dataSourceMetadata = dataSource.getDataSourceMetadata(); + DataSourceMetadata dataSourceMetadata = getDataSourceMetadata(segment, column, dataSourceCache, query); // Get lower/upper boundary value DataType dataType = dataSourceMetadata.getDataType(); @@ -222,4 +213,18 @@ public class ColumnValueSegmentPruner extends ValueBasedSegmentPruner { } return true; } + + /// Pruning reads only the column's statistics, so an immutable segment answers from its column metadata rather + /// than materializing the column. A mutable segment keeps the per-segment data-source cache it had, where the + /// lookup is a map read and the metadata is not derivable without the data source. + private static DataSourceMetadata getDataSourceMetadata(IndexSegment segment, String column, + Map<String, DataSource> dataSourceCache, QueryContext query) { + if (segment instanceof ImmutableSegment) { + return segment.getDataSourceMetadata(column, query.getSchema()); + } + DataSource dataSource = dataSourceCache.computeIfAbsent(column, + col -> segment.getDataSource(col, query.getSchema())); + assert dataSource != null; + return dataSource.getDataSourceMetadata(); + } } diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java index dd4ef193c99..32c87c45e8d 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java +++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java @@ -56,6 +56,7 @@ import org.apache.pinot.segment.spi.ColumnMetadata; import org.apache.pinot.segment.spi.FetchContext; import org.apache.pinot.segment.spi.ImmutableSegment; import org.apache.pinot.segment.spi.datasource.DataSource; +import org.apache.pinot.segment.spi.datasource.DataSourceMetadata; import org.apache.pinot.segment.spi.index.IndexReader; import org.apache.pinot.segment.spi.index.IndexType; import org.apache.pinot.segment.spi.index.StandardIndexes; @@ -451,6 +452,20 @@ public class ImmutableSegmentImpl implements ImmutableSegment { return _segmentMetadata; } + /// Answers from the column metadata, so a caller that needs only the column's statistics does not materialize the + /// column. That matters most under lazy column materialization: segment pruning asks this for every segment the + /// server holds, and building an index container per segment there would put a reader — and, for an external + /// table, a Parquet footer parse — on the query thread for segments that are about to be pruned away. + /// + /// Falls back to the data source for a column the segment does not have, which is where the schema-driven default + /// and virtual columns are created. + @Override + public DataSourceMetadata getDataSourceMetadata(String column, Schema schema) { + ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column); + return columnMetadata != null ? ImmutableDataSource.metadataOf(columnMetadata) + : getDataSource(column, schema).getDataSourceMetadata(); + } + @Override public DataSource getDataSource(String column, Schema schema) { DataSource dataSource = getDataSourceNullable(column); diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/datasource/ImmutableDataSource.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/datasource/ImmutableDataSource.java index 73b39275f22..efb4c822df9 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/datasource/ImmutableDataSource.java +++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/datasource/ImmutableDataSource.java @@ -34,6 +34,12 @@ public class ImmutableDataSource extends BaseDataSource { super(new ImmutableDataSourceMetadata(columnMetadata), columnIndexContainer); } + /// The data-source metadata for a column, without a data source. It delegates to [ColumnMetadata] and holds no + /// index readers, so a caller that needs only the column's statistics can avoid materializing the column. + public static DataSourceMetadata metadataOf(ColumnMetadata columnMetadata) { + return new ImmutableDataSourceMetadata(columnMetadata); + } + /// Exposes the segment's [ColumnMetadata] through the [DataSourceMetadata] view by delegating every accessor. /// Holding a single reference instead of copying the ten fields the view exposes keeps this object at one /// reference per column, which matters for wide segments where every loaded column retains one. The delegation diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImplTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImplTest.java index 21cf674fc29..297f527c2d0 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImplTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImplTest.java @@ -38,6 +38,7 @@ import org.apache.pinot.segment.local.segment.index.openstruct.ImmutableOpenStru import org.apache.pinot.segment.local.segment.virtualcolumn.DocIdVirtualColumnProvider; import org.apache.pinot.segment.spi.ColumnMetadata; import org.apache.pinot.segment.spi.datasource.DataSource; +import org.apache.pinot.segment.spi.datasource.DataSourceMetadata; import org.apache.pinot.segment.spi.index.StandardIndexes; import org.apache.pinot.segment.spi.index.column.ColumnIndexContainer; import org.apache.pinot.segment.spi.index.metadata.ColumnMetadataImpl; @@ -48,6 +49,7 @@ import org.apache.pinot.spi.data.ComplexFieldSpec; import org.apache.pinot.spi.data.DimensionFieldSpec; import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.data.OpenStructNaming; +import org.apache.pinot.spi.data.Schema; import org.apache.pinot.spi.utils.CommonConstants.Segment.BuiltInVirtualColumn; import org.testng.annotations.Test; @@ -161,6 +163,36 @@ public class ImmutableSegmentImplTest { verify(segmentDirectory).close(); } + /// Segment pruning reads a column's statistics for every segment the server holds, to decide which segments can + /// match at all. Reaching those statistics must not materialize the column: under lazy materialization that would + /// build an index container — and for an external table parse a Parquet footer — on the query thread, for a segment + /// that is about to be pruned away. + @Test + public void testDataSourceMetadataDoesNotMaterializeTheColumn() + throws Exception { + ColumnMetadataImpl a = columnMetadata(intColumn("a"), null); + ColumnMetadataImpl b = columnMetadata(intColumn("b"), null); + ColumnMaterializer materializer = mock(ColumnMaterializer.class); + SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); + ImmutableSegmentImpl segment = lazySegment(segmentDirectory, materializer, a, b); + + DataSourceMetadata metadata = segment.getDataSourceMetadata("a", mock(Schema.class)); + assertNotNull(metadata); + assertEquals(metadata.getFieldSpec(), a.getFieldSpec()); + assertEquals(metadata.getDataType(), a.getDataType()); + assertEquals(metadata.getNumDocs(), a.getTotalDocs()); + assertEquals(metadata.isSorted(), a.isSorted()); + // The whole point: reading the statistics built nothing. + verifyNoInteractions(materializer); + + // It agrees with what the materialized data source reports, and only THAT materializes. + assertEquals(segment.getDataSource("a", mock(Schema.class)).getDataSourceMetadata().getDataType(), + metadata.getDataType()); + verify(materializer, times(1)).createIndexContainer(a); + + segment.destroy(); + } + @Test public void testLazyModeMaterializesEachColumnOnceUnderConcurrentAccess() throws Exception { diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/IndexSegment.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/IndexSegment.java index dcc6605f312..281eab07d05 100644 --- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/IndexSegment.java +++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/IndexSegment.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Set; import javax.annotation.Nullable; import org.apache.pinot.segment.spi.datasource.DataSource; +import org.apache.pinot.segment.spi.datasource.DataSourceMetadata; import org.apache.pinot.segment.spi.index.mutable.ThreadSafeMutableRoaringBitmap; import org.apache.pinot.segment.spi.index.reader.TextIndexReader; import org.apache.pinot.segment.spi.index.startree.StarTreeV2; @@ -71,6 +72,18 @@ public interface IndexSegment { /// asked column. DataSource getDataSource(String column, Schema schema); + /// The metadata of a column's data source, for callers that need only the column's statistics — its data type, + /// min/max values, partitioning — and never read its values. + /// + /// Segment pruning is the motivating caller: it reads min/max to decide whether a segment can match at all, for + /// every segment the server holds. Going through [#getDataSource(String, Schema)] to reach that metadata forces an + /// implementation that builds its columns lazily to construct the whole index container — every index reader for + /// the column — for a segment it is about to discard. An implementation that can answer from column metadata alone + /// should override this; the default keeps the existing behaviour. + default DataSourceMetadata getDataSourceMetadata(String column, Schema schema) { + return getDataSource(column, schema).getDataSourceMetadata(); + } + /// Returns a list of star-trees (V2), or null if there is no star-tree (V2) in the segment. @Nullable List<StarTreeV2> getStarTrees(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
