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.

Sign up for GitHub

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

Jump to bottom

raft/c: fix an indefinite hang in transfer leadership #24404

Merged
merged 2 commits into from
Dec 5, 2024
Merged

raft/c: fix an indefinite hang in transfer leadership #24404

merged 2 commits into from
Dec 5, 2024

Conversation

bharathv
Copy link
Contributor

@bharathv bharathv commented Dec 3, 2024 •
edited
Loading

This PR fixes a timeout in raft leadership transfer request. The timeout is caused by a race condition in raft that results in a stuck recovery_stm, even after losing leadership. Exact sequence of events below:

  • Request to transfer leadership in term - 10
  • Starts a recovery_stm to catchup a follower term - 10
  • recovery_stm detects the follower has already been dispatched the latest log index, blocks on a CV until it hears back from the follower (indefinite wait)
  • A racy step_down from a failed replicate request (stm initiated)
  • A new leader takes over and local term incremented - term - 11
  • recovery_stm (in term 10) is stuck waiting for the CV is signaled (which happens if the node assumes leadership again in the future or at shutdown, an indefinite hang if it never assumes leadership)

This PR signals the CV upon step down to unblock the recovery_stm which then detects it is has to shutdown due to a step down (loss of leadership) and this automatically unblocks the transfer leadership request which was waiting for it.

An unrelated (but deeper) issue this PR avoids fixing is why the step down happens when a transfer is already in progress. In this case it was initiated by an STM invariant that requires a step down upon first failure (required for correctness). One can argue that it is not the right behavior to step down when a transfer is already in progress. The fix for it is likely invasive and has correctness implications at the state machine level (eg: idempotency), so avoiding for now.

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

@bharathv
Copy link
Contributor Author

bharathv commented Dec 3, 2024

/ci-repeat 1
skip-units
release
dt-repeat 20
tests/rptest/chaos_tests/single_fault_test.py::TxSubscribeTest

1 similar comment
@bharathv
Copy link
Contributor Author

bharathv commented Dec 3, 2024

/ci-repeat 1
skip-units
release
dt-repeat 20
tests/rptest/chaos_tests/single_fault_test.py::TxSubscribeTest

@bharathv
Copy link
Contributor Author

bharathv commented Dec 3, 2024

/ci-repeat 1
skip-units
release
dt-repeat=20
tests/rptest/chaos_tests/single_fault_test.py::TxSubscribeTest

@bharathv
Copy link
Contributor Author

bharathv commented Dec 3, 2024

/ci-repeat 1
skip-units
release
dt-repeat=40
tests/rptest/chaos_tests/single_fault_test.py::TxSubscribeTest

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Dec 3, 2024 •
edited
Loading

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59180#01938da2-2f44-4119-b339-a9f789515d7b
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-3629-498d-9926-1981d1ebc9fc
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-3627-405b-bd38-a5dc660bce74
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-362c-4a1e-ac0e-951f86073cd9
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-362e-4948-ba03-87047c5cbc15
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b1-4626-870d-7023ed923a22
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b3-4515-9a2c-ea0f2873d859
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b6-4e21-bd81-f895f8d48378
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b4-45b9-ad08-215f88af399b
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59270#019396fe-eff8-45ef-930c-938ccdfcd941
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59270#01939703-4a8e-4bfc-b5df-cff6c67541ce
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59270#01939703-4a91-43c6-8772-28491b120aa8
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59285#019397d8-7c8b-4ccf-9e84-6ffce3c607a6
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59285#019397d8-7c8e-48be-aadb-2c2c4c267a5e

@bharathv
Copy link
Contributor Author

bharathv commented Dec 3, 2024

/ci-repeat 1
skip-units
release
dt-repeat=40
tests/rptest/chaos_tests/single_fault_test.py::TxSubscribeTest

@bharathv bharathv marked this pull request as ready for review December 4, 2024 03:50
@bharathv bharathv changed the title raft/c: hang in transfer leadership - wip raft/c: hang in transfer leadership Dec 4, 2024
@bharathv bharathv changed the title raft/c: hang in transfer leadership raft/c: fix an indefinite hang in transfer leadership Dec 4, 2024
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Dec 4, 2024 •
edited
Loading

Retry command for Build#59208

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

/ci-repeat 1
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_license_enforcement@{"clean_node_after_recovery":true,"clean_node_before_recovery":false}
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_license_enforcement@{"clean_node_after_recovery":false,"clean_node_before_recovery":false}
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_escape_hatch_license_variable@{"clean_node_before_upgrade":false}
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_license_enforcement@{"clean_node_after_recovery":false,"clean_node_before_recovery":true}
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_license_enforcement@{"clean_node_after_recovery":true,"clean_node_before_recovery":true}
tests/rptest/tests/license_enforcement_test.py::LicenseEnforcementTest.test_escape_hatch_license_variable@{"clean_node_before_upgrade":true}

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Dec 4, 2024 •
edited
Loading

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-362c-4a1e-ac0e-951f86073cd9:

"rptest.tests.license_enforcement_test.LicenseEnforcementTest.test_license_enforcement.clean_node_before_recovery=True.clean_node_after_recovery=False"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/59208#0193900d-362e-4948-ba03-87047c5cbc15:

"rptest.tests.license_enforcement_test.LicenseEnforcementTest.test_escape_hatch_license_variable.clean_node_before_upgrade=True"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b1-4626-870d-7023ed923a22:

"rptest.tests.license_enforcement_test.LicenseEnforcementTest.test_license_enforcement.clean_node_before_recovery=True.clean_node_after_recovery=False"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/59208#01939012-94b3-4515-9a2c-ea0f2873d859:

"rptest.tests.license_enforcement_test.LicenseEnforcementTest.test_escape_hatch_license_variable.clean_node_before_upgrade=True"

@mmaslankaprv
Copy link
Member

/ci-repeat 1

@bharathv
raft/recovery: log line with more context
404c7a2
@bharathv
raft/c: notify waiters on follower state reset
d0a562b 
This typically happens when there is a stepdown and the downstream
consumers like recovery need to know about it.
@bharathv bharathv requested a review from mmaslankaprv December 5, 2024 16:07
@bharathv bharathv merged commit f091e28 into redpanda-data:dev Dec 5, 2024
16 checks passed
@bharathv bharathv deleted the fix_step_down branch December 5, 2024 20:30
@vbotbuildovich
Copy link
Collaborator

/backport v24.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v24.2.x

@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

This was referenced Dec 5, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmaslankaprv mmaslankaprv mmaslankaprv approved these changes

@ztlpn ztlpn ztlpn approved these changes

@bashtanov bashtanov Awaiting requested review from bashtanov

Assignees
No one assigned
Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bharathv @vbotbuildovich @mmaslankaprv @ztlpn