Skip to content

v4.0.x: Cleanup race condition in finalize that leads to incomplete vader cleanup #6550

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 9, 2019
Merged

v4.0.x: Cleanup race condition in finalize that leads to incomplete vader cleanup #6550

merged 2 commits into from
Apr 9, 2019

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Mar 31, 2019

@aingerson Please verify this fixes the problem

Fixes #6547

Ralph Castain added 2 commits March 31, 2019 11:23
@rhc54
Cleanup race condition in finalize
861016c 
See #5798 (comment)
for a lengthy explanation

Signed-off-by: Ralph Castain <rhc@open-mpi.org>
(cherry picked from commit 57f6b94)
@rhc54
Remove stale ORTE code
2536b4f 
Functionality moved to PMIx

Signed-off-by: Ralph Castain <rhc@open-mpi.org>
(cherry picked from commit cfdd08d)
@rhc54 rhc54 added the bug label Mar 31, 2019
@rhc54 rhc54 added this to the v4.0.2 milestone Mar 31, 2019
@rhc54 rhc54 self-assigned this Mar 31, 2019
@rhc54 rhc54 requested a review from ggouaillardet March 31, 2019 22:16
@hppritcha hppritcha added the NEWS label Apr 1, 2019
@aingerson
Copy link

I think that did the trick. Thanks, Ralph!

@hppritcha
Copy link
Member

@ggouaillardet could you please review?

@hppritcha
Copy link
Member

Intel engineers have verified that this patch fixes the issue.

ggouaillardet
ggouaillardet approved these changes Apr 5, 2019
View reviewed changes
Copy link
Contributor

@ggouaillardet ggouaillardet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with the first commit.
The second one removes quite some code, and I leave it up to the RM to approve this change in the middle of a serie (fwiw I do not have any objection)

@rhc54
Copy link
Contributor Author

rhc54 commented Apr 5, 2019

Just an FYI: the second commit removes stale code that cannot be reached any more as the functionality moved to PMIx. I only included it because the desired commit was based on it - i.e., I would have had to make some significant mods to the desired commit in order to get it to build/work.

@hppritcha
Copy link
Member

@gpaulsen let's merge this.

@gpaulsen gpaulsen merged commit 811dfc6 into open-mpi:v4.0.x Apr 9, 2019
This was referenced Apr 9, 2019
Closed
Closed
@hppritcha hppritcha mentioned this pull request Apr 10, 2019
Closed
@leofang leofang mentioned this pull request May 5, 2019
Closed
@rhc54 rhc54 deleted the cmr402/clnup branch November 27, 2019 19:17
@jsquyres jsquyres mentioned this pull request Nov 30, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggouaillardet ggouaillardet ggouaillardet approved these changes

Assignees

@rhc54 rhc54

Labels
bug NEWS
Projects
None yet
Milestone
v4.0.2
Development

Successfully merging this pull request may close these issues.
5 participants
@rhc54 @aingerson @hppritcha @ggouaillardet @gpaulsen