Skip to content

Fix singleton operations #2909

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 4 commits into from
Feb 5, 2017
Merged

Fix singleton operations #2909

merged 4 commits into from
Feb 5, 2017

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Feb 2, 2017

Cherry-pick several commits from master to fix singleton operations. Includes some partial custom repair of singleton comm_spawn - still some problems that need to be resolved.

Refs #2897

Signed-off-by: Ralph Castain [email protected]

ggouaillardet and others added 4 commits February 1, 2017 19:45
so it fixes a memory leak on finalize

Signed-off-by: Gilles Gouaillardet <[email protected]>
(cherry picked from commit fb5bcc4)
@rhc54 rhc54 added the bug label Feb 2, 2017
@rhc54 rhc54 added this to the v2.1.0 milestone Feb 2, 2017
Copy link
Member

@kawashima-fj kawashima-fj left a comment

Choose a reason for hiding this comment

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

Looks OK. I was just working on it and the diff of ess_singleton_module.c is same as mine.

@rhc54
Copy link
Contributor Author

rhc54 commented Feb 2, 2017

yeah, the rest of the problem with singleton comm_spawn is in the orte layer - i'll address it tomorrow

@rhc54
Copy link
Contributor Author

rhc54 commented Feb 2, 2017

@hppritcha @jsquyres I would suggest going ahead with this PR and #2897 for now. With the changes in this PR, singletons will work just fine with dstore enabled or disabled. Singleton comm_spawn hangs, but that is a secondary problem and can be dealt with later. I don't have time right now, so I'd suggest someone file an issue for v2.1 and we'll deal with it when time is available.

@jsquyres
Copy link
Member

jsquyres commented Feb 2, 2017

@hppritcha Signed-off-by checker failed because @rhc54 cherry-picked some master commits from before we required signed-off-by. They're commits from @ggouaillardet, who a) is a proven community member, and b) has a signed contribution form on file for commits from before we required signed-off-by.

My $0.02: good to go (hopefully Travis will finish soon).

@hppritcha
Copy link
Member

Note I have a pr in a similar signature sign off issue. Do we give founding members who commit directly to master without sign offs a pass? Note that isn't the case with this PR.

@rhc54
Copy link
Contributor Author

rhc54 commented Feb 3, 2017

This isn't a question of getting a "pass". All we are doing is porting a commit already in our code base over to a release branch. There is no code being introduced into the repository, and so technically no sign-off is required.

New code requires a sign-off, regardless of who is committing it.

@jsquyres
Copy link
Member

jsquyres commented Feb 3, 2017

👍 on @rhc54's answer.

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