Skip to content

fix grpcomm errors bug 1215 #1254

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

Closed
wants to merge 4 commits into from
Closed

fix grpcomm errors bug 1215 #1254

wants to merge 4 commits into from

Conversation

annu13
Copy link
Contributor

@annu13 annu13 commented Dec 22, 2015

This commit is a potential fix for grpcomm issue:
#1215
The race condition identified can be avoided by setting up the job structure before starting the communications in ess_base_std_orted.c

@rhc54
Copy link
Contributor

rhc54 commented Dec 22, 2015

@annu13 I'm puzzled - I see the new code, but I don't see where the old code was removed. We were obviously setting all these infrastructure up prior to this commit, so I assume the old code is being removed, yes?

java: try do dlopen libmpi with the full path

Since OS X 10.11 (aka El Capitan) DYLD_LIBRARY_PATH is no more
propagated to children, so try to dlopen libmpi with the full path
using the directory of libmpi_java

Fixes open-mpi#1220

Thanks Alexander Daryin for reporting this

Update symbol-hiding script

btl/sm: rename file after file descriptor has been closed.

Thanks George for spotting this.
@annu13
Copy link
Contributor Author

annu13 commented Dec 22, 2015

oops! that was a merge mistake, thanks a bunch for quickly pointing this. In fact I didn't add any code, I just moved the existing job setup section before comm setup.

@artpol84
Copy link
Contributor

@annu13 it seems that you haven't rebased your OMPI branch. And tried to resolve something by hands. This doesn't seems right. Your PR must have only your changes while this one:
annu13@1a8c351
seems to be related to the already existing commit from @ggouaillardet:
e918d75

@ggouaillardet
Copy link
Contributor

@rhc54 you made a PR from your master branch, and that will get you in trouble ...
(Well, it did already)

The way we work is by making a branch first, and the make a pr with that branch.
First, run got log to retrieve your commits.
Then, reset to the most recent commit that is common between your master and ompi master
git reset --hard e918d75
Then branch
git checkout -b topic/my_branch
Then cherry pick your commits
git cherry-pick
And then push your branch to your github, and make the pr

As pointed by @artpol84 , there should be only one commit so you need to rebase,
Let me know if you need any help with that

@rhc54
Copy link
Contributor

rhc54 commented Dec 22, 2015

I think you meant @annu13 , not me.

@annu13
Copy link
Contributor Author

annu13 commented Dec 22, 2015

closing this as it contains incorrect commits.

@annu13 annu13 closed this Dec 22, 2015
@annu13
Copy link
Contributor Author

annu13 commented Dec 22, 2015

@ggouaillardet I got into this mess in multiple ways and now I am unable
to fix it :-(. I followed the process but looks I missed out some steps or
did something else inadvertently. To begin with
(1)I made changes on branch grpcomm_bug,
(2) merged grpcomm_bug branch with my local master,
(3)synced my fork with upstream (I used fetch upstream and merge upstream

  • I had to manually merge my changes)
    (4) then I pushed the changes
    (5) filed a PR again master.
  • After seeing Ralph�s comment, I realized that I didn�t merge correctly
    in step 3 so
    (6) fixed my merge
    (7) resync my fork
    (8) committed the changes - this is when I picked up your changes :-(.

I am trying to remove your changes from annu13/ompi@1a8c351, with no luck.
I tried the steps that you listed but I couldn�t cherry-pick my commit
from the big overall commit. I used revert instead.
Here is what I did -
(a) I reverted 1a8c351 on my master
(b) created a new branch named fixup
(c) made my change and committed it
(d) pushed fixup branch
(e) created a new pull request from this br

I hope it works now. I am sorry hope if this has caused any inconvenience
to anyone.

Thanks,
Annu.

jsquyres added a commit to jsquyres/ompi that referenced this pull request Aug 23, 2016
…count-ireduce

mpi/c: Add each check for count==0 in nonblocking reduce interface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants