deepthi912 opened a new pull request, #19331:
URL: https://github.com/apache/pinot/pull/19331
## Summary
`MultipleTreesBuilder.build()` opens `StarTreeIndexCombiner` (line 225),
which requires the target `star_tree_index` to NOT already exist
(`StarTreeIndexCombiner.java:47` —
`Preconditions.checkState(!indexFile.exists(), ...)`). A previous build that
was hard-killed (JVM crash, container OOM, thread hard-interrupt) can leave a
partial `star_tree_index` at the segment root without a matching
`STAR_TREE_COUNT` in segment metadata, because the metadata save
(`MultipleTreesBuilder:265`) runs only after all trees complete. The catch
block (line 249-262) cleans up on Java exceptions but never runs on hard
process termination.
## Repro path
1. Build starts, `StarTreeIndexCombiner` opens `star_tree_index` at the
segment root via `RandomAccessFile("rw")` (line 225) — file exists on disk
immediately.
2. `_metadataProperties.subset(...).clear()` runs (line 200 inside
`getSeparator()` for the incremental path) and is persisted at 201-202 —
segment metadata now says "no star-tree".
3. JVM is killed (SIGKILL, container OOM, hard interrupt). The catch block
never runs; `star_tree_index` stays on disk.
4. Next preprocess: `_separator = getSeparator()` returns `null` (no
`STAR_TREE_COUNT` in metadata). `build()` runs, opens the combiner on the
leftover file → `IllegalStateException: Star-tree index file already exists`.
5. Not self-healing — leftover keeps blocking every retry until manual
cleanup.
Observed in production: 28 occurrences over 7 days on a single table, all
identical stack trace ending at `StarTreeIndexCombiner.<init>:47`. Segments
affected remained OFFLINE across server restart cycles.
## Fix
When `_separator == null` (fresh build, metadata has no matching star-tree),
delete any leftover `star_tree_index`, `star_tree_index_map`, and
`EXISTING_STAR_TREE_TEMP_DIR` before opening the combiner. Metadata is
authoritative: with `_separator == null`, any on-disk star-tree artifact is
orphaned and was never registered.
The incremental path (`_separator != null`) is untouched — `getSeparator()`
has already moved the previous files aside legitimately into
`_separatorTempDir`, and rollback via `close()` still works normally.
Log at WARN so operators see the recovery in-flight.
## Tests
New `MultipleTreesBuilderStaleCleanupTest`:
- `staleIndexFileFromKilledBuildIsCleanedUp` — stray `star_tree_index` at
segment root → build cleans it up and produces a real tree.
- `staleSeparatorTempDirFromKilledIncrementalIsCleanedUp` — leftover
`EXISTING_STAR_TREE_TEMP_DIR` from a killed incremental → cleaned up on the
next fresh build.
Both fail without the fix (`IllegalStateException: Star-tree index file
already exists`).
## Release note
Fixes a stuck-segment case where a hard-killed star-tree build could leave a
stale `star_tree_index` file that blocked every subsequent preprocess attempt
on that segment.
--
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]
