Preserve routing in Translog.Delete for CDC-based replication

[arpitsingla96]

Summary

Preserve routing in Translog.Delete operations so that CDC-based replication tools (e.g. AOSC) can correctly replay deletes for documents with custom routing against target indices with different shard configurations.

Resolves #20907

Changes

• Add routing field to Translog.Delete with new serialization format FORMAT_ROUTING = 8
• Version-gated wire serialization (V_3_8_0) for safe rolling upgrades — older nodes never see routing bytes
• Store routing in Lucene delete tombstones via RoutingFieldMapper in deleteTombstoneMetadataFieldMappers
• Reconstruct routing from tombstone stored fields in LuceneChangesSnapshot
• Thread routing through Engine.Delete, IndexShard, TransportShardBulkAction, and all IndexerEngineOperations implementors
• Backward-compatible: deprecated overloads for IndexShard.applyDeleteOperationOnPrimary/OnReplica, default method for IndexerEngineOperations.prepareDelete with routing

Wire compatibility

The version gate selects the serialization format based on the receiving node's version (StreamOutput.getVersion()). Nodes < 3.8.0 receive FORMAT_NO_DOC_TYPE (no routing bytes). Local translog always uses Version.CURRENT, so on-disk format is upgraded transparently on the writing node.

Null routing

When routing is null (the common case), no routing bytes are written to the tombstone or translog — fully backward compatible with existing data and code paths.

Test plan

• LocalTranslogTests.testDeleteRoutingTranslogRoundTrip — write/read routing through real translog file
• LocalTranslogTests.testDeleteRoutingBackwardCompatibility — old wire version drops routing, new version preserves it
• LocalTranslogTests.testTranslogOpSerialization — version-gated round-trip with random wire versions
• LuceneChangesSnapshotTests.testDeleteRoutingSerialization — Engine.Delete → Translog.Delete routing round-trip
• All existing translog and engine tests pass

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 802edd2)

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/indices/pollingingest/MessageProcessorRunnable.java [299]: routing not available from ingestion message; thread through if pull-based ingestion adds support

🔀 No multiple PR themes

⚡ Recommended focus areas for review Wire Format Version Gate The write() method selects FORMAT_ROUTING only when the peer is on or after V_3_8_0, but the PR description states this is a 3.4.0 change (see @Deprecated(since = "3.4.0") on IndexShard.prepareDelete). If the intended release is 3.4.0, the version gate here (and in the test testDeleteRoutingBackwardCompatibility) targets the wrong version, meaning routing bytes will not be sent to 3.4.0–3.7.x nodes and CDC replication will silently drop routing on those versions. Verify the target version constant matches the actual release version. private void write(final StreamOutput out) throws IOException { final int format; if (out.getVersion().onOrAfter(Version.V_3_8_0)) { format = SERIALIZATION_FORMAT; } else if (out.getVersion().onOrAfter(Version.V_2_0_0)) { format = FORMAT_NO_DOC_TYPE; } else { format = FORMAT_NO_VERSION_TYPE; } out.writeVInt(format); if (format < FORMAT_NO_DOC_TYPE) { out.writeString(MapperService.SINGLE_MAPPING_NAME); } out.writeString(id); if (format < FORMAT_NO_DOC_TYPE) { out.writeString(IdFieldMapper.NAME); out.writeBytesRef(Uid.encodeId(id)); } out.writeLong(version); if (format < FORMAT_NO_VERSION_TYPE) { out.writeByte(VersionType.EXTERNAL.getValue()); } out.writeLong(seqNo); out.writeLong(primaryTerm); if (format >= FORMAT_ROUTING) { out.writeOptionalString(routing); } Possible Issue In the segrep/remote-node branch of applyDeleteOperationOnReplica, an Engine.Delete is constructed directly with the routing parameter and passed to getIndexer().delete(delete). However, this bypasses engine.prepareDelete, so any indexer that overrides prepareDelete to add behavior (e.g. logging, transformations) is skipped. More importantly, the tombstone document creation path in InternalEngine.deleteInLucene calls newDeleteTombstoneDoc(delete.id(), delete.routing()) — this works, but ensure all Indexer implementations handle a Delete constructed with routing correctly rather than only those going through prepareDelete. if (indexSettings.isSegRepEnabledOrRemoteNode()) { final Engine.Delete delete = new Engine.Delete( id, new Term(IdFieldMapper.NAME, Uid.encodeId(id)), seqNo, opPrimaryTerm, version, null, Engine.Operation.Origin.REPLICA, System.nanoTime(), UNASSIGNED_SEQ_NO, 0, routing ); return getIndexer().delete(delete); } Routing Reconstruction Dependency Translog.Delete is reconstructed from tombstone stored fields via fields.routing(). This relies on RoutingFieldMapper being registered in deleteTombstoneMetadataFieldMappers (done in DocumentMapper) AND on the field visitor reading routing from stored fields. If the tombstone was written before this change (existing tombstones in Lucene), fields.routing() will return null even for deletes that originally had routing, which is expected but should be documented. Also verify that RoutingFieldMapper actually stores the routing value as a stored field in the tombstone doc; otherwise fields.routing() will always be null. final boolean isTombstone = parallelArray.isTombStone[docIndex]; if (isTombstone && fields.id() == null) { op = new Translog.NoOp(seqNo, primaryTerm, fields.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 { final String id = fields.id(); if (isTombstone) { op = new Translog.Delete(id, seqNo, primaryTerm, version, fields.routing()); assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Delete op but soft_deletes field is not set [" + op + "]"; } else { final BytesReference source = fields.source(); if (source == null) {

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 802edd2
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid silently dropping routing in default The default implementation silently discards the routing argument, which defeats the purpose of this PR for any IndexerEngineOperations implementer that does not override it. Since IndexShard.applyDeleteOperation now always calls the routing-aware overload, implementations relying on the default will lose routing on the tombstone. Consider making the routing-aware method the primary abstract method and providing the default in the opposite direction, or removing the default entirely. server/src/main/java/org/opensearch/index/engine/exec/IndexerEngineOperations.java [111-123] -default Engine.Delete prepareDelete( +Engine.Delete prepareDelete( String id, String routing, long seqNo, long primaryTerm, long version, VersionType versionType, Engine.Operation.Origin origin, long ifSeqNo, long ifPrimaryTerm +); + +default Engine.Delete prepareDelete( + String id, + long seqNo, + long primaryTerm, + long version, + VersionType versionType, + Engine.Operation.Origin origin, + long ifSeqNo, + long ifPrimaryTerm ) { - return prepareDelete(id, seqNo, primaryTerm, version, versionType, origin, ifSeqNo, ifPrimaryTerm); + return prepareDelete(id, null, seqNo, primaryTerm, version, versionType, origin, ifSeqNo, ifPrimaryTerm); } Suggestion importance[1-10]: 7 __ Why: Valid concern: the default implementation silently discards routing, defeating the PR's purpose for implementers that don't override it. Inverting the default direction would ensure routing is preserved by default.	Medium
	Detect silent routing drop on old wire versions If a delete with a non-null routing is written using a pre-3.8 wire format, the routing is silently dropped, which can cause data loss/inconsistency during CDC or cross-version replication. Consider logging a warning (or asserting) when routing != null but the negotiated format is older than FORMAT_ROUTING, so callers become aware that routing cannot be preserved on the wire. server/src/main/java/org/opensearch/index/translog/Translog.java [1543-1550] private void write(final StreamOutput out) throws IOException { final int format; if (out.getVersion().onOrAfter(Version.V_3_8_0)) { format = SERIALIZATION_FORMAT; } else if (out.getVersion().onOrAfter(Version.V_2_0_0)) { format = FORMAT_NO_DOC_TYPE; } else { format = FORMAT_NO_VERSION_TYPE; } + assert format >= FORMAT_ROUTING || routing == null + : "routing [" + routing + "] cannot be serialized with format [" + format + "]"; Suggestion importance[1-10]: 6 __ Why: Valid concern about silent data loss during cross-version replication when routing is dropped on older wire formats. Adding an assertion or warning would aid debugging, but this is a minor defensive check.	Low
General	Include optional-string header in size estimate The estimated size no longer accounts for the extra byte(s) written by writeOptionalString (the null/non-null marker) nor the vInt length prefix; and it also omitted these previously. Since translog capacity accounting relies on this estimate, include a small constant overhead for the optional-string header to avoid systematic under-estimation now that routing is serialized. server/src/main/java/org/opensearch/index/translog/Translog.java [1507-1510] @Override public long estimateSize() { return (id.length() * 2) + (3 * Long.BYTES) // seq_no, primary_term, and version + + 1 // optional-string presence byte for routing + (routing != null ? 2 * routing.length() : 0); } Suggestion importance[1-10]: 3 __ Why: Minor accuracy improvement to size estimation; the impact on translog capacity accounting is negligible for a single byte overhead.	Low
	Align write-allowed check across replica paths This SegRep replica branch bypasses ensureWriteAllowed(origin) that the non-SegRep branch performs via applyDeleteOperation. With the new routing parameter now flowing here, ensure this asymmetry is intentional; otherwise replicas under SegRep may accept deletes when the shard is not in a valid state for writes. Consider aligning with the non-SegRep path or explicitly documenting the difference. server/src/main/java/org/opensearch/index/shard/IndexShard.java [1468-1483] if (indexSettings.isSegRepEnabledOrRemoteNode()) { + ensureWriteAllowed(Engine.Operation.Origin.REPLICA); final Engine.Delete delete = new Engine.Delete( id, new Term(IdFieldMapper.NAME, Uid.encodeId(id)), seqNo, opPrimaryTerm, version, null, Engine.Operation.Origin.REPLICA, System.nanoTime(), UNASSIGNED_SEQ_NO, 0, routing ); return getIndexer().delete(delete); } Suggestion importance[1-10]: 3 __ Why: The asymmetry between the SegRep and non-SegRep replica branches predates this PR and is not introduced by the routing changes. The suggestion is somewhat out of scope.	Low

---

Previous suggestions

Suggestions up to commit 6dccd00

Category	Suggestion	Impact
General	Guard against silent routing loss on downgrade When serializing to an older version that does not support the routing field, silently dropping a non-null routing value can lead to data inconsistency between primary and replicas during mixed-version cluster operations (e.g., rolling upgrades). Consider logging a warning or asserting that routing == null when downgrading the format, so this loss is detectable rather than silent. server/src/main/java/org/opensearch/index/translog/Translog.java [1543-1550] final int format; if (out.getVersion().onOrAfter(Version.V_3_8_0)) { format = SERIALIZATION_FORMAT; } else if (out.getVersion().onOrAfter(Version.V_2_0_0)) { + assert routing == null : "Cannot serialize Delete with routing [" + routing + "] to older version " + out.getVersion(); format = FORMAT_NO_DOC_TYPE; } else { + assert routing == null : "Cannot serialize Delete with routing [" + routing + "] to older version " + out.getVersion(); format = FORMAT_NO_VERSION_TYPE; } Suggestion importance[1-10]: 7 __ Why: Adding assertions to detect silent routing loss during downgrade serialization is a valid concern for mixed-version cluster correctness, though assertions only fire in debug mode and may not fully prevent the issue in production.	Medium
	Fix size estimate for optional routing The estimate omits the additional bytes that writeOptionalString always emits (a boolean marker byte plus a vInt length prefix), so the size estimate is off even when routing is null. Include a small constant to account for the optional-string marker to keep translog size accounting accurate. server/src/main/java/org/opensearch/index/translog/Translog.java [1507-1510] @Override public long estimateSize() { return (id.length() * 2) + (3 * Long.BYTES) // seq_no, primary_term, and version + + 1 // routing optional-string marker + (routing != null ? 2 * routing.length() : 0); } Suggestion importance[1-10]: 4 __ Why: The suggestion correctly notes that writeOptionalString writes an additional marker byte, making the estimate slightly inaccurate. However, estimateSize is only an approximation and the impact is minor.	Low

[github-actions]

Persistent review updated to latest commit 802edd2

[github-actions]

❌ Gradle check result for 802edd2: 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?