Skip to content

Fix: Write .liv files to disk for segment replication and remote store#20573

Open
cuonghm2809 wants to merge 1 commit intoopensearch-project:mainfrom
cuonghm2809:fix-segment-replication-liv-files-main
Open

Fix: Write .liv files to disk for segment replication and remote store#20573
cuonghm2809 wants to merge 1 commit intoopensearch-project:mainfrom
cuonghm2809:fix-segment-replication-liv-files-main

Conversation

@cuonghm2809
Copy link
Contributor

Description

This PR fixes a critical bug in segment replication and remote store that causes NullPointerException during background merge operations when processing k-NN vector fields with document deletions.

Problem

When using SEGMENT replication or remote store with k-NN vectors, the index becomes RED during background merge with the following error:

java.lang.NullPointerException: Cannot read field "values" because "this.current" is null
  at org.apache.lucene.codecs.KnnVectorsWriter$MergedFloat32VectorValues.vectorValue(KnnVectorsWriter.java:225)

Root Cause

DirectoryReader.open() is called with only one parameter (IndexWriter), which defaults to writeAllDeletes=false. This keeps deletion information in memory only and does NOT write .liv (live documents) files to disk.

Impact on Segment Replication:

  1. Primary shard: Deletions stored in memory only (no .liv files written to disk)
  2. Segment replication: Copies only disk files (.vec, .cfs, etc.) but NOT .liv files
  3. Replica shard: Missing deletion metadata
  4. Background merge: Lucene assumes all documents are live (no deletion bitmap available)
  5. k-NN merge: Tries to access deleted document's vector → NPE (vector data is NULL)

The Fix

Set writeAllDeletes=true when opening DirectoryReader for segment replication and remote store:

final boolean writeAllDeletes = engineConfig.getIndexSettings().isSegRepEnabledOrRemoteNode();
DirectoryReader.open(documentIndexWriter.getAccumulatingIndexWriter(), true, writeAllDeletes);

This ensures .liv files are written to disk during refresh and replicated to replica shards.

Impact

Affected Users:

  • Any cluster using SEGMENT replication OR remote store
  • With k-NN vector fields
  • That performs document deletions or updates

Severity: HIGH

  • Index becomes RED (unavailable)
  • Requires manual shard restoration

Related Issues

Additional Notes

This fix aligns OpenSearch's behavior with Lucene's design intent: when using file-based replication (segment replication, remote store), deletion information must be persisted to disk via .liv files. The current default (writeAllDeletes=false) is optimized for document replication where each replica independently manages deletions.

The fix applies to:

  • Segment replication: Explicitly enabled via index.replication.type=SEGMENT
  • Remote store: Implicitly requires similar behavior as segment replication

Backward Compatibility: This change only affects segment replication and remote store scenarios. Document replication (default) behavior is unchanged.

In segment replication and remote store scenarios, deletion information
must be persisted to .liv (live documents) files on disk so they can be
replicated to replica shards. Without this fix, replicas lack deletion
metadata, causing NullPointerException during background merge operations
when processing k-NN vectors.

Root Cause:
- DirectoryReader.open() was called with single parameter (IndexWriter only)
- This defaults to writeAllDeletes=false, keeping deletions in memory only
- Segment replication copies only disk files, missing .liv files
- During merge, Lucene assumes all docs are live (no deletion info)
- k-NN merge tries to access deleted document vectors -> NPE

The Fix:
- Set writeAllDeletes=true for segment replication and remote store
- Forces .liv files to be written to disk during refresh
- Replicas receive complete deletion information
- Background merge correctly skips deleted documents

Impact:
- Affects: SEGMENT replication + k-NN vectors + document deletions
- Severity: Index becomes RED, requires manual shard restoration
- Fix: Aligns with Lucene's intended behavior for file-based replication

Related: Issue opensearch-project#20572

Signed-off-by: Cuong Ha <cuonghm2809@gmail.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 8, 2026

📝 Walkthrough

Walkthrough

A single file modification introduces a writeAllDeletes flag derived from engine configuration to enable delete persistence when segment replication or remote nodes are active. The flag is passed to DirectoryReader initialization to ensure deletes are written to disk for these scenarios.

Changes

Cohort / File(s) Summary
Delete Write Configuration
server/src/main/java/org/opensearch/index/engine/InternalEngine.java
Adds logic to derive writeAllDeletes flag from engine config (checking SegRep or remote store enablement) and passes it to DirectoryReader.open() to ensure delete operations are persisted to .liv files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug, Indexing:Replication

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: writing .liv files to disk for segment replication and remote store scenarios.
Description check ✅ Passed The description is comprehensive and covers all required template sections: clear problem/root cause explanation, the fix with code context, and impact assessment. Related issues are referenced appropriately.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09b6a36 and 28d531c.

📒 Files selected for processing (1)
  • server/src/main/java/org/opensearch/index/engine/InternalEngine.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T17:40:27.167Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20411
File: server/src/main/java/org/opensearch/index/codec/CodecService.java:112-133
Timestamp: 2026-01-13T17:40:27.167Z
Learning: Avoid capturing or evaluating a supplier (e.g., this::defaultCodec) upfront when passing it to a registry during object construction. If registries may replace defaults during iteration (as in EnginePlugin.getAdditionalCodecs), pass the supplier itself and only resolve it at use time. This ensures dynamic behavior is preserved during initialization and prevents premature binding of defaults in codecs/registry setup. This pattern should apply to similar initialization paths in Java server code where registries may mutate defaults during construction.

Applied to files:

  • server/src/main/java/org/opensearch/index/engine/InternalEngine.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: detect-breaking-change
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: Analyze (java)
🔇 Additional comments (1)
server/src/main/java/org/opensearch/index/engine/InternalEngine.java (1)

597-601: Clean, well-targeted fix.

The approach of conditionally enabling writeAllDeletes based on isSegRepEnabledOrRemoteNode() is correct and consistent with how this setting is used elsewhere in the file (e.g., lines 749, 1074, 1999). The flag is appropriately scoped as a local final variable since the index setting is immutable for the engine's lifetime. This is the only production code location requiring the 3-argument DirectoryReader.open() form with writeAllDeletes—other engine implementations (ReadOnlyEngine, NRTReplicationEngine) don't need this flag as they don't handle pending deletes from an active IndexWriter in the same way.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2026

✅ Gradle check result for 28d531c: SUCCESS

@codecov
Copy link

codecov bot commented Feb 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.33%. Comparing base (3ba2f37) to head (28d531c).
⚠️ Report is 87 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #20573      +/-   ##
============================================
+ Coverage     73.25%   73.33%   +0.07%     
- Complexity    72103    72170      +67     
============================================
  Files          5798     5798              
  Lines        329732   329757      +25     
  Branches      47519    47524       +5     
============================================
+ Hits         241554   241830     +276     
+ Misses        68805    68516     -289     
- Partials      19373    19411      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant