airborne12 opened a new pull request, #69976:
URL: https://github.com/apache/doris/pull/69976
### What problem does this PR solve?
Issue Number: None
Related PR: #65858
Problem Summary:
#65858 fixed non-vertical compaction losing string indexes by sealing the
index files in `SegmentFlusher::close()` and adding their size in
`BaseBetaRowsetWriter::_close_file_writers()`. That repeats the sequence
already in `_close_inverted_index_file_writers()`, which the vertical writer
uses, and splits it across two classes. The size line also runs for every write
and is only correct because other writes leave the collection total at zero.
This PR makes three changes:
1. **One close for compaction.** `_close_file_writers()` calls
`_close_inverted_index_file_writers()` when the rowset has index compaction.
`SegmentFlusher::close()` goes back to closing what was flushed, and sealing,
size accounting and waiting stay in one place.
2. **`IndexFileWriter::open()` rejects a closed writer.** The original bug
stayed silent because `open()` accepted a closed writer and the directory it
returned never reached the sealed file. A mistake of this kind now fails the
compaction instead of committing a rowset without its index.
3. **End to end test.** `test_index_compaction_non_vertical` covers this
path, including a table with both numeric and string indexes. The existing BE
unit test `HorizontalCompactionSealsIndexesAfterPostingsAreWritten` stays as it
is and passes unchanged.
Two facts the new ordering relies on:
- All segments are flushed before the close, because the merger flushes the
rowset writer when it finishes and index compaction needs the complete rowid
map.
- `SegmentFlusher::close()` still runs `finish_close()` afterwards. That
does nothing for a closed writer, on V1, V2, V3 and SNII.
Backports with the same `_close_file_writers()` change: #69933 (branch-4.1),
#69936 (branch-4.2).
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test: in non-cloud mode,
`test_index_compaction_non_vertical`, `test_index_compaction_dup_keys`,
`_unique_keys`, `_null` and `_empty_segments` pass.
- [x] Unit Test: `IndexFileWriterTest`, including the new
`OpenAfterCloseFails`, and
`HorizontalCompactionSealsIndexesAfterPostingsAreWritten` pass.
- [x] Manual test: full compaction with vertical off on V2, V3 and SNII,
with string indexes only and with numeric plus string indexes. Index size is
above zero and no index downgrade happens.
- [ ] No need to test or manual test.
Cloud mode was not run on master. It was run for the same
`_close_file_writers()` change on a 4.1-based tree.
- Behavior changed:
- [ ] No.
- [x] Yes. Opening an index on a closed `IndexFileWriter` now returns an
error. No such caller exists after this change.
- 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]
