github-actions[bot] commented on code in PR #66856:
URL: https://github.com/apache/doris/pull/66856#discussion_r3829716146
##########
be/src/storage/index/index_file_writer.cpp:
##########
@@ -186,6 +186,37 @@ Result<std::shared_ptr<lucene::store::Directory>>
IndexFileWriter::_open_snii_an
return dir;
}
+void IndexFileWriter::discard_ann_staging_directory(const TabletIndex*
index_meta) {
+ // Only SNII stages an ANN index somewhere disposable. Branching here
rather
+ // than in the caller keeps the format knowledge on the side that owns it,
+ // exactly as open_ann_directory() does.
+ if (_storage_format != InvertedIndexStorageFormatPB::SNII) {
+ return;
Review Comment:
[P2] Delete disk-backed V1/V2 staging on save failure
This early return leaves the newly recoverable save-error path without
cleanup outside SNII. With `inverted_index_ram_dir_enable=false` (or RAM
staging disabled for base compaction), `open_ann_directory()` creates a real
`DorisFSDirectory` under the temp root. Its destructor is default and `close()`
does not delete the directory; the only `deleteDirectory()` call is in a
successful `begin_close()`, but `_build_and_save()` now returns the Faiss
append/close error before that call and the segment abort is also SNII-only. An
ENOSPC/close failure can therefore strand the partial ANN directory on the
failing temp volume. Please delete the exact disk-backed V1/V2 directory while
preserving the primary save error, and add a RAM-disabled exact-path failure
test.
##########
be/src/storage/index/index_file_writer.cpp:
##########
@@ -186,6 +186,37 @@ Result<std::shared_ptr<lucene::store::Directory>>
IndexFileWriter::_open_snii_an
return dir;
}
+void IndexFileWriter::discard_ann_staging_directory(const TabletIndex*
index_meta) {
+ // Only SNII stages an ANN index somewhere disposable. Branching here
rather
+ // than in the caller keeps the format knowledge on the side that owns it,
+ // exactly as open_ann_directory() does.
+ if (_storage_format != InvertedIndexStorageFormatPB::SNII) {
+ return;
+ }
+ DCHECK(index_meta != nullptr);
+ const auto key = std::make_pair(index_meta->index_id(),
index_meta->get_index_suffix());
+ _indices_dirs.erase(key);
+ _snii_blob_dir_metas.erase(key);
+}
+
+void IndexFileWriter::abandon_snii_staging() {
+ if (_storage_format != InvertedIndexStorageFormatPB::SNII) {
Review Comment:
[P2] Release callback-owned BKD staging on segment abort
This abort only empties `_indices_dirs`, which under SNII contains the ANN
staging directories. A native BKD never enters that map:
`SniiBkdIndexColumnWriter::finish()` now moves `bkd_data` into
`add_snii_blob_index()`, so the pending compound callback is its sole owner. If
BKD finishes and a later ANN, bloom/primary-key, footer, or other non-compound
step fails, the new segment wrapper calls this method but never poisons or
destroys `_snii_compound_writer`; the caller then skips
`close_inverted_index()`, and the retained `IndexFileWriter` keeps the file,
fd, and hot BKD sources alive. This is distinct from the existing poison thread
because no compound write failed here. Please add a terminal compound
abort/release operation and a held-writer test with successful BKD registration
followed by a later injected failure.
--
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]