-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix dtrace system level scripts #1560
Conversation
The dtrace scripts did not handle multiple session IDs. Refactor the scripts get-ds-state and get-lr-state to correctly handle a PID with multiple sessions and improved the printing time for getting a system summary output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor nits but looks fine to me
tools/dtrace/get-ds-state.sh
Outdated
while read p; do | ||
# For each line in the file, pull out the PID we are looking at and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: probably doesnt matter too much here, but should be while read -r p
to avoid escape character expansion by the shell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
tools/dtrace/get-ds-state.d
Outdated
/* | ||
* Print a status line for a given PID. | ||
* Exit after 5 seconds. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question in the header comment: this prints for all PIDs rather than a given PID right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was copy cruft from where I copied the script from. I've fixed the comment to address reality.
tools/dtrace/get-lr-state.sh
Outdated
while read p; do | ||
# For each line in the file, pull out the PID we are looking at and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing about read -r
from above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks for the comments.
The test flake is #1498 |
lets merge it! |
While debugging system level issues on London, we found the dtrace scripts did not handle multiple session IDs correctly and were not showing correct results.
To fix, I did a refactor the scripts get-ds-state and get-lr-state to correctly handle a PID with multiple sessions and improved the printing time for getting a system summary output.
There is now a single dtrace sub-script that gathers all output from all process, then we sort and display a line for
each session in post-processing. This speeds up run time overall.
Sample output: