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

cluster/tests: add test case for eviction of the tx coordinator #17379

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Mar 25, 2024

We've seen in older versions of Redpanda that despite the 'delete' cleanup policy being enabled, eviction may not be triggered. This is because in older versions of Redpanda[1], we would not register the eviction STM for tx coordinator partitions.

NOTE: this bug[1] doesn't exist on dev, but does in v23.3, and not in v23.2[2].

[1]

if (is_tx_manager_topic(_raft->ntp()) && _is_tx_enabled) {
_tm_stm = builder.create_stm<cluster::tm_stm>(
clusterlog,
_raft.get(),
_feature_table,
_tm_stm_cache_manager.local().get(_raft->ntp().tp.partition));
_raft->log()->stm_manager()->add_stm(_tm_stm);
co_return co_await _raft->start(std::move(builder));
}
if (is_transform_offsets_topic(_raft->ntp())) {
vassert(
topic_cfg.has_value(),
"No topic configuration passed, stm requires configuration for "
"partition count.");
_transform_offsets_stm = builder.create_stm<transform_offsets_stm_t>(
topic_cfg->partition_count, clusterlog, _raft.get());
}
/**
* Data partitions
*/
if (!storage::deletion_exempt(_raft->ntp())) {
_log_eviction_stm = builder.create_stm<cluster::log_eviction_stm>(
_raft.get(), clusterlog, _kvstore);
_raft->log()->stm_manager()->add_stm(_log_eviction_stm);
}

[2]
} else if (is_tx_manager_topic(_raft->ntp())) {
if (
_raft->log_config().is_collectable()
&& !storage::deletion_exempt(_raft->ntp())) {
_log_eviction_stm = ss::make_shared<cluster::log_eviction_stm>(
_raft.get(), clusterlog, _as, _kvstore);
stm_manager->add_stm(_log_eviction_stm);
}
if (_is_tx_enabled) {
auto tm_stm_cache = _tm_stm_cache_manager.local().get(
_raft->ntp().tp.partition);
_tm_stm = ss::make_shared<cluster::tm_stm>(
clusterlog, _raft.get(), feature_table, tm_stm_cache);
stm_manager->add_stm(_tm_stm);
}

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
  • v23.3.x
  • v23.2.x

Release Notes

  • none

bharathv
bharathv previously approved these changes Mar 25, 2024
dotnwat
dotnwat previously approved these changes Mar 25, 2024
@dotnwat
Copy link
Member

dotnwat commented Mar 25, 2024

@andrwng should the test be backported to 23.2 like it is to dev even though neither contain the bug?

@andrwng
Copy link
Contributor Author

andrwng commented Mar 25, 2024

@andrwng should the test be backported to 23.2 like it is to dev even though neither contain the bug?

Ah yeah good point. Will do

@vbotbuildovich
Copy link
Collaborator

new failures in https://buildkite.com/redpanda/redpanda/builds/46736#018e77ed-ce61-4836-8d93-a79aa471f067:

"rptest.tests.partition_move_interruption_test.PartitionMoveInterruption.test_cancelling_partition_move.replication_factor=3.unclean_abort=False.recovery=restart_recovery.compacted=False"

@andrwng
Copy link
Contributor Author

andrwng commented Mar 26, 2024

Agh, looks like the test runs with multiple cores. I'll pull it out into its own binary with -c 1

We've seen in older versions of Redpanda that despite the 'delete'
cleanup policy being enabled, eviction may not be triggered. This is
because in older versions of Redpanda[1], we would not register the
eviction STM for tx coordinator partitions.

[1] https://github.com/redpanda-data/redpanda/blob/3ce012f9ccb64810eafcc4b8b2e0df7340172879/src/v/cluster/partition.cc#L446-L471
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Mar 26, 2024

@andrwng andrwng merged commit 6faf315 into redpanda-data:dev Mar 26, 2024
17 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-17379-v23.2.x-418 remotes/upstream/v23.2.x
git cherry-pick -x 761dfe987b3e4a97016ecbe966cc55c432aab3df

Workflow run logs.

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-17379-v23.3.x-879 remotes/upstream/v23.3.x
git cherry-pick -x 761dfe987b3e4a97016ecbe966cc55c432aab3df

Workflow run logs.

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.

None yet

5 participants