Modify the internal logic for resolve nodes/peers

The current code path for PMIx_Resolve_peers and PMIx_Resolve_nodes executes a threadshift in the preg components themselves. This is done to ensure thread safety when called from the user level. However, it causes thread-stall when someone attempts to call the regex functions from _inside_ the PMIx code base should the call occur from within an event.

Accordingly, move the threadshift to the client-level functions and make the preg components just execute their algorithms. Create a new pnet/test component to verify that the prge code can be safely accessed - set that component to be selected only when the user directly specifies it. The new component will be used to validate various logical extensions during development, and can then be discarded.

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

Author: Ralph Castain

opal/mca/pmix/pmix3x/pmix/src/client/pmix_client.c

@@ -1175,37 +1175,135 @@ static void _commitfn(int sd, short args, void *cbdata)
     return rc;
 }
 
+static void _resolve_peers(int sd, short args, void *cbdata)
+{
+    pmix_cb_t *cb = (pmix_cb_t*)cbdata;
+    pmix_status_t rc;
+
+    cb->status = pmix_preg.resolve_peers(cb->key, cb->pname.nspace,
+                                         &cb->procs, &cb->nprocs);
+    /* post the data so the receiving thread can acquire it */
+    PMIX_POST_OBJECT(cb);
+    PMIX_WAKEUP_THREAD(&cb->lock);
+}
+
 /* need to thread-shift this request */
 PMIX_EXPORT pmix_status_t PMIx_Resolve_peers(const char *nodename,
                                              const char *nspace,
                                              pmix_proc_t **procs, size_t *nprocs)
 {
+    pmix_cb_t *cb;
+    pmix_status_t rc;
+    pmix_proc_t proc;
+
     PMIX_ACQUIRE_THREAD(&pmix_global_lock);
     if (pmix_globals.init_cntr <= 0) {
         PMIX_RELEASE_THREAD(&pmix_global_lock);
         return PMIX_ERR_INIT;
     }
     PMIX_RELEASE_THREAD(&pmix_global_lock);
 
-    /* set default */
-    *procs = NULL;
-    *nprocs = 0;
 
-    return pmix_preg.resolve_peers(nodename, nspace, procs, nprocs);
+    cb = PMIX_NEW(pmix_cb_t);
+    cb->key = (char*)nodename;
+    cb->pname.nspace = strdup(nspace);
+
+    PMIX_THREADSHIFT(cb, _resolve_peers);
+
+    /* wait for the result */
+    PMIX_WAIT_THREAD(&cb->lock);
+
+    /* if the nspace wasn't found, then we need to
+     * ask the server for that info */
+    if (PMIX_ERR_INVALID_NAMESPACE == cb->status) {
+        (void)strncpy(proc.nspace, nspace, PMIX_MAX_NSLEN);
+        proc.rank = PMIX_RANK_WILDCARD;
+        /* any key will suffice as it will bring down
+         * the entire data blob */
+        rc = PMIx_Get(&proc, PMIX_UNIV_SIZE, NULL, 0, NULL);
+        if (PMIX_SUCCESS != rc) {
+            PMIX_RELEASE(cb);
+            return rc;
+        }
+        /* retry the fetch */
+        cb->lock.active = true;
+        PMIX_THREADSHIFT(cb, _resolve_peers);
+        PMIX_WAIT_THREAD(&cb->lock);
+    }
+    *procs = cb->procs;
+    *nprocs = cb->nprocs;
+
+    rc = cb->status;
+    PMIX_RELEASE(cb);
+    return rc;
+}
+
+static void _resolve_nodes(int fd, short args, void *cbdata)
+{
+    pmix_cb_t *cb = (pmix_cb_t*)cbdata;
+    char *regex, **names;
+
+    /* get a regular expression describing the PMIX_NODE_MAP */
+    cb->status = pmix_preg.resolve_nodes(cb->pname.nspace, &regex);
+    if (PMIX_SUCCESS == cb->status) {
+        /* parse it into an argv array of names */
+        cb->status = pmix_preg.parse_nodes(regex, &names);
+        if (PMIX_SUCCESS == cb->status) {
+            /* assemble it into a comma-delimited list */
+            cb->key = pmix_argv_join(names, ',');
+            pmix_argv_free(names);
+        } else {
+            free(regex);
+        }
+    }
+    /* post the data so the receiving thread can acquire it */
+    PMIX_POST_OBJECT(cb);
+    PMIX_WAKEUP_THREAD(&cb->lock);
 }
 
 /* need to thread-shift this request */
 PMIX_EXPORT pmix_status_t PMIx_Resolve_nodes(const char *nspace, char **nodelist)
 {
+    pmix_cb_t *cb;
+    pmix_status_t rc;
+    pmix_proc_t proc;
+
     PMIX_ACQUIRE_THREAD(&pmix_global_lock);
     if (pmix_globals.init_cntr <= 0) {
         PMIX_RELEASE_THREAD(&pmix_global_lock);
         return PMIX_ERR_INIT;
     }
     PMIX_RELEASE_THREAD(&pmix_global_lock);
 
-    /* set default */
-    *nodelist = NULL;
+    cb = PMIX_NEW(pmix_cb_t);
+    cb->pname.nspace = strdup(nspace);
+
+    PMIX_THREADSHIFT(cb, _resolve_nodes);
+
+    /* wait for the result */
+    PMIX_WAIT_THREAD(&cb->lock);
+
+    /* if the nspace wasn't found, then we need to
+     * ask the server for that info */
+    if (PMIX_ERR_INVALID_NAMESPACE == cb->status) {
+        (void)strncpy(proc.nspace, nspace, PMIX_MAX_NSLEN);
+        proc.rank = PMIX_RANK_WILDCARD;
+        /* any key will suffice as it will bring down
+         * the entire data blob */
+        rc = PMIx_Get(&proc, PMIX_UNIV_SIZE, NULL, 0, NULL);
+        if (PMIX_SUCCESS != rc) {
+            PMIX_RELEASE(cb);
+            return rc;
+        }
+        /* retry the fetch */
+        cb->lock.active = true;
+        PMIX_THREADSHIFT(cb, _resolve_nodes);
+        PMIX_WAIT_THREAD(&cb->lock);
+    }
+    /* the string we want is in the key field */
+    *nodelist = cb->key;
 
-    return pmix_preg.resolve_nodes(nspace, nodelist);
+    rc = cb->status;
+    PMIX_RELEASE(cb);
+    return rc;
 }

opal/mca/pmix/pmix3x/pmix/src/mca/gds/hash/gds_hash.c

@@ -333,7 +333,8 @@ static pmix_status_t store_map(pmix_hash_table_t *ht,
     }
 
     /* store the comma-delimited list of nodes hosting
-     * procs in this nspace */
+     * procs in this nspace in case someone using PMIx v2
+     * requests it */
     kp2 = PMIX_NEW(pmix_kval_t);
     kp2->key = strdup(PMIX_NODE_LIST);
     kp2->value = (pmix_value_t*)malloc(sizeof(pmix_value_t));
@@ -397,6 +398,19 @@ pmix_status_t hash_cache_job_info(struct pmix_nspace_t *ns,
     ht = &trk->internal;
     for (n=0; n < ninfo; n++) {
         if (0 == strcmp(info[n].key, PMIX_NODE_MAP)) {
+            /* store the node map itself since that is
+             * what v3 uses */
+            kp2 = PMIX_NEW(pmix_kval_t);
+            kp2->key = strdup(PMIX_NODE_MAP);
+            kp2->value = (pmix_value_t*)malloc(sizeof(pmix_value_t));
+            kp2->value->type = PMIX_STRING;
+            kp2->value->data.string = strdup(info[n].value.data.string);
+            if (PMIX_SUCCESS != (rc = pmix_hash_store(ht, PMIX_RANK_WILDCARD, kp2))) {
+                PMIX_ERROR_LOG(rc);
+                PMIX_RELEASE(kp2);
+                return rc;
+            }
+
             /* parse the regex to get the argv array of node names */
             if (PMIX_SUCCESS != (rc = pmix_preg.parse_nodes(info[n].value.data.string, &nodes))) {
                 PMIX_ERROR_LOG(rc);

opal/mca/pmix/pmix3x/pmix/src/mca/pnet/opa/pnet_opa.c

@@ -37,6 +37,7 @@
 #include "src/util/error.h"
 #include "src/util/output.h"
 #include "src/util/pmix_environ.h"
+#include "src/mca/preg/preg.h"
 
 #include "src/mca/pnet/pnet.h"
 #include "src/mca/pnet/base/base.h"
@@ -298,6 +299,10 @@ static pmix_status_t setup_local_network(pmix_nspace_t *nptr,
     size_t n;
     pmix_status_t rc;
     pmix_kval_t *kv;
+    char *nodestring, **nodes;
+    pmix_proc_t *procs;
+    size_t nprocs;
+
 
     if (NULL != info) {
         for (n=0; n < ninfo; n++) {
@@ -321,6 +326,7 @@ static pmix_status_t setup_local_network(pmix_nspace_t *nptr,
             }
         }
     }
+
     return PMIX_SUCCESS;
 }
 

opal/mca/pmix/pmix3x/pmix/src/mca/pnet/test/Makefile.am

@@ -0,0 +1,52 @@
+# -*- makefile -*-
+#
+# Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
+#                         University Research and Technology
+#                         Corporation.  All rights reserved.
+# Copyright (c) 2004-2005 The University of Tennessee and The University
+#                         of Tennessee Research Foundation.  All rights
+#                         reserved.
+# Copyright (c) 2004-2005 High Performance Computing Center Stuttgart,
+#                         University of Stuttgart.  All rights reserved.
+# Copyright (c) 2004-2005 The Regents of the University of California.
+#                         All rights reserved.
+# Copyright (c) 2012      Los Alamos National Security, Inc.  All rights reserved.
+# Copyright (c) 2013-2018 Intel, Inc. All rights reserved.
+# Copyright (c) 2017      Research Organization for Information Science
+#                         and Technology (RIST). All rights reserved.
+# $COPYRIGHT$
+#
+# Additional copyrights may follow
+#
+# $HEADER$
+#
+
+headers = pnet_test.h
+sources = \
+        pnet_test_component.c \
+        pnet_test.c
+
+# Make the output library in this directory, and name it either
+# mca_<type>_<name>.la (for DSO builds) or libmca_<type>_<name>.la
+# (for static builds).
+
+if MCA_BUILD_pmix_pnet_test_DSO
+lib =
+lib_sources =
+component = mca_pnet_test.la
+component_sources = $(headers) $(sources)
+else
+lib = libmca_pnet_test.la
+lib_sources = $(headers) $(sources)
+component =
+component_sources =
+endif
+
+mcacomponentdir = $(pmixlibdir)
+mcacomponent_LTLIBRARIES = $(component)
+mca_pnet_test_la_SOURCES = $(component_sources)
+mca_pnet_test_la_LDFLAGS = -module -avoid-version
+
+noinst_LTLIBRARIES = $(lib)
+libmca_pnet_test_la_SOURCES = $(lib_sources)
+libmca_pnet_test_la_LDFLAGS = -module -avoid-version