From 034c421a457b9dd5c654cb26292d9c05b1cd9244 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Thu, 24 Oct 2024 17:36:40 -0500 Subject: [PATCH 12/16] Low: pacemaker-attrd: track node XML IDs independent of attribute values Previously, node XML IDs were kept in attribute_value_t. That meant that they were duplicated for every value for a node, the values might be known for some values and unknown or inconsistent for others, and newly learned XML IDs would have to be broadcast per value. Now, maintain a global node XML ID cache. --- daemons/attrd/Makefile.am | 1 + daemons/attrd/attrd_attributes.c | 2 +- daemons/attrd/attrd_cib.c | 25 +++++----- daemons/attrd/attrd_corosync.c | 11 +++-- daemons/attrd/attrd_nodes.c | 82 ++++++++++++++++++++++++++++++++ daemons/attrd/attrd_utils.c | 1 - daemons/attrd/pacemaker-attrd.c | 2 + daemons/attrd/pacemaker-attrd.h | 6 +++ 8 files changed, 112 insertions(+), 18 deletions(-) create mode 100644 daemons/attrd/attrd_nodes.c diff --git a/daemons/attrd/Makefile.am b/daemons/attrd/Makefile.am index 47119679cf..a2c8fd1477 100644 --- a/daemons/attrd/Makefile.am +++ b/daemons/attrd/Makefile.am @@ -31,6 +31,7 @@ pacemaker_attrd_SOURCES = attrd_alerts.c \ attrd_elections.c \ attrd_ipc.c \ attrd_messages.c \ + attrd_nodes.c \ attrd_sync.c \ attrd_utils.c \ pacemaker-attrd.c diff --git a/daemons/attrd/attrd_attributes.c b/daemons/attrd/attrd_attributes.c index 6d80acfce1..fdc238375e 100644 --- a/daemons/attrd/attrd_attributes.c +++ b/daemons/attrd/attrd_attributes.c @@ -150,7 +150,7 @@ attrd_add_value_xml(xmlNode *parent, const attribute_t *a, * cluster IDs, we will have to drop support for rolling upgrades from * versions before those. */ - crm_xml_add(xml, PCMK__XA_ATTR_HOST_ID, v->node_xml_id); + crm_xml_add(xml, PCMK__XA_ATTR_HOST_ID, attrd_get_node_xml_id(v->nodename)); crm_xml_add(xml, PCMK__XA_ATTR_VALUE, v->current); crm_xml_add_int(xml, PCMK__XA_ATTR_DAMPENING, diff --git a/daemons/attrd/attrd_cib.c b/daemons/attrd/attrd_cib.c index 193b06739e..4231e4a668 100644 --- a/daemons/attrd/attrd_cib.c +++ b/daemons/attrd/attrd_cib.c @@ -449,11 +449,14 @@ send_alert_attributes_value(attribute_t *a, GHashTable *t) g_hash_table_iter_init(&vIter, t); while (g_hash_table_iter_next(&vIter, NULL, (gpointer *) & at)) { - rc = attrd_send_attribute_alert(at->nodename, at->node_xml_id, + const char *node_xml_id = attrd_get_node_xml_id(at->nodename); + + rc = attrd_send_attribute_alert(at->nodename, node_xml_id, a->id, at->current); crm_trace("Sent alerts for %s[%s]=%s with node XML ID %s " "(%s agents failed)", - a->id, at->nodename, at->current, at->node_xml_id, + a->id, at->nodename, at->current, + pcmk__s(node_xml_id, "unknown"), ((rc == 0)? "no" : ((rc == -1)? "some" : "all"))); } } @@ -463,7 +466,6 @@ set_alert_attribute_value(GHashTable *t, attribute_value_t *v) { attribute_value_t *a_v = pcmk__assert_alloc(1, sizeof(attribute_value_t)); - a_v->node_xml_id = pcmk__str_copy(v->node_xml_id); a_v->nodename = pcmk__str_copy(v->nodename); a_v->current = pcmk__str_copy(v->current); @@ -554,6 +556,7 @@ write_attribute(attribute_t *a, bool ignore_delay) g_hash_table_iter_init(&iter, a->values); while (g_hash_table_iter_next(&iter, NULL, (gpointer *) &v)) { const char *node_xml_id = NULL; + const char *prev_xml_id = NULL; if (!should_write) { private_updates++; @@ -566,6 +569,8 @@ write_attribute(attribute_t *a, bool ignore_delay) * a fallback. */ + prev_xml_id = attrd_get_node_xml_id(v->nodename); + if (pcmk_is_set(v->flags, attrd_value_remote)) { // A Pacemaker Remote node's XML ID is the same as its name node_xml_id = v->nodename; @@ -573,12 +578,12 @@ write_attribute(attribute_t *a, bool ignore_delay) } else { // This creates a cluster node cache entry if none exists pcmk__node_status_t *peer = pcmk__get_node(0, v->nodename, - v->node_xml_id, + prev_xml_id, pcmk__node_search_any); node_xml_id = pcmk__cluster_get_xml_id(peer); if (node_xml_id == NULL) { - node_xml_id = v->node_xml_id; + node_xml_id = prev_xml_id; } } @@ -591,15 +596,11 @@ write_attribute(attribute_t *a, bool ignore_delay) continue; } - /* Remember the XML ID and let peers know it (in case one of them - * becomes the writer later) - */ - if (!pcmk__str_eq(v->node_xml_id, node_xml_id, pcmk__str_none)) { + if (!pcmk__str_eq(prev_xml_id, node_xml_id, pcmk__str_none)) { crm_trace("Setting %s[%s] node XML ID to %s (was %s)", a->id, v->nodename, node_xml_id, - pcmk__s(v->node_xml_id, "unknown")); - pcmk__str_update(&(v->node_xml_id), node_xml_id); - attrd_broadcast_value(a, v); + pcmk__s(prev_xml_id, "unknown")); + attrd_set_node_xml_id(v->nodename, node_xml_id); } // Update this value as part of the CIB transaction we're building diff --git a/daemons/attrd/attrd_corosync.c b/daemons/attrd/attrd_corosync.c index 02816b94d2..e97e09cb86 100644 --- a/daemons/attrd/attrd_corosync.c +++ b/daemons/attrd/attrd_corosync.c @@ -222,6 +222,7 @@ update_attr_on_host(attribute_t *a, const pcmk__node_status_t *peer, int is_remote = 0; bool changed = false; attribute_value_t *v = NULL; + const char *prev_xml_id = NULL; const char *node_xml_id = crm_element_value(xml, PCMK__XA_ATTR_HOST_ID); // Create entry for value if not already existing @@ -236,8 +237,9 @@ update_attr_on_host(attribute_t *a, const pcmk__node_status_t *peer, /* If update doesn't contain the node XML ID, fall back to any previously * known value (for logging) */ + prev_xml_id = attrd_get_node_xml_id(v->nodename); if (node_xml_id == NULL) { - node_xml_id = v->node_xml_id; + node_xml_id = prev_xml_id; } // If value is for a Pacemaker Remote node, remember that @@ -317,11 +319,11 @@ update_attr_on_host(attribute_t *a, const pcmk__node_status_t *peer, // Remember node's XML ID if we're just learning it if ((node_xml_id != NULL) - && !pcmk__str_eq(node_xml_id, v->node_xml_id, pcmk__str_none)) { + && !pcmk__str_eq(node_xml_id, prev_xml_id, pcmk__str_none)) { crm_trace("Learned %s[%s] node XML ID is %s (was %s)", a->id, v->nodename, node_xml_id, - pcmk__s(v->node_xml_id, "unknown")); - pcmk__str_update(&(v->node_xml_id), node_xml_id); + pcmk__s(prev_xml_id, "unknown")); + attrd_set_node_xml_id(v->nodename, node_xml_id); if (attrd_election_won()) { // In case we couldn't write a value missing the XML ID before attrd_write_attributes(attrd_write_changed); @@ -540,6 +542,7 @@ attrd_peer_remove(const char *host, bool uncache, const char *source) if (uncache) { pcmk__purge_node_from_cache(host, 0); + attrd_forget_node_xml_id(host); } } diff --git a/daemons/attrd/attrd_nodes.c b/daemons/attrd/attrd_nodes.c new file mode 100644 index 0000000000..8fb7797f2d --- /dev/null +++ b/daemons/attrd/attrd_nodes.c @@ -0,0 +1,82 @@ +/* + * Copyright 2024-2025 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU General Public License version 2 + * or later (GPLv2+) WITHOUT ANY WARRANTY. + */ + +#include + +#include // NULL +#include // GHashTable, etc. + +#include "pacemaker-attrd.h" + +// Track the last known node XML ID for each node name +static GHashTable *node_xml_ids = NULL; + +/*! + * \internal + * \brief Get last known XML ID for a given node + * + * \param[in] node_name Name of node to check + * + * \return Last known XML ID for node (or NULL if none known) + * + * \note The return value may become invalid if attrd_set_node_xml_id() or + * attrd_forget_node_xml_id() is later called for \p node_name. + */ +const char * +attrd_get_node_xml_id(const char *node_name) +{ + if (node_xml_ids == NULL) { + return NULL; + } + return g_hash_table_lookup(node_xml_ids, node_name); +} + +/*! + * \internal + * \brief Set last known XML ID for a given node + * + * \param[in] node_name Name of node to set + * \param[in] node_xml_id New XML ID to set for node + */ +void +attrd_set_node_xml_id(const char *node_name, const char *node_xml_id) +{ + if (node_xml_ids == NULL) { + node_xml_ids = pcmk__strikey_table(free, free); + } + pcmk__insert_dup(node_xml_ids, node_name, node_xml_id); +} + +/*! + * \internal + * \brief Forget last known XML ID for a given node + * + * \param[in] node_name Name of node to forget + */ +void +attrd_forget_node_xml_id(const char *node_name) +{ + if (node_xml_ids == NULL) { + return; + } + g_hash_table_remove(node_xml_ids, node_name); +} + +/*! + * \internal + * \brief Free the node XML ID cache + */ +void +attrd_cleanup_xml_ids(void) +{ + if (node_xml_ids != NULL) { + g_hash_table_destroy(node_xml_ids); + node_xml_ids = NULL; + } +} diff --git a/daemons/attrd/attrd_utils.c b/daemons/attrd/attrd_utils.c index 3621f5f354..f219b8862d 100644 --- a/daemons/attrd/attrd_utils.c +++ b/daemons/attrd/attrd_utils.c @@ -232,7 +232,6 @@ attrd_free_attribute_value(gpointer data) free(v->nodename); free(v->current); free(v->requested); - free(v->node_xml_id); free(v); } diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c index a5dac1272a..3c31bcd932 100644 --- a/daemons/attrd/pacemaker-attrd.c +++ b/daemons/attrd/pacemaker-attrd.c @@ -207,6 +207,8 @@ main(int argc, char **argv) g_hash_table_destroy(attributes); } + attrd_cleanup_xml_ids(); + g_strfreev(processed_args); pcmk__free_arg_context(context); diff --git a/daemons/attrd/pacemaker-attrd.h b/daemons/attrd/pacemaker-attrd.h index f0535eabaa..57d707c37c 100644 --- a/daemons/attrd/pacemaker-attrd.h +++ b/daemons/attrd/pacemaker-attrd.h @@ -252,4 +252,10 @@ bool attrd_request_has_sync_point(xmlNode *xml); extern gboolean stand_alone; +// Node utilities (from attrd_nodes.c) +const char *attrd_get_node_xml_id(const char *node_name); +void attrd_set_node_xml_id(const char *node_name, const char *node_xml_id); +void attrd_forget_node_xml_id(const char *node_name); +void attrd_cleanup_xml_ids(void); + #endif /* PACEMAKER_ATTRD__H */ -- 2.43.0