Skip to content

fix(scorch): verify merge result integrity before introduction - #2389

Open
flash7777 wants to merge 1 commit into
blevesearch:masterfrom
flash7777:fix/verify-merge-result
Open

fix(scorch): verify merge result integrity before introduction#2389
flash7777 wants to merge 1 commit into
blevesearch:masterfrom
flash7777:fix/verify-merge-result

Conversation

@flash7777

Copy link
Copy Markdown

Summary

Add a post-merge verification step that reads back the newly created segment before introducing it into the index snapshot. If any posting list entry is unreadable (e.g. corrupt varint encoding), the merge result is discarded and the source segments are preserved.

This prevents corrupt segments from entering the index, where they would cause permanent read errors and eventually block the merger goroutine entirely.

Background

We observed sporadic corruption (~1% of merges) in merge result segments when merging segments that contain document drops (updates/deletes). The corruption manifests as ReadUvarint overruns in freq/norm data — the segment file has the correct size but contains unreadable varint sequences.

Key observations:

  • Only occurs when drop bitmaps are non-empty (document updates)
  • Affects high-frequency keyword terms (e.g. MimeType, RootID)
  • Drop bitmaps were cloned before passing to MergeUsing — no external mutation; the bug is in the merge write path itself
  • Without this guard, one corrupt merge result permanently blocks the merger (it retries the same segment combination indefinitely)
  • The corrupt segment replaces valid source segments, causing silent data loss for affected terms

What this PR does

After segPlugin.MergeUsing() + segPlugin.OpenUsing(), iterate every posting of every term of every field in the new segment. If any Iterator.Next() returns an error, discard the segment file and return an error from the merge task. The source segments remain in the index unchanged.

Applied in both merge paths:

  1. planMergeAtSnapshot (file merge, merger goroutine)
  2. mergeAndPersistInMemorySegments (in-memory merge, persister goroutine)

Performance

The verification reads the merge result sequentially once. For typical merge tasks (2-4 segments, a few thousand documents), this takes 1-10ms against 100-1000ms for the merge itself (<10% overhead).

Reproduction

  1. Create an index with ~10,000+ documents sharing a high-frequency keyword term (e.g. MimeType="application/pdf")
  2. Reindex all documents with force (update = drop old + write new)
  3. Let the merger run — ~1% of merge tasks produce corrupt segments
  4. Without this patch: merger blocks permanently on the corrupt segment
  5. With this patch: corrupt result discarded, merger continues

Environment: bleve v2.6.0, zapx v17, Go 1.24, 68k documents.

Related

Add a post-merge verification step that reads back the newly created
segment before introducing it into the index snapshot. If any posting
list entry is unreadable (e.g. corrupt varint encoding), the merge
result is discarded and the source segments are preserved.

This prevents corrupt segments from entering the index, where they
would cause permanent read errors and eventually block the merger
goroutine entirely.

We observed sporadic corruption (~1% of merges) in merge result
segments when merging segments that contain document drops (updates).
The corruption manifests as ReadUvarint overruns in freq/norm data.

Without this guard, one corrupt merge result permanently blocks the
merger (it retries the same segment combination indefinitely). With
it, corrupt results are discarded, the merger retries with a different
segment combination, and the index remains correct.

Applied in both merge paths:
- planMergeAtSnapshot (file merge, merger goroutine)
- mergeAndPersistInMemorySegments (in-memory merge, persister goroutine)

The verification cost is one sequential read of the merge result. For
typical merges (2-4 segments, a few thousand documents), this is 1-10ms
against 100-1000ms for the merge itself (<10% overhead).

Relates to blevesearch#1306
@flash7777
flash7777 force-pushed the fix/verify-merge-result branch from 3d0b880 to eaee180 Compare August 10, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant