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

consensus: fix expected condition variable exception #5734

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Jul 29, 2022

The Seastar docs[1] say wait() returns condition_variable_timed_out, not
timed_out_error. This resulted in error logs like:

INFO  2022-07-29 00:03:16,777 [shard 0] raft - [group_id:5, {kafka/topic-fhmhqjvhxd/4}] consensus.cc:2757 - Starting leadership transfer from {id: {5}, revision: {32}} to {id: {4}, revision: {32}} in term 1
INFO  2022-07-29 00:03:16,777 [shard 0] raft - [group_id:5, {kafka/topic-fhmhqjvhxd/4}] consensus.cc:2660 - transfer leadership: waiting for node {id: {4}, revision: {32}} to catch up
...
ERROR 2022-07-29 00:03:26,777 [shard 0] rpc - Service handler threw an exception: seastar::condition_variable_timed_out (Condition variable timed out)
// ... no corresponding message about the end of the leadership transfer

[1] https://docs.seastar.io/master/classseastar_1_1condition__variable.html#a69703300a8e56a520269062f062aebda

Partially fixes #5378
I think this also fixes #5461

The Seastar docs[1] say wait() returns condition_variable_timed_out, not
timed_out_error. This resulted in error logs like:

```
INFO  2022-07-29 00:03:16,777 [shard 0] raft - [group_id:5, {kafka/topic-fhmhqjvhxd/4}] consensus.cc:2757 - Starting leadership transfer from {id: {5}, revision: {32}} to {id: {4}, revision: {32}} in term 1
INFO  2022-07-29 00:03:16,777 [shard 0] raft - [group_id:5, {kafka/topic-fhmhqjvhxd/4}] consensus.cc:2660 - transfer leadership: waiting for node {id: {4}, revision: {32}} to catch up
...
ERROR 2022-07-29 00:03:26,777 [shard 0] rpc - Service handler threw an exception: seastar::condition_variable_timed_out (Condition variable timed out)
// ... no corresponding message about the end of the leadership transfer
```

[1] https://docs.seastar.io/master/classseastar_1_1condition__variable.html#a69703300a8e56a520269062f062aebda
@andrwng
Copy link
Contributor Author

andrwng commented Jul 29, 2022

I took a quick look around the codebase for other usages of ss:timed_out_error and looks like our usage is correct elsewhere (usually accompanied by with_timeout() rather than a condition variable)

@piyushredpanda
Copy link
Contributor

Awesome find, @andrwng!

Copy link
Contributor

@ajfabbri ajfabbri left a comment

Choose a reason for hiding this comment

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

Nice work. +1 (pending CI)

@andrwng andrwng merged commit 6e6ab2e into redpanda-data:dev Jul 29, 2022
@andrwng andrwng deleted the raft-cv-exception branch July 29, 2022 23:38
@andrwng
Copy link
Contributor Author

andrwng commented Jul 30, 2022

/backport v22.1.x

@andrwng
Copy link
Contributor Author

andrwng commented Jul 30, 2022

/backport v21.11.x

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.

excellent find!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants