Update PMIx and PRRTE #7566

rhc54 · 2020-03-25T22:46:39Z

Deprecate --am and --amca options

Avoid default param files on backend nodes
Any parameters in the PRRTE default or user param files will have been
picked up by prte and included in the environment sent to the prted, so
don't open those files on the backend.

Avoid picking up MCA param file info on backend
Avoid the scaling problem at PRRTE startup by only reading the system
and user param files on the frontend.

Complete revisions to cmd line parser for OMPI
Per specification, enforce following precedence order:

system-level default parameter file
user-level default parameter file
Anything found in the environment
"--tune" files. Note that "--amca" goes away and becomes equivalent to "--tune". Okay if it is provided more than once on a cmd line (we will aggregate the list of files, retaining order), but an error if a parameter is referenced in more than one file with a different value
"--mca" options. Again, error if the same option appears more than once with a different value. Allowed to override a parameter referenced in a "tune" file
"-x" options. Allowed to overwrite options given in a "tune" file, but cannot conflict with an explicit "--mca" option
all other options

Fix special handling of "-np"
Fixes #7565

Get agreement on jobid across the layers
Need all three pieces (PRRTE, PMIx, and OPAL) to agree on the nspace
conversion to jobid method

Ensure prte show_help messages get output
Print abnormal termination messages
Fixes #7564

Add extra libs to PRRTE binaries for external deps
libevent, hwloc, and pmix can be external and may require that their
libs be explicitly linked into the PRRTE binaries

Fix scalable launch with rsh - resolve several issues in tree-spawn

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

rhc54 · 2020-03-25T22:50:21Z

@artemry-mlnx I fully expect this to fail Mellanox CI due to the changes in cmd line parsing rules when it comes to "--tune", "--amca", "--mca", and "-x" options. I'll need your help to work thru the updates to your CI

artemry-nv · 2020-03-25T23:06:05Z

@rhc54 sure, I'll deal with the CI script update.

gpaulsen · 2020-03-26T14:28:57Z

Well, host c656f6n03 is up, but address 172.18.0.3 must be a docker overlay network.
@jjhursey can you please take a look?

rhc54 · 2020-03-26T14:29:46Z

No, it isn't anything to do with IBM CI - the MCA params need to be properly prefixed in the OMPI case and they aren't. I'm fixing it

jjhursey · 2020-03-26T15:08:08Z

Well, host c656f6n03 is up, but address 172.18.0.3 must be a docker overlay network.

Just to follow up on this question: Yeah the 172.18.0.x network is the overlay inside the virtual cluster. It is private and routable between only the "virtual nodes" in that cluster. It shouldn't be getting in the way. It sounds like Ralph is working the real issue.

jsquyres · 2020-03-26T17:11:21Z

Inside an salloc job, I see that mpirun of a non-MPI job works fine, but mpirun of an MPI job fails:

$ mpirun hostname | sort | uniq
mpi028
mpi029

$ mpirun ring_c
------------------------------------------------------------
A process or daemon was unable to complete a TCP connection
to another process:
  Local host:    savbu-usnic-a
  Remote host:   mpi028
This is usually caused by a firewall on the remote host. Please
check that any firewall (e.g., iptables) has been disabled and
try again.
------------------------------------------------------------
------------------------------------------------------------
A process or daemon was unable to complete a TCP connection
to another process:
  Local host:    savbu-usnic-a
  Remote host:   mpi029
This is usually caused by a firewall on the remote host. Please
check that any firewall (e.g., iptables) has been disabled and
try again.
------------------------------------------------------------

rhc54 · 2020-03-26T17:13:45Z

Not sure I can understand that one - did the daemon on the remote host segfault? Otherwise, it's hard to understand why the PRRTE infrastructure would care about MPI vs non-MPI apps

jsquyres · 2020-03-26T17:26:49Z

I am now unable to reproduce the error. Weird, because it was 100% reproducible before. 🤷‍♂

Sooo... let's ignore my report, I guess!

jsquyres · 2020-03-26T17:28:13Z

What's the failure in the Mellanox CI? Is that some in-flight CLI change that the Mellanox CI needs to be updated for?

rhc54 · 2020-03-26T17:30:38Z

Yes and no. Yes, the specific test should return an error because they included the same envar in two different tune files, and assigned it a different value. This is no longer permitted.

No in that there are still some bugs in the parser as it didn't error out as it should. I'm working on that one, and then we'll have to address the test itself.

rhc54 · 2020-03-26T18:56:43Z

@artemry-mlnx I believe I have this working now and reporting cmd line errors as it should. There are two tests that are going to fail:

echo "--mca mca_base_env_list \"XXX_A=1;XXX_B=2;XXX_C;XXX_D;XXX_E\"" > "$WORKSPACE/test_tune.conf"
# shellcheck disable=SC2086
val=$("${OMPI_HOME}/bin/mpirun" $mca -np 2 --tune "$WORKSPACE/test_tune.conf" --mca mca_base_env_list \
     "XXX_A=7;XXX_B=8" "${abs_path}/env_mpi" | sed -n -e 's/^XXX_.*=//p' | sed -e ':a;N;$!ba;s/\n/+/g' | bc)
# precedence goes left-to-right.
# A is set to 1 in "tune", and then reset to 7 in the --mca parameter
# B is set to 2 in "tune", and then reset to 8 in the --mca parameter
# C, D, E are taken from the environment as 3,4,5
# return (7+8+3+4+5)*2=54

val=$("${OMPI_HOME}/bin/mpirun" $mca --np 2 --tune "$WORKSPACE/test_tune.conf" \
    --am "$WORKSPACE/test_amca.conf" "${abs_path}/env_mpi" | sed -n -e 's/^XXX_.*=//p' | sed -e ':a;N;$!ba;s/\n/+/g' | bc)
# precedence goes left-to-right.
# A is first set to 1 in "tune", and then reset to 7 in "amca".
# B is first set to 2 in "tune", but then reset to 8 in "amca"
# C, D, E are taken from the environment as 3,4,5
# return (7+8+3+4+5)*2=54

These will fail because they "reset" values, which is no longer allowed. You can override values from a file by putting a new value on the cmd line itself, but values from within two files (or even within a single file) are not allowed to override each other. I'm not sure how you would like to fix those - we could just turn them off, I suppose, or you could fix the override. Up to you.

Deprecate --am and --amca options Avoid default param files on backend nodes Any parameters in the PRRTE default or user param files will have been picked up by prte and included in the environment sent to the prted, so don't open those files on the backend. Avoid picking up MCA param file info on backend Avoid the scaling problem at PRRTE startup by only reading the system and user param files on the frontend. Complete revisions to cmd line parser for OMPI Per specification, enforce following precedence order: 1. system-level default parameter file 1. user-level default parameter file 1. Anything found in the environment 1. "--tune" files. Note that "--amca" goes away and becomes equivalent to "--tune". Okay if it is provided more than once on a cmd line (we will aggregate the list of files, retaining order), but an error if a parameter is referenced in more than one file with a different value 1. "--mca" options. Again, error if the same option appears more than once with a different value. Allowed to override a parameter referenced in a "tune" file 1. "-x" options. Allowed to overwrite options given in a "tune" file, but cannot conflict with an explicit "--mca" option 1. all other options Fix special handling of "-np" Get agreement on jobid across the layers Need all three pieces (PRRTE, PMIx, and OPAL) to agree on the nspace conversion to jobid method Ensure prte show_help messages get output Print abnormal termination messages Cleanup error reporting in persistent operations Signed-off-by: Ralph Castain <[email protected]> dd Signed-off-by: Ralph Castain <[email protected]>

jsquyres

These are working for me.

rhc54 · 2020-03-29T14:43:23Z

@artemry-mlnx Any estimate on when you will update the tests? We are hitting a bit of a problem in that PRRTE is now ahead of OMPI and cannot launch OMPI jobs without this PR being committed.

Properly mark/detect that a daemon sourced the event broadcast to avoid reinjecting it into the PMIx server library. Correct the source field for the event notify call on launcher ready. Update event notification for tool support Deal with a variety of race conditions related to tool reconnection to a different server. Signed-off-by: Ralph Castain <[email protected]>

artemry-nv · 2020-03-30T17:20:13Z

@rhc54 working on the script update.
ETA: today/tomorrow.

libevent, hwloc, and pmix can be external and may require that their libs be explicitly linked into the PRRTE binaries Signed-off-by: Ralph Castain <[email protected]>

artemry-nv · 2020-03-31T01:08:02Z

@rhc54
Could you please elaborate a bit on the new logic based on the scenarios above - who is not allowed to overwrite whom?
I've temporary disabled 2 tests you mentioned above but there are still some failed test scenarios: log file - could you please take a look?

rhc54 · 2020-03-31T01:35:43Z

@artemry-mlnx Perhaps the easiest approach is for me to give you a pull request with the required changes? That will give me a chance to manually check each test and determine the required changes

rhc54 · 2020-03-31T02:06:49Z

@artemry-mlnx I believe mellanox-hpc/jenkins_scripts#96 will fix the CI tests. Can you please review and we can recheck here?

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

rhc54 · 2020-03-31T15:11:40Z

@jladd-mlnx

artemry-nv · 2020-03-31T15:27:21Z

@rhc54
Have you verified the changes?

rhc54 · 2020-03-31T15:29:09Z

@artemry-mlnx I ran the updated tests by hand and they all worked, so I believe it should pass. If not, I can deal with any failures once I see them.

artemry-nv · 2020-03-31T15:41:13Z

@rhc54
Can you please update Azure Pipeline file in your feature branch to the new CI scripts (your topic branch in jenkins_scripts repo) to verify before merging them:

ompi/.ci/mellanox/azure-pipelines.yml

Line 13 in 5dcd1f4

    
           ompi_jenkins_scripts_git_repo_url: https://github.com/mellanox-hpc/jenkins_scripts.git

ompi/.ci/mellanox/azure-pipelines.yml

Line 14 in 5dcd1f4

ompi_jenkins_scripts_git_branch: master

rhc54 · 2020-03-31T15:46:28Z

@artemry-mlnx Done - let's see how it does!

artemry-nv · 2020-03-31T16:21:20Z

@rhc54 it's passed, please revert back Azure Pipeline and I'll proceed with the merge for jenkins_scripts.

rhc54 · 2020-03-31T16:22:33Z

@artemry-mlnx Done!

artemry-nv · 2020-03-31T16:30:49Z

@rhc54
Ideally we need to merge these both PRs in the same time - are you going to merge this Open MPI PR after CI is passed?

rhc54 · 2020-03-31T16:31:40Z

@artemry-mlnx Yes - though it isn't clear that this PR is going to pass without my local CI change unless you commit the PR, true?

artemry-nv · 2020-03-31T16:35:11Z

That's true - merged first.

artemry-nv · 2020-03-31T16:35:19Z

/azp run

azure-pipelines · 2020-03-31T16:35:27Z

Azure Pipelines successfully started running 1 pipeline(s).

rhc54 · 2020-03-31T16:35:33Z

@artemry-mlnx Thanks!

rhc54 requested a review from jsquyres March 25, 2020 22:46

rhc54 self-assigned this Mar 25, 2020

open-mpi deleted a comment from ibm-ompi Mar 26, 2020

jsquyres approved these changes Mar 28, 2020

View reviewed changes

Add extra libs to PRRTE binaries for external deps

1aabbe4

libevent, hwloc, and pmix can be external and may require that their libs be explicitly linked into the PRRTE binaries Signed-off-by: Ralph Castain <[email protected]>

PRRTE: Return non-zero status on timeout

556b3fc

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

rhc54 merged commit 538d2de into open-mpi:master Mar 31, 2020

rhc54 deleted the topic/up2 branch March 31, 2020 17:29

Update PMIx and PRRTE #7566

Update PMIx and PRRTE #7566

Uh oh!

Conversation

rhc54 commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhc54 commented Mar 25, 2020

Uh oh!

artemry-nv commented Mar 25, 2020

Uh oh!

gpaulsen commented Mar 26, 2020

Uh oh!

rhc54 commented Mar 26, 2020

Uh oh!

jjhursey commented Mar 26, 2020

Uh oh!

jsquyres commented Mar 26, 2020

Uh oh!

rhc54 commented Mar 26, 2020

Uh oh!

jsquyres commented Mar 26, 2020

Uh oh!

jsquyres commented Mar 26, 2020

Uh oh!

rhc54 commented Mar 26, 2020

Uh oh!

rhc54 commented Mar 26, 2020

Uh oh!

jsquyres left a comment

Choose a reason for hiding this comment

Uh oh!

rhc54 commented Mar 29, 2020

Uh oh!

artemry-nv commented Mar 30, 2020

Uh oh!

artemry-nv commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

artemry-nv commented Mar 31, 2020

Uh oh!

azure-pipelines bot commented Mar 31, 2020

Uh oh!

rhc54 commented Mar 31, 2020

Uh oh!

Uh oh!

rhc54 commented Mar 25, 2020 •

edited

Loading

artemry-nv commented Mar 31, 2020 •

edited

Loading