-
Notifications
You must be signed in to change notification settings - Fork 42
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
[nexus] the support bundle task should execute diag commands concurrently #7461
base: main
Are you sure you want to change the base?
[nexus] the support bundle task should execute diag commands concurrently #7461
Conversation
Created using spr 1.3.6-beta.1
while let Some(result) = diag_cmds.next().await { | ||
result?; | ||
} |
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.
Just to confirm, this will exit only if we fail to write the output of the command, not if the commands themselves fail, right? Just want to make sure that one failing command on a sled would not short-circuit everything else.
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.
Right, it's only going to fail if the tokio::write call fails on the support bundle file itself.
But thinking about this, should we instead log that we failed to write to file rather then potentially not getting through the entire array?
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.
Agree it's better to get partial results than nothing at all.
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.
Fixed in 6b02fb3
while let Some(result) = diag_cmds.next().await { | ||
result?; | ||
} |
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.
Agree it's better to get partial results than nothing at all.
Created using spr 1.3.6-beta.1
The test failure seems related to oxidecomputer/buildomat#62 so I am going to re-run it. |
This PR adds some of the
sled-diagnostics
crates commands that were not yet being collected. Additionally we now have an array of commands that will be ran concurrently (currently limited to 10 at a time) that we can add to as more support commands become available.