Add support for catching signals #208
Conversation
wash-amzn
left a comment
wash-amzn
left a comment
There was a problem hiding this comment.
We need to do something about 'perf' if it's running (--profile). It should be sent a signal at the end of collection and wait()ed on to exit.
It's current behavior where we give it an amount of time to run should be a backstop in case aperf crashes. Normally it should be our signal to it that actually stops it.
| if poll(&mut poll_fds, PollTimeout::NONE)? <= 0 { | ||
| error!("Poll error."); | ||
| } | ||
| if let Some(ev) = poll_fds[0].revents() { |
There was a problem hiding this comment.
I think it's a bit nicer here to use timer_pollfd instea dof poll_fds[0]
And if possible, I'd change the argument to poll() to be the literal [timer_pollfd, signal_pollfd] (if it will let you, I'm not sure it will), to emphasize the unimportance of that array
There was a problem hiding this comment.
Yeah. It won't let me do that.
There was a problem hiding this comment.
I'm sure there's a way to make this less fragile
| } else { | ||
| error!("Caught unknown signal: {}. Exiting...", siginfo.ssi_signo); | ||
| } |
There was a problem hiding this comment.
We aren't registering an interest in any other signals, so this is impossible/dead code, I think it's fine to just omit it. Or replace it with a panic since it's not at all expected to happen.
There was a problem hiding this comment.
Yeah. We'll panic. Makes sense to become ugly if we're given a signal which we didn't register for.
We could default to always sending a SIGTERM to all the children. If we get a SIGINT, we can instead send that. |
This is used if the user ends an Aperf run using Ctrl+c or kill <pid>. We send the same signal to any child processes launched by aperf.
2 participants
This is used if the user ends an Aperf run using Ctrl+c or kill .
Ran aperf record and ctrl-c'd:
$ ./target/debug/aperf record -r ctrlcrecord
[2024-07-25T15:37:59Z INFO aperf_lib::record] Starting Data collection...
[2024-07-25T15:37:59Z INFO aperf_lib::record] Preparing data collectors...
[2024-07-25T15:37:59Z WARN aperf_lib::data::perf_stat] Instance does not expose Perf counters
[2024-07-25T15:37:59Z ERROR aperf_lib] Excluding perf_stat from collection. Error msg: No such file or directory (os error 2)
[2024-07-25T15:37:59Z INFO aperf_lib::record] Collecting data...
^C[2024-07-25T15:38:01Z INFO aperf_lib] Caught SIGINT. Exiting...
[2024-07-25T15:38:02Z INFO aperf_lib] Data collected in ctrlcrecord/, archived in ctrlcrecord.tar.gz
[2024-07-25T15:38:02Z INFO aperf_lib::record] Data collection complete.
Ran aperf record and send SIGTERM with kill:
$ ./target/debug/aperf record -r sigtermrecord -p 100
[2024-07-25T15:40:34Z INFO aperf_lib::record] Starting Data collection...
[2024-07-25T15:40:34Z INFO aperf_lib::record] Preparing data collectors...
[2024-07-25T15:40:34Z WARN aperf_lib::data::perf_stat] Instance does not expose Perf counters
[2024-07-25T15:40:34Z ERROR aperf_lib] Excluding perf_stat from collection. Error msg: No such file or directory (os error 2)
[2024-07-25T15:40:35Z INFO aperf_lib::record] Collecting data...
[2024-07-25T15:40:51Z INFO aperf_lib] Caught SIGTERM. Exiting...
[2024-07-25T15:40:51Z INFO aperf_lib] Data collected in sigtermrecord/, archived in sigtermrecord.tar.gz
[2024-07-25T15:40:51Z INFO aperf_lib::record] Data collection complete.
ctrlcrecord.tar.gz
sigtermrecord.tar.gz
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.