airborne12 opened a new pull request, #69933:
URL: https://github.com/apache/doris/pull/69933
### What problem does this PR solve?
Issue Number: None
Related PR: #65858
Problem Summary:
With `enable_vertical_compaction=false` and
`inverted_index_compaction_enable=true`, a DUP KEY or MoW table compacted
through the non-vertical path loses the inverted indexes of its string columns.
- Columns in `columns_to_do_index_compaction` are skipped by the segment
writer and merged after all segments are flushed.
- Non-vertical compaction flushes through `SegmentFlusher`, which called
`close_inverted_index()` at flush time. With no logical index present,
`begin_close()` sealed an empty container and uploaded a zero byte `.idx`.
- Index compaction then wrote into temporary directories of the closed
writer, and `SegmentFlusher::close()` only ran `finish_close()`, so the merged
index was never packed. `succeed to do index compaction` is still logged.
Result: `INDEX_DISK_SIZE=0`, and each query logs `[E-6004] inverted index
file ... is empty` and scans the whole segment. Query results stay correct.
With a numeric index on the same table, the file is sealed with the numeric
index alone, so `INDEX_DISK_SIZE` stays above zero while the string index is
gone.
master fixed this as part of the row TTL feature #65858. This PR carries
only that part, without the feature: when the rowset has index compaction
columns, sealing and size accounting move to `SegmentFlusher::close()`. This
branch still has `SegmentWriter`, so both `_flush_segment_writer` overloads are
changed.
A `DCHECK` in `BlockReader::_direct_next_block` is relaxed as well. At EOF
the block is empty and the iterator still reports the previous block's
locations, so ASAN and debug builds abort on this path. `Merger` only reads the
locations when the block has rows, so release builds were not affected.
Existing rowsets are not repaired by this change.
### Branch adaptations
- `has_index_compaction()` checks only `columns_to_do_index_compaction`,
because this branch has no SNII.
- master has no regression test for this path, so
`test_index_compaction_non_vertical` is new here.
### Release note
Fix inverted indexes of string columns being lost by non-vertical compaction
when inverted index compaction is enabled.
### Check List (For Author)
- Test
- [x] Regression test: in non-cloud mode on this branch, new
`test_index_compaction_non_vertical` fails without the fix and passes with it.
`test_index_compaction_dup_keys`, `_unique_keys`, `_null` and `_empty_segments`
pass.
- [ ] Unit Test
- [ ] Manual test
- [ ] No need to test or manual test.
Cloud mode on object storage was tested with the same change on a 4.1-based
tree, not on this branch. BE unit tests were not run.
- Behavior changed:
- [ ] No.
- [x] Yes. `INDEX_DISK_SIZE` of such compaction output is now the real
size and the output keeps its inverted indexes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
