mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 10:15:43 +00:00
enhance: build sealed tantivy indexes as a single segment (#51057)
issue: #51054 ## What Sealed tantivy index builds (inverted scalar, nested array, text match, ngram, json key stats — V5 and V7 writers alike) now deterministically produce **one tantivy segment**: - Build-mode writers set `NoMergePolicy` at creation, so background policy merges never exist during a build — the historical merge-all race ("segments could not be found in the SegmentManager") is impossible by construction rather than merely handled. The growing text index passes `enable_background_merge=true` and keeps the default policy (long-lived writer, periodic commits, needs bounded segment count). - `finish()` becomes: commit → **when more than one segment remains, merge all searchable segment ids on the same writer and propagate any merge error** → garbage-collect → wait_merging_threads → log the resulting segment ids. V5's `let _ = merge(...)` error swallow is replaced with the same error-propagating shape (its best-effort merge could silently leave multi-segment indexes when the race fired). Note: with `NoMergePolicy` the merge deterministically yields one segment, but `finish()` does not itself re-assert `len == 1` after the merge — the single-segment guarantee is covered by the `test_sealed_build_finishes_single_segment` unit test rather than a runtime check. ## Why The V7 writer lost V5's finish-time merge (commented out with a TODO referencing the long-closed #45590), so every sealed V7 index ships as ~15 MB-arena-sized segments: 19 segments for a 10M-row scalar index, ~400 for a 100M-element nested array index. Multi-segment hits range-style queries — each segment pays its own term-dict streaming + posting decode + doc-bitmap pass. Measured (10M rows, Apple M-series, median of 3): | query | multi-seg | single-seg | speedup | |---|---|---|---| | int64 `> v` @1% (19 seg) | 10.6 ms | 4.2 ms | 2.5x | | varchar `> v` @1% (19 seg) | 11.5 ms | 4.1 ms | 2.8x | | array MATCH_ANY @1% (261 seg, 60M elems) | 15.9 ms | 5.1 ms | 3.1x | | array MATCH_ANY @50% | 379.7 ms | 55.3 ms | 6.9x | Equality/term queries are unchanged (multi ≈ single within noise). Index size shrinks (int64 10M: 51→21 MB; array 60M elems: 698→174 MB). Cost: build wall time +16–30% at the 100M-element worst case (seconds at typical segment scale); total build IO is lower since intermediate policy merges no longer write discarded segments. ## Verification - 292 segcore element/array/match tests green on the development branch; text/ngram/json suites' failure sets byte-identical to pre-change baseline. - ~15 merge-all executions up to 60M docs with `RUST_BACKTRACE=full`: zero panics (#45590 did not reproduce), exactly 1 segment in every finish log (36/36 log lines on the final validation run). - Growing text index path exercised: growing never calls `Finish()` (periodic commits, background merges retained); the sealed interim text index does call `Finish()` and gets single-segment. - Cross-engine benchmark (inverted multi/single-seg vs sort index vs brute force) asserted identical result sets across all engines; the bench lands separately with the element-level query work it depends on. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com> Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: xiaofanluan <xf@hjjaq.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
xiaofanluan
parent
1c3438bc4f
commit
4955fcd73e
@@ -36,7 +36,8 @@ StripTextLogPrefix(const std::string& path, const std::string& base_prefix) {
|
||||
TextMatchIndex::TextMatchIndex(int64_t commit_interval_in_ms,
|
||||
const char* unique_id,
|
||||
const char* analyzer_name,
|
||||
const char* analyzer_params)
|
||||
const char* analyzer_params,
|
||||
bool enable_background_merge)
|
||||
: commit_interval_in_ms_(commit_interval_in_ms),
|
||||
last_commit_time_(stdclock::now()) {
|
||||
d_type_ = TantivyDataType::Text;
|
||||
@@ -47,7 +48,11 @@ TextMatchIndex::TextMatchIndex(int64_t commit_interval_in_ms,
|
||||
TANTIVY_INDEX_LATEST_VERSION /* Growing segment has no reason to use old version index*/
|
||||
,
|
||||
analyzer_name,
|
||||
analyzer_params);
|
||||
analyzer_params,
|
||||
/*analyzer_extra_info=*/"",
|
||||
milvus::tantivy::DEFAULT_NUM_THREADS,
|
||||
milvus::tantivy::DEFAULT_OVERALL_MEMORY_BUDGET_IN_BYTES,
|
||||
enable_background_merge);
|
||||
set_is_growing(true);
|
||||
}
|
||||
|
||||
|
||||
@@ -24,10 +24,16 @@ using stdclock = std::chrono::high_resolution_clock;
|
||||
class TextMatchIndex : public InvertedIndexTantivy<std::string> {
|
||||
public:
|
||||
// for growing segment.
|
||||
// In-memory writer. enable_background_merge must be true only for a
|
||||
// long-lived growing segment (periodic commits would otherwise grow the
|
||||
// segment count unbounded); a sealed interim index passes false so that
|
||||
// finish()'s explicit merge-all is the only merge and cannot race a
|
||||
// background policy merge.
|
||||
explicit TextMatchIndex(int64_t commit_interval_in_ms,
|
||||
const char* unique_id,
|
||||
const char* analyzer_name,
|
||||
const char* analyzer_params);
|
||||
const char* analyzer_params,
|
||||
bool enable_background_merge);
|
||||
// for sealed segment to create index from raw data during loading.
|
||||
explicit TextMatchIndex(const std::string& path,
|
||||
const char* unique_id,
|
||||
|
||||
@@ -275,7 +275,8 @@ TEST(TextMatch, Index) {
|
||||
auto index = std::make_unique<Index>(std::numeric_limits<int64_t>::max(),
|
||||
"unique_id",
|
||||
"milvus_tokenizer",
|
||||
"{}");
|
||||
"{}",
|
||||
/*enable_background_merge=*/false);
|
||||
index->CreateReader(milvus::index::SetBitsetSealed);
|
||||
index->AddTextSealed("football, basketball, pingpang", true, 0);
|
||||
index->AddTextSealed("", false, 1);
|
||||
@@ -457,8 +458,11 @@ TEST(TextMatch, BuildIndexFromFieldDataMultiBatchNullable) {
|
||||
|
||||
std::vector<milvus::FieldDataPtr> field_datas = {batch0, batch1, batch2};
|
||||
|
||||
auto index = std::make_unique<Index>(
|
||||
200, "test_multi_batch", "milvus_tokenizer", "{}");
|
||||
auto index = std::make_unique<Index>(200,
|
||||
"test_multi_batch",
|
||||
"milvus_tokenizer",
|
||||
"{}",
|
||||
/*enable_background_merge=*/true);
|
||||
index->CreateReader(milvus::index::SetBitsetGrowing);
|
||||
index->RegisterAnalyzer("milvus_tokenizer", "{}");
|
||||
|
||||
@@ -784,8 +788,11 @@ TEST(TextMatch, BuildIndexFromFieldDataSingleBatchNullable) {
|
||||
|
||||
std::vector<milvus::FieldDataPtr> field_datas = {fd};
|
||||
|
||||
auto index = std::make_unique<Index>(
|
||||
200, "test_single_batch", "milvus_tokenizer", "{}");
|
||||
auto index = std::make_unique<Index>(200,
|
||||
"test_single_batch",
|
||||
"milvus_tokenizer",
|
||||
"{}",
|
||||
/*enable_background_merge=*/true);
|
||||
index->CreateReader(milvus::index::SetBitsetGrowing);
|
||||
index->RegisterAnalyzer("milvus_tokenizer", "{}");
|
||||
|
||||
|
||||
@@ -4931,11 +4931,16 @@ ChunkedSegmentSealedImpl::CreateTextIndexWithSchema(
|
||||
std::string unique_id = GetUniqueFieldId(field_meta.get_id().get());
|
||||
if (!cfg.GetScalarIndexEnableMmap()) {
|
||||
// build text index in ram.
|
||||
// Sealed interim index: no background merge — finish() ends with an
|
||||
// explicit merge-all, and a racing policy merge (which finish()'s
|
||||
// NoMergePolicy cannot cancel once started) would reintroduce the
|
||||
// "segments could not be found in the SegmentManager" failure.
|
||||
index = std::make_unique<index::TextMatchIndex>(
|
||||
std::numeric_limits<int64_t>::max(),
|
||||
unique_id.c_str(),
|
||||
"milvus_tokenizer",
|
||||
field_meta.get_analyzer_params().c_str());
|
||||
field_meta.get_analyzer_params().c_str(),
|
||||
/*enable_background_merge=*/false);
|
||||
} else {
|
||||
// build text index using mmap.
|
||||
index = std::make_unique<index::TextMatchIndex>(
|
||||
|
||||
@@ -2375,7 +2375,8 @@ SegmentGrowingImpl::CreateTextIndex(FieldId field_id,
|
||||
200,
|
||||
unique_id.c_str(),
|
||||
"milvus_tokenizer",
|
||||
field_meta.get_analyzer_params().c_str());
|
||||
field_meta.get_analyzer_params().c_str(),
|
||||
/*enable_background_merge=*/true);
|
||||
index->Commit();
|
||||
index->CreateReader(milvus::index::SetBitsetGrowing);
|
||||
index->RegisterAnalyzer("milvus_tokenizer",
|
||||
|
||||
@@ -378,7 +378,8 @@ RustResult tantivy_create_index(const char *field_name,
|
||||
uint32_t tantivy_index_version,
|
||||
uintptr_t num_threads,
|
||||
uintptr_t overall_memory_budget_in_bytes,
|
||||
bool enable_user_specified_doc_id);
|
||||
bool enable_user_specified_doc_id,
|
||||
bool enable_background_merge);
|
||||
|
||||
RustResult tantivy_create_index_with_single_segment(const char *field_name,
|
||||
TantivyDataType data_type,
|
||||
@@ -561,7 +562,8 @@ RustResult tantivy_create_text_writer(const char *field_name,
|
||||
const char *analyzer_extra_info,
|
||||
uintptr_t num_threads,
|
||||
uintptr_t overall_memory_budget_in_bytes,
|
||||
bool in_ram);
|
||||
bool in_ram,
|
||||
bool enable_background_merge);
|
||||
|
||||
void tantivy_set_log_level(const char *level);
|
||||
|
||||
|
||||
@@ -49,6 +49,10 @@ impl IndexWriterWrapper {
|
||||
index.tokenizers().register(NGRAM_TOKENIZER, tokenizer);
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
// Ngram writers are only used for sealed index builds, which end with
|
||||
// an explicit merge-all in finish(); background merges would only
|
||||
// waste IO and race with it.
|
||||
index_writer.set_merge_policy(Box::new(tantivy::merge_policy::NoMergePolicy));
|
||||
|
||||
Ok(IndexWriterWrapper::V7(IndexWriterWrapperImpl {
|
||||
field,
|
||||
@@ -56,6 +60,8 @@ impl IndexWriterWrapper {
|
||||
index: Arc::new(index),
|
||||
enable_user_specified_doc_id: true,
|
||||
id_field: None,
|
||||
// Sealed-build only; merge-all runs in finish().
|
||||
enable_background_merge: false,
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -146,6 +146,7 @@ mod tests {
|
||||
1,
|
||||
50_000_000,
|
||||
false,
|
||||
false,
|
||||
TantivyIndexVersion::default_version(),
|
||||
)
|
||||
.unwrap();
|
||||
@@ -184,6 +185,7 @@ mod tests {
|
||||
1,
|
||||
50_000_000,
|
||||
false,
|
||||
false,
|
||||
TantivyIndexVersion::default_version(),
|
||||
)
|
||||
.unwrap();
|
||||
@@ -214,6 +216,7 @@ mod tests {
|
||||
1,
|
||||
50_000_000,
|
||||
false,
|
||||
false,
|
||||
TantivyIndexVersion::default_version(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
@@ -31,6 +31,7 @@ impl IndexWriterWrapper {
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
tanviy_index_version: TantivyIndexVersion,
|
||||
enable_user_specified_doc_id: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> Result<IndexWriterWrapper> {
|
||||
init_log();
|
||||
match tanviy_index_version {
|
||||
@@ -41,6 +42,7 @@ impl IndexWriterWrapper {
|
||||
path,
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
enable_background_merge,
|
||||
)?;
|
||||
Ok(IndexWriterWrapper::V5(writer))
|
||||
}
|
||||
@@ -52,6 +54,7 @@ impl IndexWriterWrapper {
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
enable_user_specified_doc_id,
|
||||
enable_background_merge,
|
||||
)?;
|
||||
Ok(IndexWriterWrapper::V7(writer))
|
||||
}
|
||||
@@ -236,6 +239,7 @@ mod tests {
|
||||
50_000_000,
|
||||
TantivyIndexVersion::V5,
|
||||
false,
|
||||
false,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
@@ -318,6 +322,7 @@ mod tests {
|
||||
1,
|
||||
50_000_000,
|
||||
false,
|
||||
false,
|
||||
TantivyIndexVersion::V5,
|
||||
)
|
||||
.unwrap();
|
||||
@@ -408,6 +413,7 @@ mod tests {
|
||||
100_000_000,
|
||||
TantivyIndexVersion::V7,
|
||||
enable,
|
||||
false,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@ pub extern "C" fn tantivy_create_index(
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
enable_user_specified_doc_id: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> RustResult {
|
||||
let field_name_str = cstr_to_str!(field_name);
|
||||
let path_str = cstr_to_str!(path);
|
||||
@@ -49,6 +50,7 @@ pub extern "C" fn tantivy_create_index(
|
||||
overall_memory_budget_in_bytes,
|
||||
tantivy_index_version,
|
||||
enable_user_specified_doc_id,
|
||||
enable_background_merge,
|
||||
) {
|
||||
Ok(wrapper) => RustResult::from_ptr(create_binding(wrapper)),
|
||||
Err(e) => RustResult::from_error(e.to_string()),
|
||||
|
||||
@@ -15,6 +15,7 @@ impl IndexWriterWrapper {
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
in_ram: bool,
|
||||
enable_background_merge: bool,
|
||||
tanviy_index_version: TantivyIndexVersion,
|
||||
) -> Result<IndexWriterWrapper> {
|
||||
match tanviy_index_version {
|
||||
@@ -27,6 +28,7 @@ impl IndexWriterWrapper {
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
in_ram,
|
||||
enable_background_merge,
|
||||
)?,
|
||||
)),
|
||||
TantivyIndexVersion::V7 => Ok(IndexWriterWrapper::V7(
|
||||
@@ -39,6 +41,7 @@ impl IndexWriterWrapper {
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
in_ram,
|
||||
enable_background_merge,
|
||||
)?,
|
||||
)),
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ pub extern "C" fn tantivy_create_text_writer(
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
in_ram: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> RustResult {
|
||||
init_log();
|
||||
let field_name_str = cstr_to_str!(field_name);
|
||||
@@ -40,6 +41,7 @@ pub extern "C" fn tantivy_create_text_writer(
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
in_ram,
|
||||
enable_background_merge,
|
||||
tantivy_index_version,
|
||||
) {
|
||||
Ok(wrapper) => RustResult::from_ptr(create_binding(wrapper)),
|
||||
|
||||
+28
-6
@@ -26,6 +26,7 @@ pub(crate) struct IndexWriterWrapperImpl {
|
||||
pub(crate) index_writer: Either<IndexWriter, SingleSegmentIndexWriter>,
|
||||
pub(crate) id_field: Option<Field>,
|
||||
pub(crate) _index: Arc<Index>,
|
||||
pub(crate) enable_background_merge: bool,
|
||||
}
|
||||
|
||||
#[inline]
|
||||
@@ -100,10 +101,11 @@ impl IndexWriterWrapperImpl {
|
||||
path: String,
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
enable_background_merge: bool,
|
||||
) -> Result<IndexWriterWrapperImpl> {
|
||||
info!(
|
||||
"create index writer, field_name: {}, data_type: {:?}, tantivy_index_version 5",
|
||||
field_name, data_type
|
||||
"create index writer, field_name: {}, data_type: {:?}, tantivy_index_version 5, enable_background_merge: {}",
|
||||
field_name, data_type, enable_background_merge
|
||||
);
|
||||
let mut schema_builder = Schema::builder();
|
||||
let field = schema_builder_add_field(&mut schema_builder, field_name, data_type);
|
||||
@@ -113,11 +115,18 @@ impl IndexWriterWrapperImpl {
|
||||
let index = Index::create_in_dir(path.clone(), schema)?;
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
if !enable_background_merge {
|
||||
// Sealed index builds end with an explicit merge-all in finish();
|
||||
// background policy-driven merges would only waste IO and race
|
||||
// with it, so disable them entirely for build-mode writers.
|
||||
index_writer.set_merge_policy(Box::new(tantivy_5::merge_policy::NoMergePolicy));
|
||||
}
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
index_writer: Either::Left(index_writer),
|
||||
id_field: Some(id_field),
|
||||
_index: Arc::new(index),
|
||||
enable_background_merge,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -140,6 +149,8 @@ impl IndexWriterWrapperImpl {
|
||||
index_writer: Either::Right(index_writer),
|
||||
id_field: None,
|
||||
_index: Arc::new(index),
|
||||
// Single-segment writer never runs finish()'s merge-all; value unused.
|
||||
enable_background_merge: false,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -284,19 +295,30 @@ impl IndexWriterWrapperImpl {
|
||||
}
|
||||
|
||||
pub fn finish(self) -> Result<()> {
|
||||
let enable_background_merge = self.enable_background_merge;
|
||||
match self.index_writer {
|
||||
Either::Left(mut index_writer) => {
|
||||
index_writer.commit()?;
|
||||
|
||||
// merge all segments
|
||||
let segment_ids = index_writer.index().searchable_segment_ids()?;
|
||||
if segment_ids.len() > 1 {
|
||||
let _ = index_writer.merge(&segment_ids).wait();
|
||||
if !enable_background_merge {
|
||||
// Build-mode writers use NoMergePolicy (set in new()), so no
|
||||
// background merge can race this explicit merge-all. Collapse
|
||||
// the auto-flushed segments into a single one. Background-merge
|
||||
// writers (e.g. growing segments) keep their own policy and are
|
||||
// not forced to a single segment here.
|
||||
let segment_ids = index_writer.index().searchable_segment_ids()?;
|
||||
if segment_ids.len() > 1 {
|
||||
index_writer.merge(&segment_ids).wait()?;
|
||||
}
|
||||
}
|
||||
|
||||
index_writer.garbage_collect_files().wait()?;
|
||||
|
||||
index_writer.wait_merging_threads()?;
|
||||
|
||||
let metas = self._index.searchable_segment_metas()?;
|
||||
let segment_ids: Vec<_> = metas.iter().map(|m| m.id().uuid_string()).collect();
|
||||
info!("tantivy index_writer finish, segments: {:?}", segment_ids);
|
||||
}
|
||||
Either::Right(single_segment_index_writer) => {
|
||||
single_segment_index_writer
|
||||
|
||||
Vendored
+6
@@ -33,11 +33,17 @@ impl IndexWriterWrapperImpl {
|
||||
};
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
// Json key stats writers are only used for sealed index builds, which
|
||||
// end with an explicit merge-all in finish(); background merges would
|
||||
// only waste IO and race with it.
|
||||
index_writer.set_merge_policy(Box::new(tantivy_5::merge_policy::NoMergePolicy));
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
index_writer: Either::Left(index_writer),
|
||||
id_field: Some(id_field),
|
||||
_index: Arc::new(index),
|
||||
// Sealed-build only; merge-all runs in finish().
|
||||
enable_background_merge: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+9
-2
@@ -31,10 +31,11 @@ impl IndexWriterWrapperImpl {
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
in_ram: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> Result<IndexWriterWrapperImpl> {
|
||||
info!(
|
||||
"create text index writer, field_name: {}, tantivy_index_version 5",
|
||||
field_name
|
||||
"create text index writer, field_name: {}, tantivy_index_version 5, enable_background_merge: {}",
|
||||
field_name, enable_background_merge
|
||||
);
|
||||
|
||||
let tokenizer = create_analyzer(tokenizer_params)?;
|
||||
@@ -48,12 +49,18 @@ impl IndexWriterWrapperImpl {
|
||||
index.tokenizers().register(tokenizer_name, tokenizer);
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
if !enable_background_merge {
|
||||
// Sealed text index builds end with an explicit merge-all in
|
||||
// finish(); disable background policy merges for them.
|
||||
index_writer.set_merge_policy(Box::new(tantivy_5::merge_policy::NoMergePolicy));
|
||||
}
|
||||
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
index_writer: Either::Left(index_writer),
|
||||
id_field: Some(id_field),
|
||||
_index: Arc::new(index),
|
||||
enable_background_merge,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+91
-3
@@ -98,6 +98,7 @@ pub struct IndexWriterWrapperImpl {
|
||||
pub(crate) index: Arc<Index>,
|
||||
pub(crate) id_field: Option<Field>,
|
||||
pub(crate) enable_user_specified_doc_id: bool,
|
||||
pub(crate) enable_background_merge: bool,
|
||||
}
|
||||
|
||||
impl IndexWriterWrapperImpl {
|
||||
@@ -108,10 +109,11 @@ impl IndexWriterWrapperImpl {
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
enable_user_specified_doc_id: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> Result<IndexWriterWrapperImpl> {
|
||||
info!(
|
||||
"create index writer, field_name: {}, data_type: {:?}, tantivy_index_version 7",
|
||||
field_name, data_type
|
||||
"create index writer, field_name: {}, data_type: {:?}, tantivy_index_version 7, enable_background_merge: {}",
|
||||
field_name, data_type, enable_background_merge
|
||||
);
|
||||
let mut schema_builder = Schema::builder();
|
||||
let field = schema_builder_add_field(&mut schema_builder, field_name, data_type);
|
||||
@@ -125,12 +127,19 @@ impl IndexWriterWrapperImpl {
|
||||
let index = Index::create_in_dir(path.clone(), schema)?;
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
if !enable_background_merge {
|
||||
// Sealed index builds end with an explicit merge-all in finish();
|
||||
// background policy-driven merges would only waste IO and race
|
||||
// with it, so disable them entirely for build-mode writers.
|
||||
index_writer.set_merge_policy(Box::new(tantivy::merge_policy::NoMergePolicy));
|
||||
}
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
index_writer,
|
||||
index: Arc::new(index),
|
||||
id_field,
|
||||
enable_user_specified_doc_id,
|
||||
enable_background_merge,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -268,7 +277,18 @@ impl IndexWriterWrapperImpl {
|
||||
|
||||
pub fn finish(mut self) -> Result<()> {
|
||||
self.index_writer.commit()?;
|
||||
// self.manual_merge();
|
||||
|
||||
if !self.enable_background_merge {
|
||||
// Build-mode writers use NoMergePolicy (set in new()), so no
|
||||
// background merge can race this explicit merge-all. Collapse the
|
||||
// auto-flushed segments into a single one. Background-merge writers
|
||||
// (e.g. growing segments) are left to their own policy and are not
|
||||
// forced to a single segment here.
|
||||
let segment_ids = self.index.searchable_segment_ids()?;
|
||||
if segment_ids.len() > 1 {
|
||||
self.index_writer.merge(&segment_ids).wait()?;
|
||||
}
|
||||
}
|
||||
block_on(self.index_writer.garbage_collect_files())?;
|
||||
self.index_writer.wait_merging_threads()?;
|
||||
|
||||
@@ -285,3 +305,71 @@ impl IndexWriterWrapperImpl {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use tantivy::Index;
|
||||
use tempfile::TempDir;
|
||||
|
||||
use super::IndexWriterWrapperImpl;
|
||||
use crate::data_type::TantivyDataType;
|
||||
|
||||
// tantivy's smallest per-thread arena (MEMORY_BUDGET_NUM_BYTES_MIN = 15 MB).
|
||||
// With a single indexing thread this is tight enough that the doc count below
|
||||
// spills into several auto-flushed segments before the finish-time commit,
|
||||
// which is exactly the multi-segment build this test needs to exercise.
|
||||
const MIN_MEMORY_BUDGET: usize = 15_000_000;
|
||||
const NUM_DOCS: i64 = 1_000_000;
|
||||
|
||||
fn build_i64_writer(path: &str, enable_background_merge: bool) -> IndexWriterWrapperImpl {
|
||||
IndexWriterWrapperImpl::new(
|
||||
"number",
|
||||
TantivyDataType::I64,
|
||||
path.to_string(),
|
||||
1, // single thread -> smallest arena -> forces multiple flushed segments
|
||||
MIN_MEMORY_BUDGET,
|
||||
false, // enable_user_specified_doc_id
|
||||
enable_background_merge,
|
||||
)
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
/// A build-mode (enable_background_merge == false) V7 writer must collapse the
|
||||
/// auto-flushed segments into exactly one searchable segment in finish().
|
||||
///
|
||||
/// Regression guard for the finish-time merge-all (issue #51054): the V7 writer
|
||||
/// previously shipped sealed indexes as many ~15 MB segments, and if the merge
|
||||
/// is ever dropped again the index silently regresses to multi-segment with only
|
||||
/// perf/logs to reveal it. The precondition assert keeps the test honest — it
|
||||
/// proves the workload really produced >1 segment before finish() merged them.
|
||||
#[test]
|
||||
fn test_sealed_build_finishes_single_segment() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let mut writer = build_i64_writer(dir.path().to_str().unwrap(), false);
|
||||
for i in 0..NUM_DOCS {
|
||||
writer.add::<i64>(i, i as u32).unwrap();
|
||||
}
|
||||
writer.commit().unwrap();
|
||||
|
||||
// Precondition: the build workload genuinely auto-flushes multiple segments,
|
||||
// so the single-segment assertion after finish() is meaningful.
|
||||
let before = writer.index.searchable_segment_metas().unwrap();
|
||||
assert!(
|
||||
before.len() > 1,
|
||||
"expected the build workload to auto-flush multiple segments, got {}",
|
||||
before.len()
|
||||
);
|
||||
|
||||
// finish() on a build-mode writer must merge them down to exactly one.
|
||||
writer.finish().unwrap();
|
||||
|
||||
let index = Index::open_in_dir(dir.path()).unwrap();
|
||||
let after = index.searchable_segment_metas().unwrap();
|
||||
assert_eq!(
|
||||
after.len(),
|
||||
1,
|
||||
"sealed build must produce exactly one tantivy segment, got {}",
|
||||
after.len()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Vendored
+6
@@ -33,12 +33,18 @@ impl IndexWriterWrapperImpl {
|
||||
};
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
// Json key stats writers are only used for sealed index builds, which
|
||||
// end with an explicit merge-all in finish(); background merges would
|
||||
// only waste IO and race with it.
|
||||
index_writer.set_merge_policy(Box::new(tantivy::merge_policy::NoMergePolicy));
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
index_writer,
|
||||
index: Arc::new(index),
|
||||
id_field: Some(id_field),
|
||||
enable_user_specified_doc_id: false,
|
||||
// Sealed-build only; merge-all runs in finish().
|
||||
enable_background_merge: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+11
-2
@@ -32,10 +32,11 @@ impl IndexWriterWrapperImpl {
|
||||
num_threads: usize,
|
||||
overall_memory_budget_in_bytes: usize,
|
||||
in_ram: bool,
|
||||
enable_background_merge: bool,
|
||||
) -> Result<IndexWriterWrapperImpl> {
|
||||
info!(
|
||||
"create text index writer, field_name: {}, tantivy_index_version 7",
|
||||
field_name
|
||||
"create text index writer, field_name: {}, tantivy_index_version 7, enable_background_merge: {}",
|
||||
field_name, enable_background_merge
|
||||
);
|
||||
|
||||
let analyzer = create_analyzer(analyzer_params, analyzer_extra_info)?;
|
||||
@@ -49,6 +50,13 @@ impl IndexWriterWrapperImpl {
|
||||
index.tokenizers().register(analyzer_name, analyzer);
|
||||
let index_writer =
|
||||
index.writer_with_num_threads(num_threads, overall_memory_budget_in_bytes)?;
|
||||
if !enable_background_merge {
|
||||
// Sealed text index builds end with an explicit merge-all in
|
||||
// finish(); disable background policy merges for them. Growing
|
||||
// segments keep the default policy, otherwise the segment count
|
||||
// grows unbounded across periodic commits.
|
||||
index_writer.set_merge_policy(Box::new(tantivy::merge_policy::NoMergePolicy));
|
||||
}
|
||||
|
||||
Ok(IndexWriterWrapperImpl {
|
||||
field,
|
||||
@@ -56,6 +64,7 @@ impl IndexWriterWrapperImpl {
|
||||
index: Arc::new(index),
|
||||
id_field: None,
|
||||
enable_user_specified_doc_id: true,
|
||||
enable_background_merge,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+12
-4
@@ -90,7 +90,8 @@ struct TantivyIndexWrapper {
|
||||
bool enable_user_specified_doc_id = true,
|
||||
uintptr_t num_threads = DEFAULT_NUM_THREADS,
|
||||
uintptr_t overall_memory_budget_in_bytes =
|
||||
DEFAULT_OVERALL_MEMORY_BUDGET_IN_BYTES) {
|
||||
DEFAULT_OVERALL_MEMORY_BUDGET_IN_BYTES,
|
||||
bool enable_background_merge = false) {
|
||||
RustResultWrapper res;
|
||||
if (inverted_single_semgnent) {
|
||||
AssertInfo(tantivy_index_version == 5,
|
||||
@@ -106,7 +107,8 @@ struct TantivyIndexWrapper {
|
||||
tantivy_index_version,
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
enable_user_specified_doc_id));
|
||||
enable_user_specified_doc_id,
|
||||
enable_background_merge));
|
||||
}
|
||||
AssertInfo(res.result_->success,
|
||||
"failed to create index: {}",
|
||||
@@ -131,6 +133,10 @@ struct TantivyIndexWrapper {
|
||||
}
|
||||
|
||||
// create index writer for text type with tokenizer.
|
||||
// enable_background_merge: growing segments must keep tantivy's default
|
||||
// merge policy (periodic commits would otherwise grow the segment count
|
||||
// unbounded); sealed index builds pass false since finish() ends with an
|
||||
// explicit merge-all.
|
||||
TantivyIndexWrapper(const char* field_name,
|
||||
bool in_ram,
|
||||
const char* path,
|
||||
@@ -140,7 +146,8 @@ struct TantivyIndexWrapper {
|
||||
const char* analyzer_extra_info = "",
|
||||
uintptr_t num_threads = DEFAULT_NUM_THREADS,
|
||||
uintptr_t overall_memory_budget_in_bytes =
|
||||
DEFAULT_OVERALL_MEMORY_BUDGET_IN_BYTES) {
|
||||
DEFAULT_OVERALL_MEMORY_BUDGET_IN_BYTES,
|
||||
bool enable_background_merge = false) {
|
||||
auto res = RustResultWrapper(
|
||||
tantivy_create_text_writer(field_name,
|
||||
path,
|
||||
@@ -150,7 +157,8 @@ struct TantivyIndexWrapper {
|
||||
analyzer_extra_info,
|
||||
num_threads,
|
||||
overall_memory_budget_in_bytes,
|
||||
in_ram));
|
||||
in_ram,
|
||||
enable_background_merge));
|
||||
AssertInfo(res.result_->success,
|
||||
"failed to create text writer: {}",
|
||||
res.result_->error);
|
||||
|
||||
Reference in New Issue
Block a user