gortiz opened a new pull request, #19349:
URL: https://github.com/apache/pinot/pull/19349
## 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).
--
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]
