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]

Reply via email to