Skip to content

Commit

Permalink
resource: ignore live resource.exclude changes
Browse files Browse the repository at this point in the history
Problem: the resource module contains code to process live changes
to the configured exclude set, but there is no longer a use case
now that flux-batch(1) et al have a --conf option that can configure
exclusions on the command line for subinstances.

The extra code includes posting exclude/unexclude events to the KVS
resource.eventlog synchronously by calling the internal function
reslog_sync().  This makes the resource module unresponsive while
the KVS commit completes.

This extra complexity is no longer justified.  Remove.
  • Loading branch information
garlick committed Jul 18, 2023
1 parent 0af0daa commit 7b2c417
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 100 deletions.
1 change: 0 additions & 1 deletion src/modules/resource/acquire.c
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ static void reslog_cb (struct reslog *reslog, const char *name, void *arg)
}
}
else if (streq (name, "online") || streq (name, "offline")
|| streq (name, "exclude") || streq (name, "unexclude")
|| streq (name, "drain") || streq (name, "undrain")) {
if (ar->response_count > 0) {
struct idset *up, *dn;
Expand Down
72 changes: 1 addition & 71 deletions src/modules/resource/exclude.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

/* exclude.c - maintain a list of exec targets excluded from scheduling
*
* Post exclude/unexclude event when configured exclusion set changes.
/* exclude.c - get static list of exec targets excluded from scheduling
*
* Caveats:
* - There is no way to exclude at a finer granularity than execution target
Expand Down Expand Up @@ -43,74 +41,6 @@ const struct idset *exclude_get (struct exclude *exclude)
return exclude->idset;
}

/* Update exclusion set and post event.
*/
int exclude_update (struct exclude *exclude,
const char *s,
flux_error_t *errp)
{
flux_t *h = exclude->ctx->h;
struct idset *idset = NULL;
struct idset *add;
struct idset *del;

if (s) {
if (!(idset = inventory_targets_to_ranks (exclude->ctx->inventory,
s,
errp)))
return -1;
if (idset_count (idset) > 0
&& idset_last (idset) >= exclude->ctx->size) {
errprintf (errp, "exclusion idset is out of range");
idset_destroy (idset);
errno = EINVAL;
return -1;
}
}
if (rutil_idset_diff (exclude->idset, idset, &add, &del) < 0) {
errprintf (errp, "error analyzing exclusion set update");
idset_destroy (idset);
return -1;
}
if (add) {
char *add_s = idset_encode (add, IDSET_FLAG_RANGE);
if (!add_s || reslog_post_pack (exclude->ctx->reslog,
NULL,
0.,
"exclude",
"{s:s}",
"idset",
add_s) < 0) {
flux_log_error (h, "error posting exclude event");
}
/* Added exclude ranks can no longer be drained:
*/
if (undrain_ranks (exclude->ctx->drain, add) < 0)
flux_log_error (h,
"exclude: failed to undrain ranks in %s",
add_s);
free (add_s);
idset_destroy (add);
}
if (del) {
char *del_s = idset_encode (del, IDSET_FLAG_RANGE);
if (!del_s || reslog_post_pack (exclude->ctx->reslog,
NULL,
0.,
"unexclude",
"{s:s}",
"idset",
del_s) < 0) {
flux_log_error (h, "error posting unexclude event");
}
free (del_s);
idset_destroy (del);
}
idset_destroy (exclude->idset);
exclude->idset = idset;
return 0;
}

void exclude_destroy (struct exclude *exclude)
{
if (exclude) {
Expand Down
4 changes: 0 additions & 4 deletions src/modules/resource/exclude.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
struct exclude *exclude_create (struct resource_ctx *ctx, const char *idset);
void exclude_destroy (struct exclude *exclude);

int exclude_update (struct exclude *exclude,
const char *idset,
flux_error_t *errp);

const struct idset *exclude_get (struct exclude *exclude);

#endif /* !_FLUX_RESOURCE_EXCLUDE_H */
Expand Down
28 changes: 4 additions & 24 deletions src/modules/resource/resource.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ static int parse_config (struct resource_ctx *ctx,
return -1;
}
}
*excludep = exclude;
if (excludep)
*excludep = exclude;
if (noverifyp)
*noverifyp = noverify ? true : false;
if (norestrictp)
Expand All @@ -130,9 +131,7 @@ static int parse_config (struct resource_ctx *ctx,

/* Broker is sending us a new config object because 'flux config reload'
* was run. Parse it and respond with human readable errors.
* If events are posted, block until they complete so that:
* - any KVS commit errors are captured by 'flux config reload'
* - tests can look for eventlog entry after running 'flux config reload'
* At the moment this doesn't do much - just cache the new config.
*/
static void config_reload_cb (flux_t *h,
flux_msg_handler_t *mh,
Expand All @@ -141,34 +140,15 @@ static void config_reload_cb (flux_t *h,
{
struct resource_ctx *ctx = arg;
const flux_conf_t *conf;
const char *exclude;
flux_error_t error;
const char *errstr = NULL;

if (flux_conf_reload_decode (msg, &conf) < 0)
goto error;
if (parse_config (ctx,
conf,
&exclude,
NULL,
NULL,
NULL,
&error) < 0) {
if (parse_config (ctx, conf, NULL, NULL, NULL, NULL, &error) < 0) {
errstr = error.text;
goto error;
}
if (ctx->rank == 0) {
if (exclude_update (ctx->exclude,
exclude,
&error) < 0) {
errstr = error.text;
goto error;
}
if (reslog_sync (ctx->reslog) < 0) {
errstr = "error posting to eventlog for reconfig";
goto error;
}
}
if (flux_set_conf (h, flux_conf_incref (conf)) < 0) {
errstr = "error updating cached configuration";
goto error;
Expand Down

0 comments on commit 7b2c417

Please sign in to comment.