report hung radostrace ops on interrupt - #92
Conversation
|
test result - https://paste.ubuntu.com/p/FvpMC8XsCX/ |
| clog << "process killed" << endl; | ||
| got_sigint = 1; | ||
| } else { | ||
| exit(signum); |
There was a problem hiding this comment.
exit(3) isn't a async-signal-safe function: https://www.man7.org/linux/man-pages/man7/signal-safety.7.html
better to use _exit() instead.
The same concern is applicable for clog too. We need to review "signal safety" across the codease. But that can be done separately later.
| int found = 0; | ||
|
|
||
| memset(&key, 0, sizeof(key)); | ||
| while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) { |
There was a problem hiding this comment.
To get the first key that's not in the map, bpf_map_get_next_key needs to be called with NULL as key. But a valid "key" is passed now (key is zero-initalized but it's not NULL), so this is likely to miss the correct first key.
struct client_op_k *cur = NULL;
while (bpf_map_get_next_key(map_fd, cur, &next_key) == 0) {
if (bpf_map_lookup_elem(map_fd, &next_key, &val) == 0) {
...
}
key = next_key;
cur = &key;
}
would fix it.
|
I also think "hung ops" better describes than "hang ops", so suggest name changes as such. |
aa20743 to
b05de17
Compare
taodd
left a comment
taodd
left a comment
There was a problem hiding this comment.
We also need to add a test to deliberately cause a hang Op
| } | ||
| if (name_len > 0) { | ||
| bpf_probe_read_user(val->object_name, name_len, (void *)name_base); | ||
| } |
b05de17 to
762503a
Compare
|
test result in microceph - https://paste.ubuntu.com/p/9NqMgrcz4K/ |
|
Thanks @zhhuabj
|
|
Also, the patch needs to be rebased |
762503a to
f67ffe2
Compare
Hi @dongdong, Thank you for the feedback! I've addressed all three suggestions: 1, pid from send_op: Moved val->pid = get_pid() to uprobe_send_op so incomplete ops also have a valid pid. and here is the latest test result - https://paste.ubuntu.com/p/3sSGqb3WGv/ |
f67ffe2 to
cf846e9
Compare
cf846e9 to
8f8b0ba
Compare
|
test results: |
radostrace previously discarded operations that were still present in the BPF ops map when tracing stopped. This made hung or slow operations invisible when the user interrupted tracing or a timeout expired. Track pending operations until completion and report entries remaining in the BPF map during shutdown. Completed and incomplete operations use the same output format, with the Complete column set to 1 or 0. Detach the probes and drain completed events before reporting pending operations to avoid classifying newly completed operations as incomplete. Add a functional test that suspends an OSD with SIGSTOP and verifies that pending operations are reported. Signed-off-by: Zhang Hua <joshua.zhang@canonical.com>
8f8b0ba to
9849fb4
Compare
The "Reporting pending ops" and "Total reported ops" lines were written to unbuffered stderr while the rows went through buffered stdout, so with output redirected to a file the status lines could land in the middle of a row. Drop them: the Complete column already marks pending rows. Flush stdout before the final cleanup log line for the same reason. Return a nonzero exit code when ring buffer setup fails; the path jumped to cleanup with ret still 0 and exited 0. Fix the indentation of the new helpers.
Add the Complete column to the sample output in README and doc/radostrace.md, describe it in the column table and CSV section, and add an "Incomplete Operations" section explaining the rows printed on Ctrl-C or timeout. Tests: _radostrace_rows now emits the Complete flag as its own field, both radostrace verifiers read it and reject anything other than 0 or 1, and the qemu test reads the extra field. The hung-op step in the microceph test uses _radostrace_rows instead of its own awk so the column layout lives in one place. Fix comments that still referred to the old NF >= 10 predicate.
pgrep -f "ceph-osd" matches any process whose command line contains the string, including a shell running a script that mentions it, so the hung-op step could SIGSTOP the wrong process. Use pgrep -x so only the ceph-osd and rados executables match.
3 participants
Handle SIGINT without exiting immediately so radostrace can dump operations still pending in the BPF ops map before cleanup.