Skip to content

Revert "Adding BWC test for remote publication enabled cluster"#20942

Merged
andrross merged 1 commit intoopensearch-project:mainfrom
andrross:revert-dbb6d2e9
Mar 20, 2026
Merged

Revert "Adding BWC test for remote publication enabled cluster"#20942
andrross merged 1 commit intoopensearch-project:mainfrom
andrross:revert-dbb6d2e9

Conversation

@andrross
Copy link
Member

The remote cluster state feature does not support forward compatibility by design. Therefore these rolling upgrade tests are not compatible with the feature because they make no guarantees whether an old-version or new-version node will be elected as cluster manager in the mixed version state. Reverting this commit and will let the remote cluster state experts come back with a non-flaky test.

This reverts commit dbb6d2e from PR #20221

Related Issues

Resolves #20910

Check List

  • Functionality includes testing.

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.

…earch-project#20221)"

The remote cluster state feature does not support forward compatibility
by design. Therefore these rolling upgrade tests are not compatible with
the feature because they make no guarantees whether an old-version or
new-version node will be elected as cluster manager in the mixed version
state. Reverting this commit and will let the remote cluster state
experts come back with a non-flaky test.

This reverts commit dbb6d2e.

Signed-off-by: Andrew Ross <andrross@amazon.com>
@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as this has been particularly painful on the core repo lately. @gargharsh3134 let's avoid hardcoding Version.CURRENT here and choose serialization more intelligently. Perhaps we can get the min node version in the cluster and publish according to that? So in a heterogeneous cluster of 2.19 and 3.x it would publish 2.19 compatible, but once homogenous on 3.x then we can publish according to what that version understands.

@andrross
Copy link
Member Author

FYI @soosinha @vikasvb90 @shwetathareja

let's avoid hardcoding Version.CURRENT here and choose serialization more intelligently

@cwperks I think we both made a change to use minimum cluster version in the serialization (yours, mine). However, properly supporting forward compatibility is probably more complicated than that. Look at just IndexMetadata, there are many places where fromXContent will fail on unknown fields. I believe this is okay as XContent versions of this structure were never sent over the wire between nodes (StreamInput/StreamOutput handles that). I think it was only used for reading to/from disk of the local node, which never had to handle forward compatibility because downgrades are not supported. Also for snapshots, but again we don't support forward compatibility with snapshots. The upshot is that actually supporting forward compatibility may be quite difficult and definitely more involved than the commits that we put together. We definitely need tests to verify backward compatibility though. It would also be nice to prevent forward compatibility failures (i.e. do not let a new-version cluster manager win an election in mixed-version cluster with remote cluster state, or do not allow mixed-version clusters, etc).

@github-actions
Copy link
Contributor

✅ Gradle check result for 888839f: SUCCESS

@codecov
Copy link

codecov bot commented Mar 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.20%. Comparing base (06539a4) to head (888839f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #20942      +/-   ##
============================================
- Coverage     73.27%   73.20%   -0.07%     
+ Complexity    72497    72482      -15     
============================================
  Files          5819     5819              
  Lines        331352   331352              
  Branches      47875    47875              
============================================
- Hits         242784   242577     -207     
- Misses        69065    69325     +260     
+ Partials      19503    19450      -53     

☔ 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.

@andrross andrross merged commit 316c070 into opensearch-project:main Mar 20, 2026
43 of 45 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Cluster Manager Project Board Mar 20, 2026
@andrross andrross deleted the revert-dbb6d2e9 branch March 20, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[BUG] Remote cluster state compatibility failures

2 participants