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

Logs should indicate which semaphore timed out or was broken #5489

Closed
ajfabbri opened this issue Jul 15, 2022 · 0 comments · Fixed by #5490
Closed

Logs should indicate which semaphore timed out or was broken #5489

ajfabbri opened this issue Jul 15, 2022 · 0 comments · Fixed by #5490
Assignees
@ajfabbri
Labels
area/redpanda area/tests kind/bug Something isn't working

Comments

@ajfabbri
Copy link
Contributor

ajfabbri commented Jul 15, 2022 •
edited
Loading

When diagnosing test failures like #5461, the logs should give better diagnostics about which type of semaphore timed out or was broken.

First improvement we should make is to use named semaphores widely; they are initialized with a short name string which is inserted into the error message when a semaphore exception is thrown.

Future work would be to further disambiguate; not just type but instance (i.e. for per-partition semaphore, which partition is it?). Also could plumb a name through utility classes like mutex and timed_mutex, etc., such that they are named with a owner-specific name, instead of mutex.
@ajfabbri ajfabbri added the kind/bug Something isn't working label Jul 15, 2022
@ajfabbri ajfabbri self-assigned this Jul 15, 2022
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 15, 2022
Use named semaphores for better debuggability.
6e4f7c5 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Ended up doing a single tree-wide commit, versus picking out the
different uses and the areas their types propagate to. As you can see in
the patch the coupling between the semaphore_units<> type and the named
semaphore specialization causes the choice of semaphore to leak quite a
bit, making a treewide change a good choice.

Fixes: redpanda-data#5489
@ajfabbri ajfabbri mentioned this issue Jul 15, 2022
Merged
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 15, 2022
Use named semaphores for better debuggability.
16c5baf 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Ended up doing a single tree-wide commit, versus picking out the
different uses and the areas their types propagate to. As you can see in
the patch the coupling between the semaphore_units<> type and the named
semaphore specialization causes the choice of semaphore to leak quite a
bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 19, 2022
Use named semaphores for better debuggability.
5eeed06 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Ended up doing a single tree-wide commit, versus picking out the
different uses and the areas their types propagate to. As you can see in
the patch the coupling between the semaphore_units<> type and the named
semaphore specialization causes the choice of semaphore to leak quite a
bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 22, 2022
tree-wide: Use named semaphores for better debuggability.
08cbd82 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 27, 2022
tree-wide: Use named semaphores for better debuggability.
04ffdef 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 27, 2022
tree-wide: Use named semaphores for better debuggability.
13309a2 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 27, 2022
tree-wide: Use named semaphores for better debuggability.
77f28f3 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 27, 2022
tree-wide: Use named semaphores for better debuggability.
91fb244 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Jul 28, 2022
tree-wide: Use named semaphores for better debuggability.
cda0464 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
ajfabbri pushed a commit to ajfabbri/redpanda that referenced this issue Aug 4, 2022
tree-wide: Use named semaphores for better debuggability.
67acec3 
Named semaphore is an enhancement to the usual seastar::semaphore, which
is initialized with a name string that helps to identify which semaphore
it is; when the semaphore raises an exception (i.e. timed out, broken),
the name string is included in the error message.

Tree-wide commit, for two reasons:

1. We need to use named semaphores widely to get the debuggability benefit.
2. The coupling between the semaphore_units<> type and the named
   semaphore specialization causes the choice of semaphore to leak quite a
   bit, making a treewide change easier.

Fixes: redpanda-data#5489
travisdowns added a commit to travisdowns/redpanda that referenced this issue Mar 8, 2024
@travisdowns
utils/mutex: remove unnamed constructor
5e688fd 
Certain structures like semaphore and mutex can be "named" which is
useful to identify a particular structure in log messages, exceptions
or just about anything you can imagine.

When we added support for named mutexes, we did not give names to all
existing mutexes, perhaps because there are a lot of them: the
unnamed version of the constructor was left and remove it was
left as a TODO.

This change crosses that TODO off the list and removes the unnamed
constructor (the prior change in this series named all existing
mutexes that weren't already named).

Issue redpanda-data#5489.
@travisdowns travisdowns mentioned this issue Mar 8, 2024
Merged
6 tasks
travisdowns added a commit to travisdowns/redpanda that referenced this issue Mar 13, 2024
@travisdowns
utils/mutex: remove unnamed constructor
7142caa 
Certain structures like semaphore and mutex can be "named" which is
useful to identify a particular structure in log messages, exceptions
or just about anything you can imagine.

When we added support for named mutexes, we did not give names to all
existing mutexes, perhaps because there are a lot of them: the
unnamed version of the constructor was left and remove it was
left as a TODO.

This change crosses that TODO off the list and removes the unnamed
constructor (the prior change in this series named all existing
mutexes that weren't already named).

Issue redpanda-data#5489.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ajfabbri ajfabbri

Labels
area/redpanda area/tests kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use named semaphores for better debuggability. ajfabbri/redpanda
2 participants
@ajfabbri @mmedenjak