Skip to content
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

OMPI/OSC/UCX: set priority to 0. #5134

Merged
merged 1 commit into from
May 2, 2018
Merged

Conversation

xinzhao3
Copy link
Contributor

@xinzhao3 xinzhao3 commented May 2, 2018

No description provided.

@hjelmn
Copy link
Member

hjelmn commented May 2, 2018

You can put this straight to v3.1.x. I wouldn't necessarily bother with master. Except to add the parameter. That should have been in there originally. No osc component should have a hard-coded priority.

@hjelmn
Copy link
Member

hjelmn commented May 2, 2018

BTW, you are missing the mca_base_component_var_register to register the priority with the MCA.

@bwbarrett
Copy link
Member

I disagree with @hjelmn on this one. I'd rather see it in master and then move to 3.1. That way, we're positive we've fixed the core issues before we change priority in master. Less chance of screwing up 4.0 the same way.

@xinzhao3 xinzhao3 force-pushed the topic/osc-topic branch from ed218e9 to 108e36a Compare May 2, 2018 18:38
Signed-off-by: Xin Zhao <xinz@mellanox.com>
@xinzhao3 xinzhao3 force-pushed the topic/osc-topic branch from 108e36a to 3f5ac97 Compare May 2, 2018 18:40
@xinzhao3
Copy link
Contributor Author

xinzhao3 commented May 2, 2018

@hjelmn @bwbarrett could you review again? I can create another PR on 3.1.x once this one looks good.

@hjelmn
Copy link
Member

hjelmn commented May 2, 2018

@xinzhao3 Thank you, looks good now. Once this passes CI please merge and open the v3.1.x PR.

@xinzhao3 xinzhao3 merged commit 147299d into open-mpi:master May 2, 2018
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.

3 participants