Promote lowest-version replica on doc-rep failover and make node_version decider Lucene-aware

[ZiwenWan]

Description

During a rolling upgrade with document replication, a shard's replicas can become permanently unassignable even when there is no real segment-format incompatibility between the nodes involved. This PR removes two constraints that are stricter than Lucene actually requires. Details and discussion in #22520.

1. Document-replication failover promotes the highest-version replica.

RoutingNodes#unassignPrimaryAndPromoteActiveReplicaIfExists branches on replication type and calls activeReplicaWithHighestVersion for document replication, so each failover ratchets the primary onto the newest node
version present. But NodeVersionAllocationDecider enforces replica.version >= primary.version -- so once the primary has been promoted onto an upgraded node, that shard's replicas are refused on every not-yet-upgraded node. Promote-highest directly contradicts the invariant the decider is trying to maintain.

Segment replication already promotes the oldest version (activeReplicaWithOldestVersion, added in #9536). The promote-highest rule dates to elastic/elasticsearch#25277 (2017) and existed only because sequence numbers were then new and older nodes could not interpret them. That hazard does not exist in any currently supported version skew, so both replication types can share the same rule.

Note this promotion runs inside failShard and is therefore not gated by EnableAllocationDecider -- setting cluster.routing.allocation.enable: primaries during an upgrade does not prevent it.

2. The decider compares OpenSearch version ids, not Lucene versions.

The constraint the decider exists to enforce is a Lucene segment-format constraint -- its own javadoc says so, and the failure it prevents is IndexFormatTooNewException. But it compares Version ids. An OpenSearch patch upgrade that changes no Lucene minor is therefore treated as incompatible even though the on-disk format is byte-for-byte the same generation. Version#luceneVersion is already available on Version, so comparing it is a small change.

Real-world impact

Reproduced on a production cluster during a 2.19.0 -> 2.19.4 patch upgrade: a .plugins-ml-config replica stuck UNASSIGNED with _cluster/allocation/explain citing node_version, with the primary on 2.19.4 and the candidate node on 2.19.0. Both run Lucene 9.12.x, so the block is a false positive -- there is no format difference to protect against.

Changes

RoutingNodes -- promote the lowest-version in-sync replica for both replication types; activeReplicaWithHighestVersion is now unused and removed.

NodeVersionAllocationDecider -- add a Lucene-compatibility check:

private static boolean isLuceneVersionCompatible(Version target, Version source) {
    org.apache.lucene.util.Version targetLucene = target.luceneVersion;
    org.apache.lucene.util.Version sourceLucene = source.luceneVersion;
    return targetLucene.major > sourceLucene.major
        || (targetLucene.major == sourceLucene.major && targetLucene.minor >= sourceLucene.minor);
}

OR'd with the existing onOrAfter check in isVersionCompatibleAllocatingReplica, isVersionCompatibleRelocatePrimary,
the snapshot-restore isVersionCompatible, and the segment-replication block in canAllocate. Decision messages were updated so _cluster/allocation/explain still explains which condition applied.

A Lucene minor bump changes the codec/segment format; a Lucene patch bump does not. Keying on major.minor is therefore the correct granularity -- this is not equivalent to "ignore the OpenSearch patch version." OpenSearch patch releases can and do bump the Lucene patch (2.19.2/3/4 -> 9.12.2/9.12.3), but never the Lucene minor. Genuine Lucene minor and major gaps remain blocked.

Related Issues

Fixes #22520

Check List

• Functionality includes testing.
  • FailedShardsRoutingTests: testReplicaOnOldestVersionIsPromotedDocRep /
    testReplicaOnOldestVersionIsPromotedSegRep now assert oldest-version
    promotion for both replication types.
  • NodeVersionAllocationDeciderTests: added
    testAllocatesReplicaOnSameLuceneMinorDifferentOpenSearchPatch (2.19.4
    primary -> 2.19.0 target, both Lucene 9.12.x, expect YES),
    testDoesNotAllocateReplicaOnOlderLuceneMinor (9.12 -> 9.11, expect NO),
    testDoesNotAllocateReplicaAcrossOlderLuceneMajor (Lucene 10 -> 9, expect
    NO). Existing testMessages assertions updated for the new wording.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a8cbcd6)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Inverted Logic in Replica Allocation In isVersionCompatibleAllocatingReplica, the condition now allows allocation when the target is onOrAfter the source OR when Lucene versions are compatible. However, isLuceneVersionCompatible(target, source) returns true when target's Lucene major.minor is >= source's. This means if target is on an older OpenSearch version but happens to share the same Lucene major.minor as the source (primary), the check returns YES. That is the intended relaxation, but the same || pattern means an older OpenSearch node with the same Lucene minor is allowed — this seems intentional, but the YES-branch explanation mentions "equal-or-newer than, or Lucene-compatible" which is misleading when the target is actually older than the source. Verify the decision message accurately reflects the older-but-Lucene-compatible case for troubleshooting/support. if (target.node().getVersion().onOrAfter(source.node().getVersion()) || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { /* we can allocate if we can recover from a node that is younger or on the same version, or if the * target's Lucene version can still read the source's segments (e.g. an OpenSearch patch-level * difference that shares the same Lucene major.minor). If the primary is already running a newer * Lucene major/minor that won't work due to possible differences in the lucene index format etc. */ return allocation.decision( Decision.YES, NAME, "can allocate replica shard to a node with version [%s] since this is equal-or-newer than, or Lucene-compatible with, the primary version [%s]", target.node().getVersion(), source.node().getVersion() ); } else { return allocation.decision( Decision.NO, NAME, "cannot allocate replica shard to a node with version [%s] since this is older than, and Lucene-incompatible with, the primary version [%s]", target.node().getVersion(), source.node().getVersion() ); } Segment Replication Guard Weakened For segment replication, the previous code blocked relocating the primary to any node newer than any replica. The new code only blocks when target is newer AND Lucene-incompatible with the replica. If two OpenSearch versions share the same Lucene minor but differ in codecs enabled only at a specific OpenSearch version (or feature-flag gated codec/field types), the replica may still fail to read newer segments. Relying purely on Lucene major.minor may be insufficient in cases where OpenSearch-specific codecs/plugins were introduced within the same Lucene minor. Confirm no such OpenSearch-only codec version-gating exists. // With segment replication the replica continuously reads segments written by the primary, // so the replica's Lucene version must be able to read the primary's for the life of the // shard. Block a newer target unless the replica can still read what it would write (e.g. // an OpenSearch patch-level difference that shares the same Lucene major.minor). RoutingNode replicaNode = allocation.routingNodes().node(replica.currentNodeId()); if (node.node().getVersion().after(replicaNode.node().getVersion()) && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { return allocation.decision( Decision.NO, NAME, "When segment replication is enabled, cannot relocate primary shard to a node with version [%s] if it has a replica on older, Lucene-incompatible version [%s]", node.node().getVersion(), replicaNode.node().getVersion() ); }

[github-actions]

Persistent review updated to latest commit a2e24ab

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a8cbcd6

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Keep fallback to highest-version replica Switching doc-rep primary promotion to the oldest-version replica reverses long-standing behavior and can cause issues during downgrades of shard state written by a newer primary (translog/checkpoints written in a newer format). Consider whether the oldest replica can safely take over as primary for doc-rep in all cases (e.g., ensure it has all in-sync operations and its Lucene/translog format can read what the failed primary produced). If not, restrict this change to cases where the oldest replica is Lucene-compatible with the failed primary, otherwise fall back to the highest-version replica. server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java [808-811] -// Promote the oldest-version in-sync replica for both document and segment replication so the -// new primary stays at the minimum version present, keeping it a valid recovery source/target for -// any not-yet-upgraded node during a rolling upgrade. +// Prefer the oldest-version in-sync replica when it can still read the failed primary's segments, +// so replicas on not-yet-upgraded nodes remain valid allocation targets during a rolling upgrade. activeReplica = activeReplicaWithOldestVersion(failedShard.shardId()); +if (activeReplica == null) { + activeReplica = activeReplicaWithHighestVersion(failedShard.shardId()); +} Suggestion importance[1-10]: 6 __ Why: Raises a legitimate concern about the behavior change for doc-rep primary promotion, though the recommended fallback may itself have correctness implications that would need careful review.	Low
	Differentiate explanation messages for YES cases In isVersionCompatibleRelocatePrimary, when target is older than source, calling isLuceneVersionCompatible(target, source) will return true whenever target's Lucene major/minor is >= source's. But if target OS version is older than source, target's Lucene is typically older too, so this correctly returns false; however the "YES" explanation message now says "equal-or-newer, Lucene-compatible" which is misleading when the target is older but Lucene-compatible. Consider producing distinct explanations for the two YES cases (equal-or-newer vs older-but-Lucene-compatible) to aid debugging. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [144-152] -if (target.node().getVersion().onOrAfter(source.node().getVersion()) - || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { +if (target.node().getVersion().onOrAfter(source.node().getVersion())) { + return allocation.decision(Decision.YES, NAME, + "can relocate primary shard from a node with version [%s] to a node with equal-or-newer version [%s]", + source.node().getVersion(), target.node().getVersion()); +} else if (isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { + return allocation.decision(Decision.YES, NAME, + "can relocate primary shard from a node with version [%s] to a node with older but Lucene-compatible version [%s]", + source.node().getVersion(), target.node().getVersion()); +} Suggestion importance[1-10]: 3 __ Why: A reasonable readability/debugging improvement for the explanation messages, but not functionally important.	Low
Possible issue	Guard against null Lucene versions Version.luceneVersion may be null for some unknown/mocked Version instances (e.g. in tests or when a node advertises an unrecognized version). Direct field access will NPE and fail allocation decisions. Guard against nulls and fall back to a conservative "not compatible" decision so behavior degrades safely. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [80-84] org.apache.lucene.util.Version targetLucene = target.luceneVersion; org.apache.lucene.util.Version sourceLucene = source.luceneVersion; +if (targetLucene == null || sourceLucene == null) { + return false; +} return targetLucene.major > sourceLucene.major || (targetLucene.major == sourceLucene.major && targetLucene.minor >= sourceLucene.minor); Suggestion importance[1-10]: 5 __ Why: Defensive null-check for luceneVersion could prevent NPEs in edge cases, though in practice Version.luceneVersion is generally non-null for known versions.	Low
	Verify argument order for Lucene compatibility check The arguments to isLuceneVersionCompatible appear swapped. The method is documented as isLuceneVersionCompatible(target, source) returning true when target can read segments written by source. Here we want to know whether the replica (target reader) can read segments written by the primary on the new node (source writer), so the call should be isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) — currently the arguments are in the correct order, but the naming above matches; double-check by verifying the check for segrep primary allocation: replica must be able to read primary's segments, so target=replica, source=primary-node. Confirm/rename or fix accordingly to avoid inverted logic. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [101-102] +// target=replica (reader), source=candidate primary node (writer) if (node.node().getVersion().after(replicaNode.node().getVersion()) && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { Suggestion importance[1-10]: 2 __ Why: The suggestion is confused - it asserts the arguments might be swapped but then concludes they are correct, and provides improved_code identical to existing_code. Low value.	Low

---

Previous suggestions

Suggestions up to commit a8cbcd6

Category	Suggestion	Impact
Possible issue	Tighten Lucene compatibility to same minor isLuceneVersionCompatible(target, source) returns true whenever target's Lucene major.minor is >= source's, including cases where target's OpenSearch version is strictly older but happens to bundle the same-or-newer Lucene minor. This can allow relocating a primary to an older OpenSearch node purely because Lucene versions align, which was not the stated intent (patch-level differences on the same Lucene minor). Consider also requiring that target and source share the same Lucene major.minor, or otherwise restrict to the "same Lucene minor" case rather than "target Lucene >= source Lucene". server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [144-149] if (target.node().getVersion().onOrAfter(source.node().getVersion()) - || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { + || (target.node().getVersion().luceneVersion.major == source.node().getVersion().luceneVersion.major + && target.node().getVersion().luceneVersion.minor == source.node().getVersion().luceneVersion.minor)) { return allocation.decision( Decision.YES, NAME, "can relocate primary shard from a node with version [%s] to a node with equal-or-newer, Lucene-compatible version [%s]", Suggestion importance[1-10]: 4 __ Why: The concern about allowing relocation to an older OpenSearch node when Lucene versions happen to align is valid in principle, but the PR's intent is specifically about Lucene format compatibility. The proposed tighter check may over-restrict legitimate cases.	Low
General	Verify behavior change safety for doc-rep Promoting the oldest-version replica for document replication changes long-standing failover behavior and could regress rolling-upgrade scenarios where the primary intentionally moves forward in version. Confirm that BWC tests and rolling-upgrade flows still succeed, and consider gating this behavior change behind a version check or feature flag so mixed clusters upgrading from older majors are not affected in unexpected ways. server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java [807-812] +if (activeReplica == null) { + // Promote the oldest-version in-sync replica for both document and segment replication so the + // new primary stays at the minimum version present, keeping it a valid recovery source/target for + // any not-yet-upgraded node during a rolling upgrade. + activeReplica = activeReplicaWithOldestVersion(failedShard.shardId()); +} - Suggestion importance[1-10]: 3 __ Why: The suggestion only asks for verification and the improved_code is identical to existing_code. While the concern about behavior change is valid, it doesn't propose a concrete fix.	Low

Suggestions up to commit a229718

Category	Suggestion	Impact
General	Restrict Lucene-compat relaxation to patch differences isLuceneVersionCompatible returns true whenever target's Lucene major.minor is >= source's, which is a superset of the onOrAfter check and also returns true for cases like target OpenSearch older but same Lucene minor. This effectively renders the onOrAfter clause redundant and, more importantly, allows relocating a primary to an OLDER OpenSearch patch node as long as Lucene minors match — but the "NO" branch's error message says "older" implying that was previously forbidden. Confirm that allocating a primary to a strictly older OpenSearch patch is intended; if not, tighten the condition to require onOrAfter OR (older-by-patch-only AND same Lucene minor). server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [144-145] +if (target.node().getVersion().onOrAfter(source.node().getVersion()) + || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { - Suggestion importance[1-10]: 5 __ Why: Raises a legitimate concern that the relaxed condition may allow relocation to older OpenSearch patch nodes while error messages still say "older", but the improved_code is identical to existing_code, providing no fix.	Low
	Verify resync semantics after oldest-version promotion Promoting the oldest-version replica unconditionally can select a replica whose Lucene major.minor is older than the failed primary's, but that replica may hold older segments (which is fine) — however after promotion it becomes the source for any newer-version replicas, and those newer replicas' existing state may have been derived from the previous newer primary. Ensure the promotion path handles or reconciles newer replicas correctly (e.g., forces resync), otherwise this can lead to Lucene format issues on the surviving newer replicas. server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java [807-812] +if (activeReplica == null) { + // Promote the oldest-version in-sync replica for both document and segment replication so the + // new primary stays at the minimum version present, keeping it a valid recovery source/target for + // any not-yet-upgraded node during a rolling upgrade. + activeReplica = activeReplicaWithOldestVersion(failedShard.shardId()); +} - Suggestion importance[1-10]: 4 __ Why: Raises a valid consideration about resync of newer replicas after oldest-version promotion, but the improved_code is identical to existing_code and only asks for verification without a concrete change.	Low
Possible issue	Verify argument order for Lucene compatibility check The arguments to isLuceneVersionCompatible appear swapped. Here we want to check whether the existing replica (older node) can read segments produced by the target (newer primary), i.e. isLuceneVersionCompatible(target=replicaNode, source=node). As called, it checks if the older replica's Lucene can read the newer node's segments — but the current arg order passes (replicaNode, node) which under the helper's semantics means "can replicaNode read node's segments", which is the intended check, however the second-arg naming (source) is node (the primary target). Please verify the call direction matches the doc: target must be able to read source. Given the newer primary writes and older replica reads, the correct call is isLuceneVersionCompatible(replicaNode.version, node.version) — confirm and add a test asserting the segrep block still fires when Lucene minors differ. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [101-102] +if (node.node().getVersion().after(replicaNode.node().getVersion()) + && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { - Suggestion importance[1-10]: 3 __ Why: The suggestion is ambiguous—it acknowledges the current call order matches the helper's semantics but asks for verification. The improved_code is identical to existing_code, offering no concrete change.	Low

Suggestions up to commit 190a3f8

Category	Suggestion	Impact
General	Restrict Lucene-compat relaxation to same major This condition will now return YES whenever the target's Lucene major.minor is >= source's, including when the target is on an older OpenSearch version but same Lucene minor. That's the intended relaxation for patch-level differences, but it also allows arbitrary downgrades to any earlier OpenSearch release sharing the same Lucene minor, which may violate other version-compat assumptions. Consider constraining the Lucene-compat branch to only apply within the same OpenSearch major, e.g. requiring target.getVersion().major == source.getVersion().major in addition to Lucene compatibility. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [144-145] if (target.node().getVersion().onOrAfter(source.node().getVersion()) - || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { + || (target.node().getVersion().major == source.node().getVersion().major + && isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion()))) { Suggestion importance[1-10]: 5 __ Why: Adding a same-major guard is a reasonable defensive tightening, though isLuceneVersionCompatible already ensures Lucene format compat which is the underlying concern. The impact is moderate as a safety hardening.	Low
Possible issue	Ensure promoted replica is in-sync Unconditionally promoting the oldest-version replica may pick a replica that is behind the primary's checkpoint on a shard-copy or write-history basis (particularly relevant with remote-store off/segrep). Ensure activeReplicaWithOldestVersion filters to in-sync allocation IDs only (which the existing implementation should already do), and consider preferring an in-sync replica even at a slightly newer version over an out-of-sync older one to avoid data loss on promotion. server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java [807-812] if (activeReplica == null) { - // Promote the oldest-version in-sync replica for both document and segment replication so the - // new primary stays at the minimum version present, keeping it a valid recovery source/target for - // any not-yet-upgraded node during a rolling upgrade. - activeReplica = activeReplicaWithOldestVersion(shardId); + activeReplica = activeReplicaWithOldestVersion(failedShard.shardId()); } Suggestion importance[1-10]: 3 __ Why: The suggestion raises a concern but doesn't propose a concrete code change (improved_code is essentially unchanged), and merely asks the author to verify in-sync filtering behavior.	Low
	Fix swapped Lucene compatibility arguments The arguments to isLuceneVersionCompatible appear swapped for this check. Here we need to verify the replica (older node) can read segments written by the primary on the target (newer node), so the target's Lucene version is the source of writes and the replica must be the reader. The call should be isLuceneVersionCompatible(node.node().getVersion(), replicaNode.node().getVersion()) — i.e. can the replica's Lucene read what the target writes? Currently it asks the inverse, which will incorrectly permit relocations to a newer Lucene minor where the replica cannot read the new segments. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [101-102] +if (node.node().getVersion().after(replicaNode.node().getVersion()) + && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { - Suggestion importance[1-10]: 2 __ Why: The suggestion claims arguments are swapped, but the improved_code is identical to the existing_code, so no actual change is proposed. Also, the analysis is questionable since isLuceneVersionCompatible(target, source) returns true if target can read source's segments, and here we want to check whether replica (older) can read segments written by new primary (target).	Low

Suggestions up to commit 678509d

Category	Suggestion	Impact
General	Tighten Lucene compatibility to same major When target is older than source, isLuceneVersionCompatible(target, source) returns true only when both share the same Lucene major.minor. However, if target has an even older Lucene minor, this still (correctly) evaluates false — but if target.luceneVersion.major > source.luceneVersion.major, it returns true even when target's OpenSearch version is older, which may not be desired for primary relocation. Consider restricting the relaxation to only same-major-newer-or-equal-minor, matching the stated intent of only allowing OpenSearch patch-level differences. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [142-143] if (target.node().getVersion().onOrAfter(source.node().getVersion()) - || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { + || (target.node().getVersion().luceneVersion.major == source.node().getVersion().luceneVersion.major + && target.node().getVersion().luceneVersion.minor >= source.node().getVersion().luceneVersion.minor)) { Suggestion importance[1-10]: 3 __ Why: The concern about cross-major Lucene compatibility is somewhat valid, but the isLuceneVersionCompatible helper's major > branch is unlikely to be reached in practice since OpenSearch versions tie Lucene majors together. The impact is marginal and the suggested change duplicates logic rather than fixing the helper.	Low
Possible issue	Fix reversed Lucene compatibility check direction The condition uses isLuceneVersionCompatible(node, replicaNode), but the concern here is whether the replica node (older) can read segments written by the primary being relocated to node (newer). The compatibility check direction should be isLuceneVersionCompatible(replicaNode, node) — i.e., can the replica's Lucene version read segments from the target node's Lucene version. As currently written, it will always return true whenever node.version is newer, since a newer Lucene can always read older segments, effectively disabling the guard. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [99-100] if (node.node().getVersion().after(replicaNode.node().getVersion()) - && isLuceneVersionCompatible(node.node().getVersion(), replicaNode.node().getVersion()) == false) { + && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { Suggestion importance[1-10]: 2 __ Why: The suggestion misreads the intent. The code guards against relocating a primary to a newer node when a replica is on an older node; isLuceneVersionCompatible(node, replicaNode) checks whether the target (newer) is compatible with the source (older), which matches the pattern used consistently elsewhere in the PR. Reversing it would break the intended semantics.	Low

Suggestions up to commit d611d39

Category	Suggestion	Impact
General	Restrict Lucene relaxation to equal minor isLuceneVersionCompatible returns true whenever target's Lucene major.minor is >= source's, which includes the case where target's OpenSearch version is older than source's but happens to share (or have a newer) Lucene minor. This effectively allows relocating a primary to an older OpenSearch node as long as Lucene minors match, which is broader than the intended "patch-level" relaxation and may bypass other version-based invariants. Consider restricting the relaxation to same Lucene major.minor only (==), rather than >=. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [142-143] if (target.node().getVersion().onOrAfter(source.node().getVersion()) - || isLuceneVersionCompatible(target.node().getVersion(), source.node().getVersion())) { + || (targetLuceneMajorMinorEquals(target.node().getVersion(), source.node().getVersion()))) { return allocation.decision( Decision.YES, NAME, "can relocate primary shard from a node with version [%s] to a node with equal-or-newer, Lucene-compatible version [%s]", Suggestion importance[1-10]: 6 __ Why: Valid concern: the current relaxation allows relocating to an older OpenSearch node whenever its Lucene minor is >= source's, which is broader than the stated "patch-level" intent. Restricting to equal Lucene major.minor would better match the PR's described goal.	Low
	Avoid unconditional oldest-replica promotion Always promoting the oldest-version replica for document replication is a behavioral change that can pin an index at an old version indefinitely if the oldest replica never leaves. Consider guarding this behavior (e.g., only during a detected rolling upgrade, or when replicas span multiple versions) so that in steady-state clusters the previous "highest version" promotion semantics still apply, avoiding unintentional version regression of primaries. server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java [807-812] if (activeReplica == null) { - // Promote the oldest-version in-sync replica for both document and segment replication so the - // new primary stays at the minimum version present, keeping it a valid recovery source/target for - // any not-yet-upgraded node during a rolling upgrade. + // Promote the oldest-version in-sync replica so the new primary stays at the minimum version + // present during a rolling upgrade, keeping it a valid recovery source/target for not-yet-upgraded nodes. activeReplica = activeReplicaWithOldestVersion(failedShard.shardId()); } Suggestion importance[1-10]: 5 __ Why: Raises a legitimate design concern about always promoting the oldest replica, but the PR appears to intentionally make this change for both replication types; the suggestion is speculative without concrete evidence of regression.	Low
Possible issue	Fix inverted Lucene compatibility direction The Lucene-compatibility check here is inverted relative to the other call sites. For segment replication, if the target primary node is newer than a replica, the replica (older Lucene) must be able to read the primary's (newer Lucene) segments, so the check should be isLuceneVersionCompatible(replicaNode, node) (target=replica reading source=primary), not isLuceneVersionCompatible(node, replicaNode). As written, it will incorrectly permit relocation when the replica's Lucene is older than the primary's. server/src/main/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java [99-100] if (node.node().getVersion().after(replicaNode.node().getVersion()) - && isLuceneVersionCompatible(node.node().getVersion(), replicaNode.node().getVersion()) == false) { + && isLuceneVersionCompatible(replicaNode.node().getVersion(), node.node().getVersion()) == false) { return allocation.decision( Decision.NO, NAME, "When segment replication is enabled, cannot relocate primary shard to a node with version [%s] if it has a replica on older, Lucene-incompatible version [%s]", node.node().getVersion(), replicaNode.node().getVersion() ); } Suggestion importance[1-10]: 3 __ Why: The suggestion misreads the semantics: isLuceneVersionCompatible(target, source) returns true if target's Lucene >= source's. Here node is newer than replicaNode, so isLuceneVersionCompatible(node, replicaNode) is naturally true, which is not what the guard needs. The original code's guard is the one that's questionable, but the suggested swap does not clearly fix it and may introduce another issue.	Low

[github-actions]

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

[github-actions]

Persistent review updated to latest commit a2e24ab

[github-actions]

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

[github-actions]

Persistent review updated to latest commit d611d39

[github-actions]

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

[github-actions]

Persistent review updated to latest commit 678509d

[github-actions]

❌ Gradle check result for 678509d: 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?

[github-actions]

Persistent review updated to latest commit 190a3f8

[github-actions]

❌ Gradle check result for 190a3f8: 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?

[github-actions]

Persistent review updated to latest commit a229718

[github-actions]

✅ Gradle check result for a229718: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.51%. Comparing base (b068dd3) to head (a8cbcd6).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...location/decider/NodeVersionAllocationDecider.java	81.81%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22668      +/-   ##
============================================
+ Coverage     71.49%   71.51%   +0.02%     
+ Complexity    77005    77002       -3     
============================================
  Files          6156     6156              
  Lines        358413   358418       +5     
  Branches      52243    52247       +4     
============================================
+ Hits         256237   256328      +91     
+ Misses        81769    81713      -56     
+ Partials      20407    20377      -30     

☔ View full report in Codecov by Harness.
📢 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.

[github-actions]

Persistent review updated to latest commit a8cbcd6

[github-actions]

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

[github-actions]

Persistent review updated to latest commit a8cbcd6

[github-actions]

✅ Gradle check result for a8cbcd6: SUCCESS