airborne12 opened a new pull request, #70271:
URL: https://github.com/apache/doris/pull/70271
> Stacked PR in the SNII / text analysis / ANN CLucene removal series (index
at the end). It depends on #70270 (T11), whose commits show in this diff until
they merge; review only this PR's own commits: `83e608f857c` [refactor](be)
Read and write SNII's ANN blobs without CLucene directories. It stays a draft
until its dependencies merge.
### What problem does this PR solve?
Issue Number: None
Related PR: #70270
Problem Summary:
since the ANN index got a storage interface of its own,
V1 and V2 implement it through a CLucene adapter, and SNII still went through
the same adapter over two CLucene directory shims: SniiBlobStagingDirectory
staged the faiss output, and the SNII branch of IndexFileReader::_open opened
the container a second time to read a blob through FSIndexInput and
DorisCompoundReader. This change implements the interface for SNII directly
and deletes the shims. The bytes on disk do not change.
- inverted/snii/snii_ann_store.{h,cpp}:
- SniiAnnSource reads the sub-files of an ANN blob through the container
reader IndexFileReader already opened, so a load opens the file once
instead of twice.
- SniiAnnInput buffers like CLucene's BufferedIndexInput, which V1 and V2
read through: a miss refills inverted_index_read_buffer_size bytes, cut
at the end of the sub-file, and the rest of a read at least that large
goes straight to the caller. read_at refuses offset + size past the
sub-file's length before any I/O, because the container reader bounds a
read by the container only and would serve the next blob.
- Each read goes out with the caller's IO context and is_index_data false,
as FSIndexInput sent it. So inverted_index_snii_read_no_write_file_cache
now covers ANN reads like every other SNII read, and their time goes to
inverted_index_io_timer.
- A NOT_FOUND from the file is IO_ERROR, since an ANN read has no fallback.
- SniiAnnStaging replaces SniiBlobStagingDirectory. Its output buffers
64 KiB, as CLucene's output did, because a staged file's append is a
write(2) of what it is given. close() does nothing when called again, and
an empty append, which faiss makes with a null pointer, writes nothing.
- IndexFileWriter keeps the SNII staging in its own map, with the metadata
the seal needs, so _indices_dirs holds only V1 and V2 directories and the
type assertion on getObjectName() goes away. delete_index, discard and
abandon of the staging, the seal, the release and debug_string use the new
map, and both maps refuse a duplicate key through one helper.
- IndexFileReader::open_ann_source reads SNII through SniiAnnSource. The SNII
branch of _open, which only ANN used, is gone, so open() now returns
INVERTED_INDEX_NOT_SUPPORTED for SNII. The refusal of a blob that is not an
ANN index moves to open_ann_source, without its "it has no CLucene
directory" tail.
- snii_blob_directory.{h,cpp}, which nothing in be/src used, move to the test
directory for the round-trip test that does; check-header-deps.py drops the
deleted files.
A blob that lacks a sub-file its index type writes, or a read past a
sub-file, is now INVERTED_INDEX_FILE_CORRUPTED at the input; through faiss it
is still IO_ERROR, and the ANN reader falls back to brute force on any load
failure as before.
### 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: the index suites (ANN, index file writer and reader,
SNII, inverted index, faiss, vector), 2201 of 2201 tests in 282 suites;
segment, rowset and compaction suites 945 of 948 with 3 skipped; the
frozen goldens pass unchanged, among them AnnWriteGolden, which now
reads
SNII's sub-files through the new input, and SniiWriterGoldenBytes.
New: SniiAnnStoreTest (10 tests: a read past the end fails before any
I/O and reads nothing; NOT_FOUND is IO_ERROR and is_index_data is false
even when the caller sets it; reads and their time are charged to the
caller's context; the no-write-file-cache switch sends ANN reads as
REMOTE_ONLY_ON_MISS when on and READ_THROUGH_AND_WRITE_BACK when off; a
staging output closes once, also after a failed finalize; buffering
against BufferedIndexInput's rules) and four SniiAnnContainerTest cases
(open() refuses every SNII index; the staging is kept apart from the
CLucene directories and refuses a duplicate key; an ANN index with no
rows leaves no staging; the staging calls leave V2's directories
alone).
SniiAnnContainerTest passes in full, 24 tests, with the texts of
IvfDataWriteFailuresAreReturnedFromFinish and
ANonAnnBlobIsRefusedByTheDirectoryPath.
- AnnIndexRangeReadTest now covers SNII too, cold and with the file cache
off, recorded on the old code first. The read counts are the same in
every case: loads of 10 (HNSW), 35 (IVF) and 3 (IVF_ON_DISK) reads, and
for IVF_ON_DISK 8 in a narrow top-N, 24 in a range search, 0 once the
lists are cached. The bytes are the same or fewer, fewer where the old
4
KiB refill ran past the end of a sub-file (IVF_ON_DISK load 4191 ->
2697
bytes, top-N 81495 -> 81400). The V1 and V2 numbers are unchanged.
- 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.
- Behavior changed:
- [x] No.
- [ ] Yes.
The index files and the reads are the same. With
inverted_index_snii_read_no_write_file_cache on, a query's ANN reads no longer
fill the file cache, as the variable already did for SNII's other reads.
- 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]
