Drop star-tree indexes left unreadable by a dimension encoding change

[gortiz]

Problem

A segment containing a star-tree whose dimension split order includes column X becomes
permanently unloadable if the table config is later changed so that X moves to
noDictionaryColumns and the segment is not re-pushed. It surfaces on the next segment
(re)load — a restart, or any reload that re-opens the segment.

java.lang.IllegalStateException
    at FixedBitIntReader.getReader(FixedBitIntReader.java:109)
    at FixedBitSVForwardIndexReaderV2.<init>(FixedBitSVForwardIndexReaderV2.java:37)
    at StarTreeLoaderUtils.loadStarTreeV2(StarTreeLoaderUtils.java:73)
    at StarTreeIndexContainer.<init>(StarTreeIndexContainer.java:38)
    at ImmutableSegmentLoader.load(ImmutableSegmentLoader.java:250)

On load, ForwardIndexHandler converts X from dictionary to raw to match the current
noDictionaryColumns, updating the column metadata. With
enableDynamicStarTreeCreation=false, SegmentPreProcessor#processStarTrees early-returns, so
the now-stale star-tree is neither rebuilt nor removed. StarTreeLoaderUtils#loadStarTreeV2 then
builds a fixed-bit reader for the star-tree's X dimension and takes the bit width from the
main column metadata (ColumnMetadata#getBitsPerElement()), which is UNAVAILABLE for a raw
column. FixedBitIntReader#getReader hits its default: branch and throws, failing the whole
segment load.

Two properties make this hard to recover from:

• The dict-to-raw conversion is persisted, so after the first load attempt the segment is
  already inconsistent on disk and needProcess() reports nothing to do. A plain
  POST /segments/{seg}/reload re-runs the same failing path.
• Re-encoding the column is not a missing remediation step; it is what triggers the failure,
  because the star-tree still references X as a dictionary-encoded dimension.

Fix

Three layers, in pinot-segment-local:

1. Repair during pre-processing. StarTreeBuilderUtils#findUnloadableDimensions reports
   star-tree dimensions whose column no longer has a dictionary.
   SegmentPreProcessor#needProcessStarTrees/#processStarTrees use it to remove such star-trees
   even when enableDynamicStarTreeCreation is disabled. Removing star-trees only deletes
   files, so it is cheap enough for that flag; the rebuild the flag actually guards against stays
   gated on it. When the flag is enabled the existing flow already rebuilds them, because the
   split order no longer matches the builder configs.
2. Do not fail the segment at load. StarTreeLoaderUtils#loadStarTreeV2 skips an unreadable
   star-tree with a WARN naming the segment, the star-tree and the offending column, instead of
   throwing. This covers segments whose pre-processing is skipped.
3. Diagnostics. FixedBitIntReader#getReader now reports the offending bit width rather than
   throwing a message-less IllegalStateException.

Note that TableConfigUtils#validateStarTreeIndexConfigs already rejects a star-tree config whose
dimension has no dictionary, so the inconsistency only ever exists between the segment and the
config, never within the config itself. That is why this is fixed by repairing the segment rather
than by making ForwardIndexHandler refuse the conversion — refusing it would silently ignore
noDictionaryColumns and would still leave already-affected segments broken.

Testing

Four tests in SegmentPreProcessorTest, covering the drift with dynamic star-tree creation
disabled, the already-persisted variant where nothing else needs updating, the load path with
pre-processing skipped, and the dynamic-creation path (which was already safe —
MultipleTreesBuilder#getSeparator moves the star-tree files aside before loading the segment).
The first three fail on master with the stack above.

Existing suites pass: SegmentPreProcessorTest, LoaderTest, ForwardIndexHandler*,
*StarTree* and FixedBit* in pinot-segment-local, and org.apache.pinot.core.startree.v2.*
in pinot-core.

Release notes

Segments are no longer sent to ERROR when a star-tree dimension column is moved to
noDictionaryColumns without the segment being rebuilt. The stale star-tree is dropped on the
next reload (and rebuilt from the current config if enableDynamicStarTreeCreation is enabled).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.13%. Comparing base (899e40d) to head (58ada3e).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
...t/segment/local/startree/StarTreeBuilderUtils.java	84.61%	0 Missing and 2 partials ⚠️
...egment/local/io/reader/impl/FixedBitIntReader.java	0.00%	1 Missing ⚠️
...ocal/segment/index/loader/SegmentPreProcessor.java	92.85%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19349      +/-   ##
============================================
+ Coverage     67.10%   67.13%   +0.03%     
  Complexity     1424     1424              
============================================
  Files          3468     3468              
  Lines        222353   222378      +25     
  Branches      34999    35009      +10     
============================================
+ Hits         149212   149304      +92     
+ Misses        61224    61153      -71     
- Partials      11917    11921       +4     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	67.13% <88.57%> (+0.03%)	⬆️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	67.13% <88.57%> (+0.03%)	⬆️
unittests	67.13% <88.57%> (+0.03%)	⬆️
unittests1	57.85% <51.42%> (-0.02%)	⬇️
unittests2	38.90% <88.57%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jackie-Jiang]

Thinking more of the problem, when a dictionary encoded column is removed, but star-tree is configured, we should keep the dictionary, just dropping the forward index. Star-tree can be used with RAW forward index and dictionary. See #19153

[Jackie-Jiang]

@deepthi912 Can you also take a look and see if this works well with the new added raw dimension support

[Jackie-Jiang]

#19153 added support for star-tree on raw encoded column with separate dictionary. We should verify if the preprocess can rebuild the star-tree to generate dictionary.

[gortiz]

Good point — I'd like to check we're targeting the same scenario, because I think there are two.

What this PR fixes: the star-tree config is already gone from the table config (or enableDynamicStarTreeCreation is false, so it's never reconciled), the column moves to noDictionaryColumns, and the segment isn't re-pushed. ForwardIndexHandler drops the dictionary and the stale star-tree in the segment becomes unreadable — StarTreeLoaderUtils takes the bit width from the main ColumnMetadata, which is now UNAVAILABLE, and the whole segment load fails. There's no star-tree config left to key "keep the dictionary" off, and for segments already converted on disk the dictionary is gone regardless, so those need a repair path either way.

The scenario your comment describes — star-tree still configured on the column while the column moves to noDictionaryColumns — is rejected upfront today by TableConfigUtils#validateStarTreeIndexConfigs: "Cannot create star-tree index on dimension column: %s without dictionary". So it's only reachable with validation skipped. Are you thinking of that case (and of relaxing the validation now that #19153 makes RAW forward + separated dictionary viable), or of the config-dropped case?

If the former, I agree it's worth doing and it looks tractable: ForwardIndexHandler#computeOperations already preserves the dictionary through a DICT→RAW flip via dictRequiredByIndex = DictionaryIndexConfig.requiresDictionary(...), so it's mostly a matter of making a star-tree dimension count as "requires dictionary". The wrinkle is that star-tree configs are table-level and not reachable from the per-column FieldIndexConfigs the handler sees. I'd prefer it as a follow-up on top of this PR, since it's additive and doesn't remove the need for this repair path — happy to take it if you agree on the shape.

On the two inline questions about #19153: the check here is on the dictionary, not on the forward-index encoding, so a RAW-forward + separated-dictionary dimension is never flagged. BaseSegmentCreator writes hasDictionary = (dictionaryCreator != null) independently of FORWARD_INDEX_ENCODING and writes BITS_PER_ELEMENT from cardinality regardless; ColumnMetadataImpl only forces bitsPerElement = UNAVAILABLE when hasDictionary is false. I've pushed testStarTreeDimensionConvertedToRawWithSeparatedDictionary to confirm: with the star-tree config kept and the dimension switched to encodingType: RAW + dictionary index enabled, pre-processing flips the forward index, keeps the dictionary, and leaves the star-tree in place — the loaded segment still serves it, with a live dictionary on the column. No rebuild needed.