Support range index on ingestion-aggregated no-dictionary columns - #19111
Support range index on ingestion-aggregated no-dictionary columns#19111raghavyadav01 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19111 +/- ##
============================================
- Coverage 65.70% 65.67% -0.04%
Complexity 1423 1423
============================================
Files 3439 3439
Lines 218064 218133 +69
Branches 34679 34693 +14
============================================
- Hits 143289 143249 -40
- Misses 63226 63323 +97
- Partials 11549 11561 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Jackie-Jiang
left a comment
There was a problem hiding this comment.
This is not the correct fix. We should allow building range index, and scan the column to figure out min/max value when they are unavailable.
|
Thanks @Jackie-Jiang . I am thinking to compute min/max on demand in MutableNoDictColumnStatistics — when the mutable segment reports null (aggregated columns skip min/max tracking since the value mutates), scan the sealed forward index once (same pass isSorted() already does), before the BitSliced creator is constructed. This adds one extra O(numDocs) scan per such column at seal time. Will this be OK? |
Yes. That is the overhead we need to pay in order to add range index. We should also check how RangeIndexHandler handles null min/max value. |
Ingestion-time metrics aggregation forces its aggregated metric columns to be no-dictionary and skips min/max tracking during consumption (the values mutate in place). The BitSliced range index (version 2) creator subtracts the column min for INT/LONG columns, so with a null value domain segment commit failed with an NPE. Instead of rejecting the combination, recover the value domain: - MutableNoDictColumnStatistics computes min/max with a single scan of the sealed forward index when the mutable segment reports null, scoped to single-value INT/LONG (the only types whose BitSliced index reads min/max; FLOAT/DOUBLE use the full floating-point ordinal domain). The recovered values also flow into the committed segment metadata, improving pruning. - RangeIndexHandler recomputes min/max from the forward index on the index-reload path for segments committed before this change, via a new min/max override on IndexCreationContext.Builder. Adds unit coverage for min/max recovery and an end-to-end seal-path test that builds and verifies a range index on an aggregated no-dictionary column, and flips the validation tests to assert the combination is now accepted.
690e9ba to
542f75a
Compare
Problem
Configuring a BitSliced (version 2) range index on an ingestion-aggregated metric column causes segments to fail to commit.
Ingestion-time metrics aggregation (
aggregationConfigs, or the legacyaggregateMetricsflag) forces its aggregated metric columns to be no-dictionary and skips min/max tracking while the consuming segment is being built — the values mutate in place, so a running min/max would be meaningless (MutableSegmentImpldeliberately skips it). At segment commit time min/max are thereforenull, and the BitSliced range index creator reads them for a single-value no-dictionary column:For INT/LONG columns this throws a
NullPointerExceptionat segment build time (((Number) minValue).longValue()), so the affected real-time segments can never commit.Fix
Rather than reject the combination, recover the value domain at build time:
MutableNoDictColumnStatistics— when the mutable segment reportsnullmin/max, compute them with a single scan of the sealed forward index. Scoped to single-value INT/LONG, the only stored types whose BitSliced range index reads min/max (FLOAT/DOUBLE use the full floating-point ordinal domain and never dereference min/max). The recovered values also flow into the committed segment metadata via the existing metadata path, so aggregated columns now get accurate segment-level min/max (a pruning improvement).RangeIndexHandler— the index-reload path recomputes min/max from the forward index for segments that were committed before this change (whose metadata min/max are stillnull), via a new min/max override onIndexCreationContext.Builder. The recovered domain is written into the range index header (which the reader uses for the subtract-min); for such legacy segments the reader still reads a null metadata max and falls back toLong.MAX_VALUE— results stay correct (the RangeBitmap domain is self-contained), only segment-level max pruning is weaker until the segment is rebuilt. Segments sealed after this change carry proper metadata min/max and do not hit that path.Reverts the interim table-config rejection in
RangeIndexType.validate()and the loud-fail guard inBitSlicedRangeIndexCreator(both introduced on this branch, never released).Notes / discussion
isSorted()scan already performed there). This is the "overhead we pay to add the range index"; it doubles as a pruning benefit since these columns previously had no segment min/max. Happy to gate it further if preferred.Testing
MutableNoDictColumnStatisticsTest— min/max recovered for single-value INT/LONG when metadata is null; FLOAT/DOUBLE and multi-value columns intentionally stay null.RealtimeSegmentConverterTest— end-to-end: an ingestion-aggregated no-dictionary LONG metric column with a range index seals successfully, its column metadata carries the recovered min/max, and a BitSliced range index is present on the column.IndexCombinationValidationTest— the previously-rejected combinations (both aggregation paths, REALTIME) now validate; version-1, offline, and non-aggregated cases still validate.BitSlicedIndexCreatorTest— reverted to the pre-guard state.Release Notes
A version-2 (BitSliced) range index on an ingestion-aggregated no-dictionary column is now supported: its min/max value domain is recovered from the sealed forward index at segment build time instead of failing to commit.