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.

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

Make stream buffing an MCA option again #10746

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

jjhursey
Copy link
Member

@jjhursey jjhursey commented Sep 1, 2022

  • Way back when the stream buffering option was introduced
    it was an MCA: 4dd9f89a9
    • It changed to a CLI option at some point.
  • Make this an MCA option once again, and connect it with the
    PRRTE CLI options.

 * Way back when the stream buffering option was introduced
   it was an MCA: open-mpi@4dd9f89a9
   - It changed to a CLI option at some point.
 * Make this an MCA option once again, and connect it with the
   PRRTE CLI options.

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey
Copy link
Member Author

jjhursey commented Sep 1, 2022

Pairs with: openpmix/prrte#1489

@rhc54
Copy link
Contributor

rhc54 commented Sep 1, 2022

Isn't that going to break mpirun operations where PMIx is doing the IOF and has no idea that you have done this? I suspect PRRTE/PMIx will feel free to overwrite, which is going to confuse - yes?

@jjhursey jjhursey mentioned this pull request Sep 1, 2022
14 tasks
@rhc54
Copy link
Contributor

rhc54 commented Sep 1, 2022

I think you may run into issues here - but it's totally up to you over here. I'm not yet convinced of the piece over in PRRTE, but that's another matter.

@jjhursey
Copy link
Member Author

jjhursey commented Sep 1, 2022

@rhc54 I'm not sure I understand your concern. The application (whether in OMPI or their code) can call setvbuf to adjust the kernel buffering of an IO stream at any point. How would this break mpirun? This would only impact how often the kernel is flushing the stream (stdout/stderr in this case) from the MPI application process to whoever is watching that stream.

I did test this with a few different scenarios, and it works fine.

@rhc54
Copy link
Contributor

rhc54 commented Sep 1, 2022

I'm half blind on painkillers, so my brain is mush - please let things percolate down slowly thru the mud. I agree about the app always being able to do this. Just trying to get my head around the overall IO path and what we want to happen, and what they expect to see.

@gpaulsen
Copy link
Member

gpaulsen commented Sep 2, 2022

@rhc54 hope you feel better soon.

Is there a test we can add to ensure proper streaming behavior for both mpirun and prterun? I've been trying to read manpages, but didn't fine a way to query that from a file descriptor.

@rhc54
Copy link
Contributor

rhc54 commented Sep 2, 2022

I think Josh and I are discussing whether mpirun needs to have a particular behavior or not over on the PRRTE side.

@jjhursey
Copy link
Member Author

jjhursey commented Sep 2, 2022

FYI: We resolved the issue on the PRRTE side (openpmix/prrte#1489) so this PR is ready to go.

@jjhursey jjhursey merged commit 4b327e5 into open-mpi:main Sep 2, 2022
@jjhursey jjhursey deleted the rm-stream-buffering branch September 2, 2022 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants