airborne12 commented on code in PR #67859:
URL: https://github.com/apache/doris/pull/67859#discussion_r3988512427
##########
be/src/storage/index/index_file_writer.cpp:
##########
@@ -508,10 +507,16 @@ Status IndexFileWriter::begin_close() {
return _idx_v2_writer->close(true);
}
if (_indices_dirs.empty()) {
- // An empty file must still be created even if there are no indexes to
write
- if (dynamic_cast<io::StreamSinkFileWriter*>(_idx_v2_writer.get()) !=
nullptr ||
- dynamic_cast<io::S3FileWriter*>(_idx_v2_writer.get()) != nullptr ||
- dynamic_cast<io::PackedFileWriter*>(_idx_v2_writer.get()) !=
nullptr) {
+ // A schema that owns an index file always gets one, even when no
logical
Review Comment:
The persisted shape you describe is real, but it is the state this PR *stops
producing*, not one it creates — and the compatibility path you are asking for
is a safety trade that should not ride along in a bug fix.
**Verified.** `BetaRowset::link_files_to()`'s V2/V3 branch
(`be/src/storage/rowset/beta_rowset.cpp:453-472`) links unconditionally, unlike
its V1 branch just above, which explicitly skips a missing file (`"skip create
hard link to not existed index file"`). Callers that would hit it on an
affected rowset: `BaseBetaRowsetWriter::add_rowset`
(`beta_rowset_writer.cpp:847`, linked schema change), `SnapshotManager`
(`snapshot_manager.cpp:449` and `:670`, snapshot/backup/clone) and
`CompactionMixin::do_compact_ordered_rowsets` (`compaction.cpp:477`).
Two corrections to the scope:
- The ADD/DROP INDEX path is **not** affected. `index_builder.cpp:345`
passes a non-empty `excluded_index_ids`, and the V2 branch only links the file
when `without_index_uids == nullptr || without_index_uids->empty()`.
- `BetaRowset::remove()` is **not** affected:
`LocalFileSystem::delete_file_impl` returns `OK` when the file does not exist.
**Why not here.** Affected rowsets self-heal: ordinary compaction marks the
segment ineligible for index compaction and rebuilds every schema index from
the raw columns, and after this fix that output carries a correct (empty) index
file. Ordered compaction is the one path that would not heal it, and it is
already aware of this hazard — `handle_ordered_data_compaction()` carries the
comment *"If some rowsets has idx files and some rowsets has not, we can not do
link file compaction. Since the output rowset will be broken."*
Teaching `link_files_to()` to tolerate a missing V2/V3 file would equally
mask genuine index-file loss and propagate the broken shape into the target
rowset, so it needs its own discussion rather than being folded into this fix.
Happy to open a follow-up issue for the legacy-state repair if you would like
it tracked.
--
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]
