Repository navigation
[refactor](be) Give the ANN index a storage interface of its own - #70270
Draft
airborne12 wants to merge 4 commits into
Draft
airborne12 wants to merge 4 commits into
airborne12 wants to merge 4 commits into
Conversation
…kens
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The SNII text analysis is moving off CLucene one analyzer at a
time, and every step has to show two things against the code before it: that it
is faster, and that every row still gives the same tokens. There was no tool for
either.
benchmark_analyzer.hpp measures every builtin parser and a few custom chains on
the path the SNII writer takes for a row (a new stream per row) and with one
stream across rows, over corpora given as `.rows` files. With
ANALYZER_BENCH_DIGEST set it registers only the benchmark "digest", which writes
one line per analyzer, corpus and row: the number of terms and a digest of their
bytes and positions. Diffing the files of two binaries names every row whose
tokens changed, on real text and with the real kuromoji dictionary, which the
unit tests cannot hold.
analyzer_ab.py runs two binaries in alternating rounds and reports the median of
the per-round throughput ratios, because the host load moves a single
measurement by 10 to 20 percent. analyzer_corpus.py builds the corpora; the
Chinese and Japanese ones are cut from the Wikipedia CirrusSearch dumps of
2025-12-29, so the same date gives the same corpus. analyzer_benchmark.md lists
the steps.
The analysis rewrite is measured against this commit, branch
snii-analysis-bench-baseline.
### Release note
None
### Check List (For Author)
- Test: Manual test
- BUILD_TYPE=RELEASE DORIS_BENCHMARK_HEADER=benchmark_analyzer.hpp
./build.sh --benchmark builds benchmark_test.
- The digest mode over six corpora of 8 MB (four mixed ones, zh_wiki and
ja_wiki) writes 5,240,060 lines in 67 s. A build with the separate
dynamic_cast fix of the tokenizer base class, which keeps every token,
gives a byte-identical file.
- analyzer_ab.py, five rounds: that fix measures 1.84 on basic and 1.64 on
custom standard + lowercase. The A/A run stayed within 0.94 to 1.08 per
analyzer on a host at load 140 to 280, more than the 5% the method
allows, so finer comparisons need a quieter host.
- Behavior changed: No
- Does this need documentation: No
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…being frozen
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Freezing the output of every text analyzer, in the next commit,
ran into three bugs that no frozen digest can hold.
1. The word_delimiter filter threw std::out_of_range for a token holding a NUL
byte when catenate_words, catenate_numbers or catenate_all was set.
WordDelimiterConcatenation::append called std::string::append(const char*,
pos, n), which resolves to the string_view overload and measures the text up
to its first NUL, so for "foo\0bar" the second part started past the end.
The SNII and V1/V2 writers catch only CLuceneError and doris::Exception, so
the error left the writer. The part is now appended from its offset.
2. IKAnalyzer::tokenStream created its tokenizer with new and then called
reset(), which throws when a value starts with a character cut short, such as
"\xE4\xB8". The tokenizer and its buffers, about 100 KB, leaked once per such
value, on the query path and on the SNII write path, which build a stream per
value. A unique_ptr now owns it until reset() returns.
3. The IK dictionary was loaded under std::call_once, which is used up even when
loading fails. After one failure, for example a dictionary path that is fixed
later, every IK analyzer kept failing until the BE restarted, and in the unit
tests the result of the IK tests depended on whether a test with a bad path
ran first. Dictionary::initial now publishes the dictionary only after it has
loaded, and the next call tries again. reload() and getSingleton(cfg), which
only existed to recover from that state, are removed.
### Release note
Fix the word_delimiter filter failing on values with a NUL byte when a catenate
option is set, and a memory leak of the IK analyzer on values it cannot decode.
### Check List (For Author)
- Test: Unit Test
- New: WordDelimiterFilterTest.CatenatePartsAcrossNul, and
IKTokenizerTest.TestUndecodableValueThrowsWithoutLeakingTheStream, whose
leak LeakSanitizer reports when the binary exits. Changed:
IKTokenizerTest.TestDictionaryExceptionHandling checks that a failed load
is retried and that the first loaded configuration stays.
- ./run-be-ut.sh --run --filter='WordDelimiterFilterTest*:IKTokenizerTest*'
passes on ASAN, also three times with GTEST_SHUFFLE=1 together with the
golden tests of the next commit.
- Behavior changed: Yes. A value with a NUL byte under word_delimiter with a
catenate option is indexed instead of failing; a failed IK dictionary load is
retried by the next IK analyzer instead of failing until a restart.
- Does this need documentation: No
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The SNII text analysis is about to be rewritten without
CLucene. A change in what an analyzer emits raises no error anywhere: queries
just stop finding the rows indexed with the old tokens. So the output is frozen
before any implementation changes, and every later step has to keep it.
AnalysisGolden runs 155 analyzer configurations over 13 corpus families. The
configurations are written the way users write them, as index properties and
analyzer, normalizer, tokenizer and filter policies, so the matrix does not move
when the implementation behind it does: every builtin parser with its lower_case
and stopwords values, every custom tokenizer, token filter and char filter with
its settings (pairwise covering arrays for the 16 pinyin and 9 word_delimiter
switches), the index's own char filter, normalizers, and the normalize() of
prefix and wildcard queries. The families aim at known risks: token windows at
255 bytes and at 255 UTF-16 code units, batch sizes of 255 to 513 tokens,
characters across 4096-byte chunks, invalid UTF-8, sampled jieba and IK
dictionary words, unknown Han strings, every pair of word break classes, sampled
BMP code points, NUL bytes and long lines. Each configuration runs with a new
stream per line, as queries and the SNII writer read values, and with one stream
across lines, as the V1/V2 writers do. Both must give the same XXH3 digest, the
one frozen in analysis_golden.txt; on the current code no configuration
disagrees. ANALYSIS_GOLDEN_FULL=1 scans every code point against
analysis_golden_full.txt. A test checks that every component AnalysisFactoryMgr
registers is in some configuration, and that the ICU analyzers give the same
output with Turkish as the default locale. The kuromoji configurations use a
small dictionary the test builds, because the real one is not in the
repository. The 4030 jobs run on up to 16 threads.
AnalysisDifferential compares the production english, standard, unicode,
custom standard and chinese analyzers with their old implementations, which stay
linked into the unit tests, on the same families and on one second of random
input, and shows that an injected difference is caught.
AnalysisWriteGolden freezes what the writers make of the tokens: the bytes of
the SNII file, and, because V1/V2 files carry timestamps, the terms with their
documents and positions read back, for chinese, ik, icu and a custom chain.
AnnWriteGolden freezes every sub-file of HNSW, IVF and IVF_ON_DISK indexes in V2
and in SNII, built on one OpenMP thread from vectors whose distances are exact
in float, and written twice to show the bytes repeat.
Lines of the digest files are only added: ANALYSIS_GOLDEN_REGEN=<commit> appends
the missing ones and never rewrites one; ANALYSIS_GOLDEN_DUMP writes the token
streams to diff two binaries.
check-header-deps.py gets two rules for the code moving off CLucene (snii/,
analysis/, query/, common/ and ann/ under storage/index): no file there, sources
included, and no project header it reaches may include a CLucene header, and
the code there may not name a CLucene symbol once comments and strings are
stripped, because the PCH makes CLucene visible without an include. What still
breaks the rules is listed by the file that breaks it, 31 and 51 entries; a
missing or stale entry fails the gate, so the lists only shrink.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh --run --filter='*AnalysisGolden*:*AnalysisDifferential*:*AnalysisWriteGolden*:*AnnWriteGolden*:SniiWriterGoldenBytes*:SniiGoldenCorpus*'
passes on ASAN. The default set takes 33 s of wall time on 16 threads
for 515 s of work; ANALYSIS_GOLDEN_FULL=1 takes 920 s.
- GTEST_SHUFFLE=1 with '*AnalysisGolden*:*IK*' passes three times, seeds
56420, 13481 and 56363.
- The same tests pass on a RELEASE build, and both builds write
byte-identical digests (ANALYSIS_GOLDEN_WRITE, 2065 lines). The ANN
vectors have small integer coordinates for that: with fractional ones
the two builds wrote different HNSW graphs, because -O0 and -O3 round
faiss's distance sums differently.
- build-support/run-clang-tidy.sh --base origin/master reports no warning
on the changed lines.
- Mutations, each built, run and reverted: SimpleTokenizer<char> cutting
english tokens at 254 bytes instead of 255 fails the 4 english
configurations; basic splitting tokens over 255 bytes instead of
truncating them fails the 6 basic ones; IK lexemes ending one character
later fail the 10 IK ones; jieba without its lowercasing fails the 2
chinese ones with lower_case=true. Removing only the first-byte condition
of that lowercasing changes no digest: no token of the shipped
dictionaries starts with a non-ASCII byte and holds an ASCII letter.
- build-support/tests/test-build-hygiene-header-deps.sh passes, with new
cases for an angle include two hops from a root, a root source file,
stale entries in both tables, and CLucene words in comments and strings.
- Behavior changed: No
- Does this need documentation: No
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
### What problem does this PR solve?
Issue Number: None
Related PR: None
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: Unit Test
- 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: No. 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: No
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 of 16 tasks
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
This was referenced Nov 5, 2026
Merged
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #70260 (depends on)
Problem Summary: Give the ANN index a storage interface of its own and move its CLucene use behind an adapter for V1/V2. Bytes on disk, reads and error codes are unchanged.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)
🤖 Generated with Claude Code