Skip to content

SPML/UCX: fixed hang in SHMEM_FINALIZE - v4.0 #6922

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
Aug 26, 2019
Merged

SPML/UCX: fixed hang in SHMEM_FINALIZE - v4.0 #6922

merged 2 commits into from
Aug 26, 2019

Conversation

hoopoepg
Copy link
Contributor

  • used MPI _Barrier to synchronize processes

backport from #6918

Sergey Oblomov added 2 commits August 22, 2019 11:41
SPML/UCX: fixed hang in SHMEM_FINALIZE
66e1856 
- used MPI _Barrier to synchronize processes

Signed-off-by: Sergey Oblomov <[email protected]>
(cherry picked from commit 182023f)
SPML/UCX: fixed comment
1f9fce8 
Signed-off-by: Sergey Oblomov <[email protected]>
(cherry picked from commit 01dacaa)
jsquyres
jsquyres previously requested changes Aug 22, 2019
View reviewed changes
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

Please squash these commits and/or fix the commit message (because right now the 1st commit content directly contradicts the commit message).

@jsquyres
Copy link
Member

#@$%#@%$

I only just realized that this is a release branch PR, which means that this is a cherry pick of 2 poor commits from master. Please try to make reviews on master better so that we don't have to propagate bad commits like this to release branches.

@jsquyres jsquyres dismissed their stale review August 22, 2019 14:28

These are cherry picks, so the 2 original poor commits must remain

@hoopoepg
Copy link
Contributor Author

bot:retest

@jsquyres
Copy link
Member

Just to make sure our messages didn't cross in the ether: do not squash. This PR is cherry-picks from master, and it's more important to preserve the cherry picks.

@yosefe
Copy link
Contributor

yosefe commented Aug 26, 2019

@jsquyres sorry for poor review of the commit message.
so shall we merge this PR or make a new one with fixed commit message?

@gpaulsen gpaulsen added this to the v4.0.2 milestone Aug 26, 2019
@gpaulsen gpaulsen merged commit be67734 into open-mpi:v4.0.x Aug 26, 2019
@jsquyres
Copy link
Member

Merging this PR was appropriate because it accurately reflected the cherry-picks from master. We should try to catch these things before they hit master in the future.

Thanks!

@yosefe yosefe deleted the topic/fixed-hand-on-shmem-finalize-v4.0 branch August 27, 2019 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yosefe yosefe yosefe approved these changes

@jsquyres jsquyres jsquyres left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v4.0.2
Development

Successfully merging this pull request may close these issues.
4 participants
@hoopoepg @jsquyres @yosefe @gpaulsen