github-actions[bot] commented on code in PR #66856:
URL: https://github.com/apache/doris/pull/66856#discussion_r3804882059
##########
be/src/storage/index/snii/snii_blob_staging_directory.cpp:
##########
@@ -151,12 +163,18 @@ void SniiBlobStagingDirectory::touchFile(const char*
/*name*/) {
lucene::store::IndexOutput* SniiBlobStagingDirectory::createOutput(const char*
name) {
DORIS_CHECK(name != nullptr);
// Same semantics as a filesystem directory: creating an existing name
- // truncates it. The buffer is replaced rather than cleared, so a blob
source
- // already taken over the old content keeps reading the old content
instead of
- // seeing it mutate underneath.
- auto buffer = std::make_shared<Buffer>();
- _files[name] = buffer;
- return _CLNEW StagingIndexOutput(std::move(buffer));
+ // truncates it. The file is replaced rather than reused, so a blob source
+ // already taken over the old content keeps reading the old content instead
+ // of seeing it mutate underneath.
+ std::unique_ptr<snii::bkd::StagedBlobFile> created;
+ Status status = snii::bkd::StagedBlobFile::create(name, &created);
+ if (!status.ok()) {
+ const std::string message = status.to_string();
+ _CLTHROWA(CL_ERR_IO, message.c_str());
+ }
+ auto file = std::shared_ptr<snii::bkd::StagedBlobFile>(std::move(created));
+ _files[name] = file;
Review Comment:
[P2] Drop staging files when ANN serialization fails
Once this file is inserted into `_files`, an append or `finalize()/fsync`
failure destroys only the output owner. `FaissVectorIndex::save()` returns a
`Status`, but neither it nor `AnnIndexColumnWriter::close_on_error()` erases
the staging directory held by `IndexFileWriter`; the segment path also returns
before `clear()`/`close_inverted_index()`. A real ENOSPC/fsync failure
therefore leaves an ANN-sized partial file and fd linked until the failed outer
writer is destroyed (IVF-on-disk can retain the completed first sub-file too).
This is before blob registration, so the existing `release_all_blob_sources()`
fix cannot reach it. Discard the SNII directory/file binding on save failure
and add a held-writer test that verifies the path is unlinked before either
outer writer dies.
##########
be/src/storage/index/index_file_writer.cpp:
##########
@@ -216,11 +215,11 @@ Status IndexFileWriter::_seal_snii_blob_directories() {
}
void IndexFileWriter::_release_snii_blob_directories() {
- // Dropping the map is the whole release: a staging directory holds its
bytes
- // in memory and owns no file, so there is nothing on disk to remove and --
- // unlike DorisFSDirectory::deleteDirectory() -- no throwing call to make
from
- // a Status-returning close path. Any buffer a registered blob source still
- // needs stays alive through that source until finish() has pulled it.
+ // Dropping the map is the whole release: each staging file unlinks itself
Review Comment:
[P2] Relinquish producer ownership when staging is sealed
Dropping this map is not the whole release on SNII ADD INDEX: each
`AnnIndexColumnWriter` retained in `IndexBuilder::_index_column_writers` still
owns the same directory through `_dir`, and that flow builds every segment,
closes every `IndexFileWriter`, then clears the producers. File-backed staging
is intentional, but these producer/directory owners keep the rowset's staged
files and fds pinned through the close loop, so per-blob cleanup cannot drain
them after their bytes are copied; `_indices_dirs` similarly keeps all sources
through a multi-ANN compound finish. Transfer/take `_files` out of the
directory when registering callbacks, close segments promptly in ADD INDEX, and
add a held-producer multi-segment cleanup test.
--
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]
