From c053cabe575ab55cb18011e2140869e707529315 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 11 Apr 2024 10:21:32 -0700 Subject: [PATCH] job-manager: refactor annotate clear interface Problem: code for clearing annotations and posting a NULL annotations event is duplicated in several places in alloc.c Replace annotations_clear() and annotations_sched_clear() with annotations_clear_and_publish(), which cuts down on the code duplication. --- src/modules/job-manager/alloc.c | 54 +++--------------------------- src/modules/job-manager/annotate.c | 45 +++++++++++++++---------- src/modules/job-manager/annotate.h | 10 ++++-- 3 files changed, 40 insertions(+), 69 deletions(-) diff --git a/src/modules/job-manager/alloc.c b/src/modules/job-manager/alloc.c index 44d889bf6027..4f534db05c75 100644 --- a/src/modules/job-manager/alloc.c +++ b/src/modules/job-manager/alloc.c @@ -60,7 +60,6 @@ static void requeue_pending (struct alloc *alloc, struct job *job) { struct job_manager *ctx = alloc->ctx; bool fwd = job->priority > (FLUX_JOB_PRIORITY_MAX / 2); - bool cleared = false; assert (job->alloc_pending); if (job->handle) { @@ -74,18 +73,7 @@ static void requeue_pending (struct alloc *alloc, struct job *job) flux_log (ctx->h, LOG_ERR, "failed to enqueue job for scheduling"); job->alloc_queued = 1; } - annotations_sched_clear (job, &cleared); - if (cleared) { - if (event_job_post_pack (ctx->event, - job, - "annotations", - EVENT_NO_COMMIT, - "{s:n}", - "annotations") < 0) - flux_log_error (ctx->h, - "%s: event_job_post_pack", - __FUNCTION__); - } + annotations_clear_and_publish (ctx, job, "sched"); } /* Initiate teardown. Clear any alloc/free requests, and clear @@ -176,7 +164,6 @@ static void alloc_response_cb (flux_t *h, json_t *annotations = NULL; json_t *R = NULL; struct job *job; - bool cleared = false; if (flux_response_decode (msg, NULL, NULL) < 0) goto teardown; // ENOSYS here if scheduler not loaded/shutting down @@ -261,19 +248,7 @@ static void alloc_response_cb (flux_t *h, flux_log (ctx->h, LOG_ERR, "failed to dequeue pending job"); job->handle = NULL; } - annotations_clear (job, &cleared); - if (cleared) { - if (event_job_post_pack (ctx->event, - job, - "annotations", - EVENT_NO_COMMIT, - "{s:n}", - "annotations") < 0) - flux_log_error (ctx->h, - "%s: event_job_post_pack: id=%s", - __FUNCTION__, - idf58 (id)); - } + annotations_clear_and_publish (ctx, job, NULL); if (raise_job_exception (ctx, job, "alloc", @@ -294,21 +269,9 @@ static void alloc_response_cb (flux_t *h, flux_log (ctx->h, LOG_ERR, "failed to dequeue pending job"); job->handle = NULL; } - annotations_clear (job, &cleared); + annotations_clear_and_publish (ctx, job, NULL); } job->alloc_pending = 0; - if (cleared) { - if (event_job_post_pack (ctx->event, - job, - "annotations", - EVENT_NO_COMMIT, - "{s:n}", - "annotations") < 0) - flux_log_error (ctx->h, - "%s: event_job_post_pack: id=%s", - __FUNCTION__, - idf58 (id)); - } if (queue_started (alloc->ctx->queue, job)) { if (event_job_action (ctx->event, job) < 0) { flux_log_error (h, @@ -628,16 +591,7 @@ int alloc_cancel_alloc_request (struct alloc *alloc, (void)zlistx_delete (alloc->pending_jobs, job->handle); job->handle = NULL; } - bool cleared = false; - annotations_clear (job, &cleared); - if (cleared) { - (void)event_job_post_pack (alloc->ctx->event, - job, - "annotations", - EVENT_NO_COMMIT, - "{s:n}", - "annotations"); - } + annotations_clear_and_publish (alloc->ctx, job, NULL); } } return 0; diff --git a/src/modules/job-manager/annotate.c b/src/modules/job-manager/annotate.c index d6fc74c10b99..22327014c18b 100644 --- a/src/modules/job-manager/annotate.c +++ b/src/modules/job-manager/annotate.c @@ -29,6 +29,7 @@ #include "src/common/libczmqcontainers/czmq_containers.h" #include "src/common/libutil/jpath.h" +#include "src/common/libjob/idf58.h" #include "job.h" #include "event.h" @@ -40,26 +41,11 @@ struct annotate { flux_msg_handler_t **handlers; }; -void annotations_clear (struct job *job, bool *cleared) +static void annotations_clear (struct job *job) { if (job->annotations) { json_decref (job->annotations); job->annotations = NULL; - if (cleared) - (*cleared) = true; - } -} - -void annotations_sched_clear (struct job *job, bool *cleared) -{ - if (job->annotations) { - if (json_object_del (job->annotations, "sched") == 0) { - /* Special case if annotations are now empty */ - if (!json_object_size (job->annotations)) - annotations_clear (job, NULL); - if (cleared) - (*cleared) = true; - } } } @@ -93,11 +79,36 @@ int annotations_update (struct job *job, const char *path, json_t *annotations) * will handle advertisement of the clear. */ if (!json_object_size (job->annotations)) - annotations_clear (job, NULL); + annotations_clear (job); } return 0; } +void annotations_clear_and_publish (struct job_manager *ctx, + struct job *job, + const char *key) +{ + if (job->annotations) { + if (key) + (void)json_object_del (job->annotations, key); + else + (void)json_object_clear (job->annotations); + if (json_object_size (job->annotations) == 0) { + annotations_clear (job); + if (event_job_post_pack (ctx->event, + job, + "annotations", + EVENT_NO_COMMIT, + "{s:n}", + "annotations") < 0) { + flux_log_error (ctx->h, + "error posting null annotations event for %s", + idf58 (job->id)); + } + } + } +} + int annotations_update_and_publish (struct job_manager *ctx, struct job *job, json_t *annotations) diff --git a/src/modules/job-manager/annotate.h b/src/modules/job-manager/annotate.h index 291f41fd3094..8e17e74abb3c 100644 --- a/src/modules/job-manager/annotate.h +++ b/src/modules/job-manager/annotate.h @@ -16,8 +16,6 @@ #include "job.h" #include "job-manager.h" -void annotations_clear (struct job *job, bool *cleared); -void annotations_sched_clear (struct job *job, bool *cleared); int annotations_update (struct job *job, const char *path, json_t *annotations); struct annotate *annotate_ctx_create (struct job_manager *ctx); @@ -30,6 +28,14 @@ int annotations_update_and_publish (struct job_manager *ctx, struct job *job, json_t *annotations); +/* clear key from annotations, or clear all annotations if key == NULL. + * If that transitioned the annotations object from non-empty to empty, + * post an annotations event with the context of {"annotations":null}. + */ +void annotations_clear_and_publish (struct job_manager *ctx, + struct job *job, + const char *key); + #endif /* ! _FLUX_JOB_MANAGER_ANNOTATE_H */ /* * vi:tabstop=4 shiftwidth=4 expandtab