From 83ce749a172253779de66fd141e23746feb03a39 Mon Sep 17 00:00:00 2001
From: Utz-Uwe Haus <uhaus@cray.com>
Date: Thu, 11 Feb 2021 09:44:51 +0100
Subject: [PATCH] Push name attribute into attributes at SEAL time
ensures that subscriptions can match on the name in the same way that all other
attributes can be matched.
---
include/maestro/attributes.h | 9 ++++++++
maestro/attributes.c | 3 +++
maestro/cdo.c | 40 +++++++++++++++++++++++++++++++++
maestro/pool.c | 7 ------
maestro/subscription_registry.c | 32 ++++++++++++++++++--------
5 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/include/maestro/attributes.h b/include/maestro/attributes.h
index ecd390a6..0788c5d2 100644
--- a/include/maestro/attributes.h
+++ b/include/maestro/attributes.h
@@ -68,6 +68,15 @@ typedef struct mstro_attribute_dict_* mstro_cdo_attributes;
Predefined symbolic key strings exist for the well-known attributes
**/
+/**@brief maestro.core.cdo.name
+ **
+ ** The name of the CDO
+ **
+ ** C-side data type: `char *'
+ ** default value: false (set automatically by mstro_cdo_declare())
+ **/
+extern const char *MSTRO_ATTR_CORE_CDO_NAME;
+
/**@brief maestro.core.cdo.allocate-now
**
** Indicate that Maestro should perform allocation before DECLARE completes
diff --git a/maestro/attributes.c b/maestro/attributes.c
index e9105e84..8a3250ad 100644
--- a/maestro/attributes.c
+++ b/maestro/attributes.c
@@ -50,6 +50,9 @@
/* FIXME: we should have a table of name/type/... and make the const
* char* refer to the entries in that */
+const char *MSTRO_ATTR_CORE_CDO_NAME =
+ ".maestro.core.cdo.name";
+
const char *MSTRO_ATTR_CORE_CDO_ALLOCATE_NOW =
".maestro.core.cdo.allocate-now";
diff --git a/maestro/cdo.c b/maestro/cdo.c
index 37f25cca..8261cb45 100644
--- a/maestro/cdo.c
+++ b/maestro/cdo.c
@@ -639,6 +639,46 @@ mstro_cdo_declaration_seal(mstro_cdo cdo)
goto BAILOUT;
}
}
+
+ /* Ensure a sane name is filled in. The attribute is, in fact, only
+ * needed for subscription handling (so users can match on names the
+ * same way that they can for other attributes).
+ *
+ * The user does not typically set the name attribute explicitly,
+ * but if they do we need to ensure it's not in disagreement with
+ * the name specified at mstro_cdo_declare() time. */
+ const void *cdo_name_attrval=NULL;
+ enum mstro_cdo_attr_value_type valtype;
+
+ mstro_status s_cdoname = mstro_attribute_dict_get(
+ cdo->attributes, MSTRO_ATTR_CORE_CDO_NAME,
+ &valtype, &cdo_name_attrval, NULL, false);
+ if(s_cdoname==MSTRO_OK || s_cdoname==MSTRO_NOENT) {
+ const char *n_cdo = mstro_cdo_name(cdo);
+ if(s_cdoname==MSTRO_OK) {
+ /* ok, already have a name, check it matches */
+ const char *n_attr = (const char *)cdo_name_attrval;
+
+ if(0!=strcmp(n_cdo,n_attr)) {
+ ERR("CDO |%s| has name attribute set to |%s|, overriding\n");
+ }
+ }
+
+ s_cdoname = mstro_attribute_dict_set(cdo->attributes,
+ MSTRO_ATTR_CORE_CDO_NAME,
+ MSTRO_CDO_ATTR_VALUE_cstring,
+ &n_cdo, true);
+ if(s_cdoname!=MSTRO_OK) {
+ ERR("Failed to set CDO name attribute: %d (%s)\n", s_cdoname,
+ mstro_status_description(s_cdoname));
+ status = s_cdoname;
+ goto BAILOUT;
+ }
+ } else {
+ ERR("Failed to inquire about CDO name attribute\n");
+ status = s_cdoname;
+ goto BAILOUT;
+ }
/* we're done, but we need to ensure we've seen the DECLARE_ACK with
* the global ID by now */
diff --git a/maestro/pool.c b/maestro/pool.c
index 201fc667..4fd09ec4 100644
--- a/maestro/pool.c
+++ b/maestro/pool.c
@@ -1816,10 +1816,3 @@ mstro_subscription_dispose(mstro_subscription s)
-mstro_status
-mstro_pool__resolve_cdoid(const struct mstro_cdo_id *id,
- const char **name_p)
-{
- /* we use the subscription infrastructure for this */
- return mstro_pool_resolve_cdoid(id, name_p);
-}
diff --git a/maestro/subscription_registry.c b/maestro/subscription_registry.c
index 0574d50a..fb7cdab6 100644
--- a/maestro/subscription_registry.c
+++ b/maestro/subscription_registry.c
@@ -324,6 +324,7 @@ mstro__subscr_resolver_lookup_cdoid(const struct mstro_cdo_id *id,
goto ABORT;
}
}
+ DEBUG("Entering wait for resolver lookup reply\n");
/* setup done, time to wait. ResolveReply will be handled by
* @ref mstro_pool_resolve_reply_bh will will trigger our
* event */
@@ -1424,13 +1425,15 @@ const Mstro__Pool__KvEntry *
mstro_subscription__pool_attr_find(const Mstro__Pool__Attributes__Map *kv_map,
const char *key)
{
- if(kv_map==NULL)
+ if(kv_map==NULL) {
+ DEBUG("Empty kv map when looking for key |%s|\n", key);
return NULL;
- else {
- for(size_t i=0; i<kv_map->n_map; i++)
+ } else {
+ for(size_t i=0; i<kv_map->n_map; i++) {
if(strcmp(key,kv_map->map[i]->key)==0) {
return kv_map->map[i];
}
+ }
return NULL;
}
}
@@ -1526,7 +1529,8 @@ mstro_subscription__csq_eval(const struct mstro_csq_val *csq,
return MSTRO_NOMATCH;
}
} else {
- /* key matched. Now need to parse the value in the csq and call the appropriate comparison function */
+ /* key matched. Now need to parse the value in the csq and
+ * call the appropriate comparison function */
const Mstro__Pool__AVal *aval = entry->val;
void *rhsval=NULL;
size_t rhsval_size=0;
@@ -1605,7 +1609,8 @@ mstro_subscription_selector_eval(const struct mstro_cdo_id *cdoid,
struct mstro_csq_val *csq = sel->csq;
if(csq==NULL) {
- /* FIXME: maybe we need to create it on the first use of SEL if we're on the PM? */
+ /* FIXME: maybe we need to create it on the first use of SEL if
+ * we're on the PM? */
ERR("No parsed query (CSQ) available\n");
return MSTRO_FAIL;
}
@@ -1633,7 +1638,7 @@ mstro_subscription_selector_check(struct mstro_subscription_ *s,
/* declare is special: no CDOID known yet, no attribuites, only
* string name. Efficient subscriptions will subscribe to
* DECLARE_ACK instead. */
- WARN("DECLARE event always matches -- FIXME\n");
+ WARN("DECLARE event always matches -- FIXME: only name could be checked (and we don't)\n");
return MSTRO_OK;
} else {
/* we always need the CDO id. Unfortunately it's slightly buried in the EV */
@@ -1656,7 +1661,8 @@ mstro_subscription_selector_check(struct mstro_subscription_ *s,
ERR("Failed to serialized attribute message for CDO\n");
return status;
}
- status = mstro_subscription_selector_eval(&id, s->cdo_selector, attributes);
+ status = mstro_subscription_selector_eval(&id, s->cdo_selector,
+ attributes);
}
}
return status;
@@ -1901,8 +1907,14 @@ mstro_pool_event_consume(const Mstro__Pool__Event *eventmsg)
const char* cdo_name;
cdo_name = malloc(MSTRO_CDO_NAME_MAX);
assert(cdo_name!=NULL);
- mstro_status s = mstro__subscr_resolver_lookup_cdoid(
- (struct mstro_cdo_id*)eventmsg->offer->cdoid, &cdo_name);
+ struct mstro_cdo_id id = {
+ .qw[0] = eventmsg->offer->cdoid->qw0,
+ .qw[1] = eventmsg->offer->cdoid->qw1
+ };
+ WITH_CDO_ID_STR(idstr, &id, {
+ DEBUG("Resolving for OFFER event: %s\n", idstr);
+ });
+ mstro_status s = mstro__subscr_resolver_lookup_cdoid(&id, &cdo_name);
ev->offer.cdo_name = (char*)cdo_name;
DEBUG("Event: OFFER for |%s| from %" PRIu64 "\n",
ev->offer.cdo_name, ev->offer.appid);
@@ -2155,9 +2167,11 @@ mstro_subscriptions_finalize(void)
return status;
}
+/* called on pool client to handle resolver reply */
mstro_status
mstro_pool_resolve_reply_bh(const Mstro__Pool__ResolveReply *reply)
{
+ DEBUG("Resolver reply received\n");
if(reply==NULL ||reply->query==NULL) {
return MSTRO_INVMSG;
}
--
GitLab