Skip to content

Fixes the pmix1x external component #3677

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

Merged
merged 1 commit into from
Jun 20, 2017
Merged

Conversation

karasevb
Copy link
Member

@karasevb karasevb commented Jun 8, 2017

Fixes the pmix1x external component

  • fixed the jobid tracker, was not added item before using it
  • the server side of PMIx v1 component didn't register own
    nspace, that didn't allow the use of the PMIx functions

Fixed #3200

Signed-off-by: Boris Karasev [email protected]

@karasevb karasevb added the bug label Jun 8, 2017
@karasevb karasevb added this to the v3.0.0 milestone Jun 8, 2017
@karasevb karasevb requested review from rhc54 and artpol84 June 8, 2017 11:02
- fixed the `jobid` tracker, was not added item before using it
- the server side of PMIx v1 component didn't register own
  nspace, that didn't allow the use of the PMIx functions

Signed-off-by: Boris Karasev <[email protected]>
@rhc54
Copy link
Contributor

rhc54 commented Jun 8, 2017

@karasevb This looks fine, but it needs to go into master first. Thanks for debugging it!

@artpol84
Copy link
Contributor

artpol84 commented Jun 8, 2017

@rhc54 as far as I recall, master is not affected. @karasevb what is the difference between two?

@karasevb
Copy link
Member Author

karasevb commented Jun 9, 2017

@rhc54 @artpol84
I investigated the master, and the next thing attracts attention: the master had the same problem in 9850832, and the next commit (92c9964) contains the fixes of this problem.
Probably, some changes of 92c9964 didn't get into v3.x, but these changes contains into v3.0.x.
We can fix that in v3.x by cherry-picking from 92c9964, but I guess not all changes of it should be porting to v3.x. Hence in that case this PR will be closed.

@rhc54
Copy link
Contributor

rhc54 commented Jun 9, 2017

My point was that these changes are not present in master, and need to be there prior to bringing them to a release branch. I've submitted #3687 to do so. Once that is committed and "soaks" over the weekend, I'll approve this PR.

@artpol84
Copy link
Contributor

artpol84 commented Jun 9, 2017

@karasevb 's point is that differences in master and v3.x are sufficient. If I understood him correctly it seems that unless static ports are used this piece of code will not be executed. However in v3.x this is not a case.
In master:

In v3.0.x:

In v3.x

Why Master and v3.0.x rely on static ports being used and v3.x doesn't? This is the thing that we need to converge before going further.

@artpol84
Copy link
Contributor

artpol84 commented Jun 9, 2017

"Once that is committed and "soaks" over the weekend, I'll approve this PR."
I believe that this code will lay intact during the weekend unless somebody tests with static ports.

@bwbarrett bwbarrett merged commit f7fb55d into open-mpi:v3.x Jun 20, 2017
@karasevb karasevb deleted the v3.x_fix branch July 14, 2017 03:41
npe9 pushed a commit to npe9/ompi that referenced this pull request Feb 3, 2019
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.

5 participants