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.
flux-framework · Apr 11, 2024 · c053cab · c053cab
1 parent 112da32
commit c053cab
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 69 deletions.
diff --git 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
@@ -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
@@ -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