Correct mapping errors #4760
Correct mapping errors #4760
Conversation
rhc54
requested review from
ggouaillardet,
gvallee,
jjhursey,
jsquyres and
naughtont3
|
You may or may not want this backport from the PMIx reference server. While investigating a problem reported by @ggouaillardet regarding oversubscription, I found that the slot accounting system was generating erroneous results whenever a job failed, and that multiple job executions were resulting in allocation confusion as the hostfile was adding nodes on every invocation. These changes should have no impact on one-shot executions (i.e., mpirun), but they definitely resolve some orte-dvm problems. However, I can't absolutely swear that they won't impact somebody doing something unusual. You all know how many times we have discovered that someone's corner case behavior changed when we touch the allocation code! So look this over carefully - as I said, totally up to you. |
|
i will review this. out of curiosity, what is the rationale for having |
|
It is mostly politics - asking PMIx users and developers to clone the OMPI repository is unacceptable to some non-trivial portion of the community. It also allows for divergence, as this PR may well represent. |
|
@jjhursey The DVM will likely have problems without this change as the resource accounting is off. |
|
@jjhursey @ggouaillardet Ping. Can you guys review this? Thanks! |
|
(reviewing/testing now) Sorry for the delay |
|
I reviewed the code and it looked good. However, I am trying to do some runtime testing to verify the fix and hit a snag that I think I've seen before on I have most of a fix, and will post when it's done. It looks like a NULL pointer in the PMIx_tool_init logic. |
|
Yeah, this is one of the fixes that failed to come across - should be fixed in a separate PR that got committed, I think. If not, it certainly is fixed upstream in PMIx. |
|
The NULL pointer was in the diff --git a/opal/mca/pmix/pmix3x/pmix/src/tool/pmix_tool.c b/opal/mca/pmix/pmix3x/pmix/src/tool/pmix_tool.c
index 31020c56..675d724b 100644
--- a/opal/mca/pmix/pmix3x/pmix/src/tool/pmix_tool.c
+++ b/opal/mca/pmix/pmix3x/pmix/src/tool/pmix_tool.c
@@ -386,6 +386,10 @@ PMIX_EXPORT int PMIx_tool_init(pmix_proc_t *proc,
if (NULL == pmix_globals.mypeer->nptr->nspace) {
pmix_globals.mypeer->nptr->nspace = strdup(proc->nspace);
}
+ if( NULL == pmix_globals.mypeer->info ) {
+ pmix_globals.mypeer->info = PMIX_NEW(pmix_rank_info_t);
+ pmix_globals.mypeer->info->pname.nspace = (char*)malloc(sizeof(char)*(PMIX_MAX_NSLEN+1));
+ }
(void)strncpy(pmix_globals.mypeer->info->pname.nspace, proc->nspace, PMIX_MAX_NSLEN);
pmix_globals.mypeer->info->pname.rank = proc->rank; |
|
Sigh - I fear things are getting out of sync as the PR's sit too long. I can try to resync this one. |
Since we now support the dynamic addition of hosts to the orte_node_pool, there is no longer any reason to require advanced specification of all possible nodes. Instead, use a precedence method to initially allocate only those hosts that were specified in the cmd line: * rankfile, if given, as that will specify the nodes * -host, aggregated across all app_contexts * -hostfile, aggregated across all app_contexts * default hostfile * assign local node Fix slots_inuse accounting so that the nodes are correctly reset upon error termination - e.g., when oversubscribed without permission. Ensure we accurately track the user's specified desires for oversubscribe and no-use-local when dynamically spawning jobs. Signed-off-by: Ralph Castain <[email protected]> (cherry picked from commit c9b3e68)
Signed-off-by: Ralph Castain <[email protected]>
|
okay, I have brought this up-to-date. Problem wasn't in the mapping code, but simply the IOF output was being lost. |
|
I did a run with the updated branch, and it looks like it is working just fine now. I do see this warning from the PMIx v3 component (might just be my build though): @rhc54 I think this is good to go. I don't know if you also want to wait for @ggouaillardet to sign off as well. This is what I tested: |
|
Yeah, that loading error is irrelevant - just forgot to remove a component that only applies to the PMIx IOF branch. I can do that separately. Thx for checking this out! |
5 participants
Since we now support the dynamic addition of hosts to the orte_node_pool, there is no longer any reason to require advanced specification of all possible nodes. Instead, use a precedence method to initially allocate only those hosts that were specified in the cmd line:
rankfile, if given, as that will specify the nodes
-host, aggregated across all app_contexts
-hostfile, aggregated across all app_contexts
default hostfile
assign local node
Fix slots_inuse accounting so that the nodes are correctly reset upon error termination - e.g., when oversubscribed without permission.
Ensure we accurately track the user's specified desires for oversubscribe and no-use-local when dynamically spawning jobs.
Signed-off-by: Ralph Castain [email protected]
(cherry picked from commit c9b3e68)