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

comm: couple of enhancements wrt pmix groups #12996

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hppritcha
Copy link
Member

add a pmix timeout option for group operations.
This may be a no-op with certain pmix server variants but set it anyway.

For protection make sure when creating intercomm communicators using pmix group construct that all procs supply the same ordered list of pmix procs to pmix group construct.

update the description for ompi_mca_mpi_pmix_connect_timeout to not it can be used to control timeout for pmix group calls as well.

add a pmix timeout option for group operations.
This may be a no-op with certain pmix server variants but set it anyway.

For protection make sure when creating intercomm communicators using
pmix group construct that all procs supply the same ordered list of pmix procs
to pmix group construct.

update the description for ompi_mca_mpi_pmix_connect_timeout to not
it can be used to control timeout for pmix group calls as well.

Signed-off-by: Howard Pritchard <[email protected]>
@@ -383,24 +395,51 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
pinfo = (pmix_info_t*)darray.array;
ninfo = darray.size;

proc_count = newcomm->c_local_group->grp_proc_count;
/*
* Make sure all processes participating in the PMIx group construct operation
Copy link
Contributor

Choose a reason for hiding this comment

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

PLEASE don't do this! The last PMIx version that needed this was years ago in the v3 series, which OMPI does not support. The Standard has been updated to stipulate that order is not guaranteed, so we won't see it return.

All this does is add more overhead for no value.

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.

2 participants