Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm_stm: remove always true assert on transaction_ga feature #24335

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

bharathv
Copy link
Contributor

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

@rockwotj rockwotj merged commit 6233705 into redpanda-data:v24.2.x Nov 27, 2024
17 checks passed
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

why did we remove this assert with no explanation at all?

@bashtanov
Copy link
Contributor

The explanation is it is always true. The feature is always available, as it depends on the code only and it has already been on for quite a few versions.

The background for the change is that feature_table and machinery around it misbehaves under some arguably unsupported scenario played in one of our tests. The scenario is having a node that both has empty storage and is not the highest version among all nodes in the cluster. The test is also under impression I'm not sure whether it is prohibited in the docs, and I wouldn't be surprised if it can happen in production. Say, a cluster is halfway through upgrade, and one node that still has the older version loses its disk. The test was also probably run with an incorrectly versioned build, see https://redpandadata.slack.com/archives/C06MCRY75U7/p1732741186471949

All in all, this change looks legit to me and it makes a test pass too, but seems to hide other problems in RP, tests and CI.

@bharathv bharathv deleted the v24.2.x_remove_assert branch November 27, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants