Feature cleanup #24055

bashtanov · 2024-11-07T11:57:22Z

I mostly started it because of the first commit to always get retriable RPC response when not all services started.
The rest is just cleanup for the sake of removing unneeded code.

Backports Required

Release Notes

none

bashtanov · 2024-11-07T11:57:34Z

/dt

vbotbuildovich · 2024-11-07T17:33:36Z

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#0193076e-5362-4916-ad29-22271c770799:

"rptest.tests.cluster_features_test.FeaturesSingleNodeTest.test_get_features"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#0193076e-5364-47b5-b206-449540932c26:

"rptest.tests.cluster_features_test.FeaturesMultiNodeTest.test_get_features"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#0193076e-535c-4938-99dc-325d973c05d8:

"rptest.tests.controller_snapshot_test.ControllerSnapshotTest.test_upgrade_compat"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#01930788-4f8d-4ffb-99c1-7694a65bc261:

"rptest.tests.controller_snapshot_test.ControllerSnapshotTest.test_upgrade_compat"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#01930788-4f90-4180-b3f7-5a7b6204b707:

"rptest.tests.cluster_features_test.FeaturesSingleNodeTest.test_get_features"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57783#01930788-4f8c-4667-98a0-2c2994c04648:

"rptest.tests.cluster_features_test.FeaturesMultiNodeTest.test_get_features"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57955#01932106-407e-4144-bb6f-399068f79303:

"rptest.tests.tiered_storage_model_test.TieredStorageTest.test_tiered_storage.cloud_storage_type_and_url_style=.CloudStorageType.S3.1.virtual_host.test_case=.TS_Read==True.SegmentRolledByTimeout==True"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/58373#01934b13-e2b4-4de3-9ea2-922f6599cb72:

"rptest.tests.consumer_offsets_consistency_test.ConsumerOffsetsConsistencyTest.test_flipping_leadership"

vbotbuildovich · 2024-11-07T17:33:47Z

Retry command for Build#57783

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/cluster_features_test.py::FeaturesSingleNodeTest.test_get_features
tests/rptest/tests/cluster_features_test.py::FeaturesMultiNodeTest.test_get_features
tests/rptest/tests/controller_snapshot_test.py::ControllerSnapshotTest.test_upgrade_compat

BenPope

My understanding:

features::earliest_version is already v23_3_1
application::post_start_tasks checks against earliest
So we don't need to assert anywhere on any of these features

I basically approve this PR, but think we should wait until the branch of v24.3 to merge it.

src/v/cluster/feature_manager.cc

src/v/cluster/controller_backend.cc

src/v/features/feature_table.h

bashtanov · 2024-11-12T13:58:24Z

@BenPope thanks for your review

we should wait until the branch of v24.3 to merge it

I'm not sure I understand why. features::earliest_version is already v23_3_1, and I'm only removing features that require this or earlier versions. They are also all available immediately, i.e. depend only on the code. So if we start some dev branch code, (whether before or after v24.3 branch) against any allowed data (which should be at least v23.3), we can safely assume the features are available straight away. Or am I missing something?

vbotbuildovich · 2024-11-12T17:09:36Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/57955#01932103-0b84-4b8b-b377-816084c35acd
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/57955#01932106-407d-4b44-8eb4-0473324e669e
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/58373#01934b08-390b-4ff0-8b46-cd1d8ca58bac
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/58452#01934ed6-1b64-4db4-81fc-7d1a10a2670c

BenPope · 2024-11-12T17:11:33Z

src/v/features/feature_table.h

+  "serde_raft_0",
+  "license",
+  "raft_improved_configuration",
+  "raftless_node_status",


I think you're now missing "transaction_ga",

no, it is already there

BenPope · 2024-11-12T17:19:30Z

@BenPope thanks for your review

we should wait until the branch of v24.3 to merge it

I'm not sure I understand why. features::earliest_version is already v23_3_1, and I'm only removing features that require this or earlier versions. They are also all available immediately, i.e. depend only on the code. So if we start some dev branch code, (whether before or after v24.3 branch) against any allowed data (which should be at least v23.3), we can safely assume the features are available straight away. Or am I missing something?

I agree, it should be safe.

You may want to fix some subtasks of CORE-2754

vbotbuildovich · 2024-11-12T17:32:27Z

Retry command for Build#57955

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/tiered_storage_model_test.py::TieredStorageTest.test_tiered_storage@{"cloud_storage_type_and_url_style":[1,"virtual_host"],"test_case":{"name":"(TS_Read == True, SegmentRolledByTimeout == True)"}}

bashtanov · 2024-11-13T11:03:52Z

the failure is https://redpandadata.atlassian.net/issues/CORE-7833

bashtanov · 2024-11-13T11:04:03Z

/ci-repeat 1
tests/rptest/tests/tiered_storage_model_test.py::TieredStorageTest.test_tiered_storage@{"cloud_storage_type_and_url_style":[1,"virtual_host"],"test_case":{"name":"(TS_Read == True, SegmentRolledByTimeout == True)"}}

bashtanov · 2024-11-13T13:57:47Z

/ci-repeat 1
tests/rptest/tests/tiered_storage_model_test.py::TieredStorageTest.test_tiered_storage@{"cloud_storage_type_and_url_style":[1,"virtual_host"],"test_case":{"name":"(TS_Read == True, SegmentRolledByTimeout == True)"}}

bashtanov · 2024-11-15T14:56:20Z

/ci-repeat 1
tests/rptest/tests/tiered_storage_model_test.py::TieredStorageTest.test_tiered_storage@{"cloud_storage_type_and_url_style":[1,"virtual_host"],"test_case":{"name":"(TS_Read == True, SegmentRolledByTimeout == True)"}}

bashtanov · 2024-11-19T14:25:42Z

/dt

dotnwat · 2024-11-19T17:31:01Z

src/v/cluster/feature_manager.cc

@@ -299,20 +299,18 @@ ss::future<> feature_manager::maybe_log_license_check_info() {
        // Shutting down - next iteration will drop out


redpanda/admin/server, cluster/feature_manager: do not check feature:…
…:license
as it is on everywhere now

Commit message subject should not wrap. Body should stand on its own.

oops, sorry, I thought the limit was 80, but it's 50 for header and 72 for body, will fix

mmaslankaprv · 2024-11-20T09:39:59Z

lgtm, afte Noah's suggestion is applied

mmaslankaprv · 2024-11-20T15:29:18Z

do we need to backport it v24.3.x ?

dotnwat

c/rm_stm: remove feature::transaction_ga check

this commit removes an assertion but doesn't explain anything. seems like you may be missing a statement in the commit message.

That's feature::rpc_transport_unknown_errc. On boostrap, when a node doesn't have all RPC services available yet, it may return non-retriable errors on RPC requests. There is already a mechanism thatmakes RPC return a retriable error on unknown method if some services are still yet to start. Unfortunately, this mechanism requires all nodes to support it, so we check for a feature flag before we use it. As a result, non-retriable errors still can sneak through, as checking the feature flag across all nodes takes some time. This is to remove the feature flag check, as current and previous versions definitely support it.

as it is always on in all suppoted versions now

feature::raft_improved_configuration is always on in all suppoted versions now

as it is always on in all suppoted versions now

feature::raftless_node_status is always on in all suppoted versions now

feature::node_id_assignment is always on in all suppoted versions now

feature::replication_factor_change is always on in all suppoted versions

feature::ephemeral_secrets is always on in all suppoted versions now

feature::seeds_driven_bootstrap_capable is always on in all suppoted versions now

it is always on in all suppoted versions now

feature::partition_move_revert_cancel is always on in all suppoted versions now

it is always on in all suppoted versions now

vbotbuildovich · 2024-11-20T21:13:07Z

Retry command for Build#58373

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/consumer_offsets_consistency_test.py::ConsumerOffsetsConsistencyTest.test_flipping_leadership

bashtanov · 2024-11-21T11:50:57Z

/dt

vbotbuildovich · 2024-11-22T15:34:41Z

/backport v24.3.x

github-actions bot added area/build area/redpanda labels Nov 7, 2024

bashtanov force-pushed the feature-cleanup branch from 001d8d6 to 4ae8c7b Compare November 7, 2024 14:09

bashtanov marked this pull request as ready for review November 7, 2024 14:10

bashtanov force-pushed the feature-cleanup branch 2 times, most recently from 00d1de9 to b763ec7 Compare November 7, 2024 15:08

bashtanov force-pushed the feature-cleanup branch from b763ec7 to d1d15c5 Compare November 7, 2024 18:56

bashtanov requested review from ztlpn, andrwng, mmaslankaprv and BenPope November 12, 2024 09:53

BenPope reviewed Nov 12, 2024

View reviewed changes

src/v/cluster/feature_manager.cc Show resolved Hide resolved

src/v/cluster/controller_backend.cc Outdated Show resolved Hide resolved

src/v/features/feature_table.h Outdated Show resolved Hide resolved

src/v/features/feature_table.h Outdated Show resolved Hide resolved

bashtanov force-pushed the feature-cleanup branch from d1d15c5 to b7abd1b Compare November 12, 2024 13:53

BenPope reviewed Nov 12, 2024

View reviewed changes

dotnwat reviewed Nov 19, 2024

View reviewed changes

bashtanov force-pushed the feature-cleanup branch from b7abd1b to 009a3d2 Compare November 20, 2024 14:57

mmaslankaprv approved these changes Nov 20, 2024

View reviewed changes

dotnwat reviewed Nov 20, 2024

View reviewed changes

bashtanov added 15 commits November 20, 2024 17:34

feat/tests: use new features to retire older ones

81f4709

redpanda/admin/server: don't check license feature

247ce02

as it is always on in all suppoted versions now

raft: don't check retired feature

1bc4dc7

feature::raft_improved_configuration is always on in all suppoted versions now

c/rm_stm: remove feature::transaction_ga check

f8f5691

as it is always on in all suppoted versions now

c/tx_gateway_frontend: remove dead code

4314c8d

c/node_status_backend: don't check retired feature

6ab5551

feature::raftless_node_status is always on in all suppoted versions now

c/members_manager: do not check retired feature

3f27b93

feature::node_id_assignment is always on in all suppoted versions now

c/topics_frontend: do not check retired feature

77d6b4d

feature::replication_factor_change is always on in all suppoted versions

c/ephemeral_credential_FE: don't check old feature

748de08

feature::ephemeral_secrets is always on in all suppoted versions now

c/controller: do not check old feature

2ce51dd

feature::seeds_driven_bootstrap_capable is always on in all suppoted versions now

k/server: do not check feature::kafka_gssapi

9d0248c

it is always on in all suppoted versions now

c/controller_backend: do not check old feature

c749379

feature::partition_move_revert_cancel is always on in all suppoted versions now

k/server: do not check feature::node_isolation

61cd125

it is always on in all suppoted versions now

feature_table: remove unused feature flags

40e7af3

bashtanov force-pushed the feature-cleanup branch from 009a3d2 to 40e7af3 Compare November 20, 2024 17:36

bashtanov merged commit ee98e07 into redpanda-data:dev Nov 22, 2024
16 checks passed

vbotbuildovich mentioned this pull request Nov 22, 2024

[v24.3.x] Feature cleanup #24260

Closed

bashtanov mentioned this pull request Nov 26, 2024

Features cleanup v24.3.x #24322

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature cleanup #24055

Feature cleanup #24055

bashtanov commented Nov 7, 2024 •

edited

Loading

bashtanov commented Nov 7, 2024

vbotbuildovich commented Nov 7, 2024 •

edited

Loading

vbotbuildovich commented Nov 7, 2024 •

edited

Loading

BenPope left a comment •

edited

Loading

bashtanov commented Nov 12, 2024

vbotbuildovich commented Nov 12, 2024 •

edited

Loading

BenPope Nov 12, 2024

bashtanov Nov 12, 2024

BenPope commented Nov 12, 2024 •

edited

Loading

vbotbuildovich commented Nov 12, 2024

bashtanov commented Nov 13, 2024

bashtanov commented Nov 13, 2024

bashtanov commented Nov 13, 2024

bashtanov commented Nov 15, 2024

bashtanov commented Nov 19, 2024

dotnwat Nov 19, 2024

bashtanov Nov 20, 2024

mmaslankaprv commented Nov 20, 2024

mmaslankaprv commented Nov 20, 2024

dotnwat left a comment •

edited

Loading

vbotbuildovich commented Nov 20, 2024

bashtanov commented Nov 21, 2024

vbotbuildovich commented Nov 22, 2024

		@@ -299,20 +299,18 @@ ss::future<> feature_manager::maybe_log_license_check_info() {
		// Shutting down - next iteration will drop out

Feature cleanup #24055

Feature cleanup #24055

Conversation

bashtanov commented Nov 7, 2024 • edited Loading

Backports Required

Release Notes

bashtanov commented Nov 7, 2024

vbotbuildovich commented Nov 7, 2024 • edited Loading

vbotbuildovich commented Nov 7, 2024 • edited Loading

Retry command for Build#57783

BenPope left a comment • edited Loading

Choose a reason for hiding this comment

bashtanov commented Nov 12, 2024

vbotbuildovich commented Nov 12, 2024 • edited Loading

BenPope Nov 12, 2024

Choose a reason for hiding this comment

bashtanov Nov 12, 2024

Choose a reason for hiding this comment

BenPope commented Nov 12, 2024 • edited Loading

vbotbuildovich commented Nov 12, 2024

Retry command for Build#57955

bashtanov commented Nov 13, 2024

bashtanov commented Nov 13, 2024

bashtanov commented Nov 13, 2024

bashtanov commented Nov 15, 2024

bashtanov commented Nov 19, 2024

dotnwat Nov 19, 2024

Choose a reason for hiding this comment

bashtanov Nov 20, 2024

Choose a reason for hiding this comment

mmaslankaprv commented Nov 20, 2024

mmaslankaprv commented Nov 20, 2024

dotnwat left a comment • edited Loading

Choose a reason for hiding this comment

vbotbuildovich commented Nov 20, 2024

Retry command for Build#58373

bashtanov commented Nov 21, 2024

vbotbuildovich commented Nov 22, 2024

bashtanov commented Nov 7, 2024 •

edited

Loading

vbotbuildovich commented Nov 7, 2024 •

edited

Loading

vbotbuildovich commented Nov 7, 2024 •

edited

Loading

BenPope left a comment •

edited

Loading

vbotbuildovich commented Nov 12, 2024 •

edited

Loading

BenPope commented Nov 12, 2024 •

edited

Loading

dotnwat left a comment •

edited

Loading