Handle missing source for no-op tombstones

[aneesh-db]

Description

Fixes #22317.

LuceneChangesSnapshot already handles missing source for index operations, but no-op tombstones were still dereferencing fields.source() directly. This change routes no-op tombstones through the same missing-source handling:

• throw MissingHistoryOperationsException when a complete range is required
• skip the operation and increment skipped operations when a complete range is not required

The regression tests cover both paths by reading a no-op tombstone through a reader that hides the stored _source field.

Testing

• server:test --tests "org.opensearch.index.engine.LuceneChangesSnapshotTests"
• server:spotlessJavaCheck

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

📝 TODO sections server/src/main/java/org/opensearch/index/engine/LuceneChangesSnapshot.java [343]: Callers should ask for the range that source should be retained; always check source existence once peer-recovery sends ops after local checkpoint.

🔀 No multiple PR themes

⚡ Recommended focus areas for review Assertion on uninitialized variable After the early return handleMissingSource(seqNo) in the no-op tombstone branch, when source is present, the subsequent assertion assertDocSoftDeleted(...) includes op in its failure message. Since op is assigned just before via new Translog.NoOp(...), this is fine in the happy path, but the assert version == 1L check now executes after constructing the NoOp; if that assertion fails (assertions enabled), the NoOp has already been created. Minor, but worth verifying assertion ordering matches the original intent (previously the version assertion ran before/with op construction). if (isTombstone && fields.id() == null) { final BytesReference source = fields.source(); if (source == null) { return handleMissingSource(seqNo); } op = new Translog.NoOp(seqNo, primaryTerm, source.utf8ToString()); assert version == 1L : "Noop tombstone should have version 1L; actual version [" + version + "]"; assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Noop but soft_deletes field is not set [" + op + "]"; } else {

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Verify state updates on null return When handleMissingSource returns null (i.e., requiredFullRange is false), the method returns early before reaching the post-loop assertions and seqNo bookkeeping at the end of readDocAsOp. However, the caller loop likely relies on skippedOperations accounting which is handled, but skipping the trailing assertion block is fine. Confirm that returning null here does not bypass any required state updates (e.g., lastSeenSeqNo) that the original delete/index branch performed; otherwise subsequent ops may fail invariant checks. server/src/main/java/org/opensearch/index/engine/LuceneChangesSnapshot.java [298-306] +if (isTombstone && fields.id() == null) { + final BytesReference source = fields.source(); + if (source == null) { + return handleMissingSource(seqNo); + } + op = new Translog.NoOp(seqNo, primaryTerm, source.utf8ToString()); + assert version == 1L : "Noop tombstone should have version 1L; actual version [" + version + "]"; + assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Noop but soft_deletes field is not set [" + op + "]"; +} else { - Suggestion importance[1-10]: 4 __ Why: The suggestion only asks the author to verify behavior without proposing any actual code change (existing_code equals improved_code). It raises a reasonable concern about state bookkeeping but provides no concrete fix.	Low

[github-actions]

❌ Gradle check result for 25dff87: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?