airborne12 opened a new pull request, #70270:
URL: https://github.com/apache/doris/pull/70270
> Stacked PR in the SNII / text analysis / ANN CLucene removal series (index
at the end). It depends on #70260 (T1), whose commits show in this diff until
they merge; review only this PR's own commits: `00da6453bb6` [refactor](be)
Give the ANN index a storage interface of its own. It stays a draft until its
dependencies merge.
### What problem does this PR solve?
Issue Number: None
Related PR: #70260
Problem Summary:
the ANN index read and wrote its files through CLucene itself.
VectorIndex::save and load took a lucene::store::Directory,
faiss_ann_index.cpp
drove IndexInput and IndexOutput and caught CLuceneError in six places,
AnnIndexReader kept a DorisCompoundReader, and ann_index_writer.h included
CLucene.h and three more CLucene headers it did not use. That ties every
format that stores ANN, SNII included, to CLucene directories. This change
gives ANN a narrow storage interface and puts CLucene behind an adapter; the
bytes on disk, the error codes and the reads V1 and V2 send to the index file
stay the same.
- ann/ann_index_storage.h: AnnIndexInput (length, and read_at with the IO
context of the call, coalescing small reads), AnnIndexOutput (append, and a
close that may be called again), AnnIndexSource (open_input) and
AnnIndexSink (create_output). No CLucene.
- inverted/clucene/lucene_ann_store.{h,cpp}: LuceneAnnSource and
LuceneAnnSink
over a lucene::store::Directory, the base class, so the tests keep their
RAMDirectory. read_at binds the IO context to the CLucene input for the one
call and reads through its BufferedIndexInput as before; reads and writes
over 2 GiB go in Int32 pieces; every CLuceneError becomes
INVERTED_INDEX_CLUCENE_ERROR with the file's name.
- ann/: VectorIndex::save(AnnIndexSink*) and load(AnnIndexSource*,
const io::IOContext*). FaissIndexWriter, FaissIndexReader and
CachedRandomAccessReader go through the interface; CachedRandomAccessReader
keeps its _io_mutex and passes each search's g_current_io_ctx to read_at. A
failed read or write inside faiss is still IO_ERROR, a failed open, create
or
close still INVERTED_INDEX_CLUCENE_ERROR. AnnIndexReader keeps an
AnnIndexSource, declared before the index as the compound reader was,
because an IVF_ON_DISK index reads through it on every search; load_index
no
longer catches CLuceneError. ann_index_writer.h drops CLucene.h,
LanguageBasedAnalyzer.h, bkd_writer.h, index_fs_directory.h and
index_file_writer.h.
- IndexFileWriter::open_ann_sink and discard_ann_staging replace
open_ann_directory and discard_ann_staging_directory, and
IndexFileReader::open_ann_source opens the read side. For now both wrap the
directories of today in the adapters: V1's and V2's directory and compound
reader, SNII's staging directory and its compound reader over the blob.
- check-header-deps.py: the ANN files and index_fs_directory.h leave the
lists
of files allowed to reach CLucene.
- index_writer.cpp and segment_read_ahead.cpp include what they use
(index_file_writer.h, cloud/config.h): the first got it through
ann_index_writer.h, the second from a unity batch the new file moves.
Error messages change in three places: save reports the adapter's create,
write or close failure instead of "Failed to save vector index", a missing
ivfdata file is "Failed to open index file: ann.ivfdata, error: ...", and an
exception other than CLuceneError thrown inside a CLucene call keeps its own
code instead of becoming IO_ERROR. An IVF list over 2 GiB is now written in
pieces where it used to fail.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
Details:
- BE UT under ASAN: be/test/storage/index/ann/ in full with the other
index
suites (index file writer and reader, SNII, inverted index, faiss,
vector), 2187 of 2187 tests in 281 suites; segment, rowset and
compaction
suites 945 of 948 with 3 skipped; the frozen goldens pass unchanged,
AnnWriteGolden among them, so the index files are byte for byte the
same.
The goldens run apart from CloudFileCacheWriteIndexOnlyTest: its
TearDown
clears ExecEnv's tmp file dirs, and a test writing index files after
it in
one process crashes, SniiWriterGoldenBytes as well (not new here).
New: LuceneAnnStoreTest (a CLuceneError from open, create, append,
close,
a read past the end and a failed read comes out as
INVERTED_INDEX_CLUCENE_ERROR; small adjacent reads share one buffered
read; a list's ids come out of the refill of its codes; a read larger
than the buffer goes straight to the file; the IO context is bound for
one call; an output closes once), AnnIndexStorageTest (the codes save
and
load report through FaissVectorIndex) and
AnnIndexReaderTest.IvfOnDiskIndexIsDestroyedBeforeItsSource (the inputs
of a searched IVF_ON_DISK index are destroyed before their source,
which
would be a heap-use-after-free under ASAN otherwise).
- AnnIndexRangeReadTest writes V1 and V2 indexes of HNSW, IVF and
IVF_ON_DISK to local files, loads them through AnnIndexReader and
searches them, and asserts the range reads and bytes the old code made,
recorded before this change: loads of 9 reads (HNSW), 34 (IVF) and 1
(IVF_ON_DISK), and for IVF_ON_DISK 8 reads in a narrow top-N and 24 in
a
range search. The new code makes exactly the same.
- check-build-hygiene.sh passes; clang-format 16 leaves the changed files
as they are; run-clang-tidy.sh reports nothing on the changed lines
(the
three files that include omp.h through faiss were checked with the
toolchain's include directory added, which clang-tidy's command lacks).
- Behavior changed:
- [x] No.
- [ ] Yes.
The index files, the error codes and the reads of V1 and V2 are the
same; only the message texts above change.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 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]
