Skip to content

pmix/pmix112: Enable PMIx shared memory dstore by default #2932

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
Feb 21, 2017

Conversation

kawashima-fj
Copy link
Member

The problem of PMIx shared memory dstore + MPI singleton is fixed now. See #2859 and #2909. We need PMIx shared memory dstore in v2.1.x.

The master branch had this change long ago and the commit includes many changes so this commit is a direct commit (no cherry-pick).

@jjhursey Please review.

Signed-off-by: KAWASHIMA Takahiro [email protected]

@jjhursey
Copy link
Member

jjhursey commented Feb 7, 2017

Thanks for this PR. This is good to go.

However, we should have a discussion in the OMPI Teleconf about if we should have this enabled by default in the 2.1.x series. I cannot remember if we decided on this yet or not. @gpaulsen Can you add this to the agenda for tomorrow.

@kawashima-fj
Copy link
Member Author

@jjhursey @gpaulsen OK. I added the WIP-DNM label. Please remove it if you confirmed it.

@hppritcha
Copy link
Member

LANL CI down for today due to NERSC quarterly maintenance.

@jsquyres
Copy link
Member

jsquyres commented Feb 7, 2017

Per 2017-02-07 webex discussion, there appears to be a bug in dstore on master. When that gets fixed and is brought over to v2.x, it seems like we want dstore to be the default on v2.x.

@hppritcha
Copy link
Member

@hjelmn has reason to believe there is a memory corruption problem if shared memory is enabled by default.

@jjhursey
Copy link
Member

jjhursey commented Feb 7, 2017

This is the PMIx issue that @hjelmn @artpol84 are working on.

Once that is fixed in PMIx, then I'll update the v2.x branch with updated PMIx v1.2.1rc. We'll need those fixes in before merging this PR - so we don't break default builds.

@kawashima-fj
Copy link
Member Author

bot:mellanox:retest

@hppritcha
Copy link
Member

merge this after #2958

@jsquyres
Copy link
Member

#2958 has been merged. Now merging this one to enable PMIx shared memory datastore by default.

@jsquyres jsquyres merged commit b03288c into open-mpi:v2.x Feb 21, 2017
@kawashima-fj kawashima-fj deleted the pr/v2.x/enable-pmix-dstore branch February 22, 2017 02:45
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