v2.0.140: fix WAL corruption after region growth (#230) and Tantivy temp-dir leak (#215)
Docker Release / build-and-push (push) Has been cancelled
Docker Release / build-and-push (push) Has been cancelled
- #230: refresh cached_payload_end after WAL growth so rebuild_indexes no longer seeks into the grown WAL region and overwrites record payloads (was surfacing as "wal record checksum mismatch"). - #215: make TantivyEngine drop its TempDir last and release the index writer first, so per-put working directories are cleaned up instead of leaking into the system temp dir (notably on Windows). - add commit_per_put_survives_wal_growth regression test.
This commit is contained in:
+1
-1
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "memvid-core"
|
||||
version = "2.0.139"
|
||||
version = "2.0.140"
|
||||
edition = "2024"
|
||||
rust-version = "1.85.0"
|
||||
license = "Apache-2.0"
|
||||
|
||||
+20
-6
@@ -607,9 +607,7 @@ impl Memvid {
|
||||
|
||||
self.shift_data_for_wal_growth(delta)?;
|
||||
self.header.wal_size = new_size;
|
||||
self.header.footer_offset = self.header.footer_offset.saturating_add(delta);
|
||||
self.data_end = self.data_end.saturating_add(delta);
|
||||
self.adjust_offsets_after_wal_growth(delta);
|
||||
self.apply_wal_growth_offsets(delta);
|
||||
|
||||
let catalog_end = self.catalog_data_end();
|
||||
self.header.footer_offset = catalog_end
|
||||
@@ -659,6 +657,24 @@ impl Memvid {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn apply_wal_growth_offsets(&mut self, delta: u64) {
|
||||
if delta == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
self.header.footer_offset = self.header.footer_offset.saturating_add(delta);
|
||||
self.data_end = self.data_end.saturating_add(delta);
|
||||
// `cached_payload_end` mirrors `data_end` / footer positioning. After
|
||||
// `shift_data_for_wal_growth` every byte past the old WAL boundary
|
||||
// moved right by `delta`, so the cached payload boundary must also
|
||||
// advance. Forgetting this caused `rebuild_indexes` (which seeks to
|
||||
// `payload_region_end()`) to write embedded indexes back into the
|
||||
// grown portion of the WAL region, corrupting WAL record payloads.
|
||||
// See https://github.com/memvid/memvid/issues/230.
|
||||
self.cached_payload_end = self.cached_payload_end.saturating_add(delta);
|
||||
self.adjust_offsets_after_wal_growth(delta);
|
||||
}
|
||||
|
||||
fn adjust_offsets_after_wal_growth(&mut self, delta: u64) {
|
||||
if delta == 0 {
|
||||
return;
|
||||
@@ -801,9 +817,7 @@ impl Memvid {
|
||||
|
||||
self.shift_data_for_wal_growth(delta)?;
|
||||
self.header.wal_size = target;
|
||||
self.header.footer_offset = self.header.footer_offset.saturating_add(delta);
|
||||
self.data_end = self.data_end.saturating_add(delta);
|
||||
self.adjust_offsets_after_wal_growth(delta);
|
||||
self.apply_wal_growth_offsets(delta);
|
||||
|
||||
let catalog_end = self.catalog_data_end();
|
||||
self.header.footer_offset = catalog_end
|
||||
|
||||
@@ -12,8 +12,16 @@ use tantivy::{Index, IndexReader, Term, doc};
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Tantivy-backed search index used when the `lex` feature is enabled.
|
||||
///
|
||||
/// Field order is load-bearing for `Drop`: Rust drops struct fields in
|
||||
/// declaration order, so `work_dir` (the temporary directory backing the
|
||||
/// index) **must remain the last field**. The `index`, `reader`, and
|
||||
/// `index_writer` all keep file handles and lock files open inside that
|
||||
/// directory; if the `TempDir` were dropped first, its directory removal
|
||||
/// would fail on platforms that refuse to delete files with open handles
|
||||
/// (notably Windows), silently leaking one working directory per discarded
|
||||
/// engine. See https://github.com/memvid/memvid/issues/215.
|
||||
pub struct TantivyEngine {
|
||||
pub(super) work_dir: TempDir,
|
||||
pub(super) index: Index,
|
||||
pub(super) _schema: Schema,
|
||||
pub(super) content: Field,
|
||||
@@ -26,6 +34,20 @@ pub struct TantivyEngine {
|
||||
pub(super) index_writer: Option<IndexWriter>,
|
||||
pub(super) reader: IndexReader,
|
||||
pub(super) tokenizer: Option<String>,
|
||||
// MUST be the last field — see the type-level comment above.
|
||||
pub(super) work_dir: TempDir,
|
||||
}
|
||||
|
||||
impl Drop for TantivyEngine {
|
||||
fn drop(&mut self) {
|
||||
// Release the exclusive writer (and its `.tantivy-writer.lock`) before
|
||||
// the `work_dir` TempDir is removed during normal field drop. Without
|
||||
// this the writer lock can still be held when the directory removal
|
||||
// runs, leaking the working directory on Windows. See issue #215.
|
||||
if let Some(writer) = self.index_writer.take() {
|
||||
drop(writer);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Search hit returned from Tantivy queries.
|
||||
|
||||
+83
-1
@@ -3,11 +3,23 @@
|
||||
|
||||
use memvid_core::{
|
||||
EmbeddingIdentitySummary, MEMVID_EMBEDDING_MODEL_KEY, MEMVID_EMBEDDING_PROVIDER_KEY, Memvid,
|
||||
MemvidError, PutOptions, TimelineQuery,
|
||||
MemvidError, PutOptions, TimelineQuery, constants::HEADER_SIZE, io::header::HeaderCodec,
|
||||
};
|
||||
use std::fs::File;
|
||||
use std::io::Read;
|
||||
use std::num::NonZeroU64;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn read_wal_size(path: &Path) -> u64 {
|
||||
let mut header_bytes = [0u8; HEADER_SIZE];
|
||||
File::open(path)
|
||||
.unwrap()
|
||||
.read_exact(&mut header_bytes)
|
||||
.unwrap();
|
||||
HeaderCodec::decode(&header_bytes).unwrap().wal_size
|
||||
}
|
||||
|
||||
/// Test basic put operation with bytes.
|
||||
#[test]
|
||||
fn put_bytes_basic() {
|
||||
@@ -434,3 +446,73 @@ fn timeline_iteration() {
|
||||
|
||||
assert_eq!(entries.len(), 3, "Should have 3 timeline entries");
|
||||
}
|
||||
|
||||
/// Regression test for memvid/memvid#230 — sustained commit-per-put workloads
|
||||
/// that span multiple WAL growth cycles must keep the embedded WAL intact.
|
||||
///
|
||||
/// Before the fix, `grow_wal_region` / `ensure_wal_capacity` updated
|
||||
/// `header.footer_offset` and `self.data_end` after shifting the data region
|
||||
/// but left the cached `payload_region_end()` value stale. The next call to
|
||||
/// `rebuild_indexes` then sought to that pre-growth offset (which now lies
|
||||
/// inside the grown WAL region) and overwrote WAL record payloads, producing
|
||||
/// `Embedded WAL is corrupted at offset N: wal record checksum mismatch` on
|
||||
/// the following commit.
|
||||
#[test]
|
||||
fn commit_per_put_survives_wal_growth() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let path = dir.path().join("wal_growth.mv2");
|
||||
|
||||
let mut mem = Memvid::create(&path).unwrap();
|
||||
let initial_wal_size = read_wal_size(&path);
|
||||
let mut max_wal_size = initial_wal_size;
|
||||
|
||||
// Use text-indexable payloads of varying length so each commit drives the
|
||||
// full Tantivy rebuild path (`rebuild_indexes` → `flush_tantivy`) that
|
||||
// seeks to `payload_region_end()`. The mix of sizes ensures multiple
|
||||
// WAL growth cycles occur across the run.
|
||||
let words: &[&str] = &[
|
||||
"alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", "india",
|
||||
"juliet", "kilo", "lima", "mike", "november", "oscar", "papa", "quebec", "romeo", "sierra",
|
||||
"tango", "uniform", "victor", "whiskey", "x-ray", "yankee", "zulu",
|
||||
];
|
||||
|
||||
for i in 0..60u32 {
|
||||
// Build a ~1-3 KiB text body so commits exercise variable-size WAL
|
||||
// entries similar to the upstream repro.
|
||||
let body_len = 256usize + ((i as usize * 137) % 1024);
|
||||
let mut body = String::with_capacity(body_len * 8);
|
||||
let mut idx = i as usize;
|
||||
while body.len() < body_len {
|
||||
body.push_str(words[idx % words.len()]);
|
||||
body.push(' ');
|
||||
idx = idx.wrapping_add(1);
|
||||
}
|
||||
let opts = PutOptions {
|
||||
uri: Some(format!("mv2://wal-growth/doc-{i}")),
|
||||
title: Some(format!("doc-{i}")),
|
||||
search_text: Some(body.clone()),
|
||||
..Default::default()
|
||||
};
|
||||
mem.put_bytes_with_options(body.as_bytes(), opts)
|
||||
.unwrap_or_else(|e| panic!("put #{i} failed: {e}"));
|
||||
mem.commit()
|
||||
.unwrap_or_else(|e| panic!("commit #{i} failed: {e}"));
|
||||
max_wal_size = max_wal_size.max(read_wal_size(&path));
|
||||
}
|
||||
|
||||
assert!(
|
||||
max_wal_size > initial_wal_size,
|
||||
"test must exercise WAL growth (initial={initial_wal_size}, max={max_wal_size})"
|
||||
);
|
||||
|
||||
drop(mem);
|
||||
|
||||
// Reopening forces a full WAL scan; checksum verification will fire here
|
||||
// if any record payload was clobbered by a stale-offset index write.
|
||||
let reopened = Memvid::open_read_only(&path).unwrap();
|
||||
assert_eq!(
|
||||
reopened.stats().unwrap().frame_count,
|
||||
60,
|
||||
"all puts should be durable after WAL growth"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user