Merge pull request #6191 from garlick/issue#6179

improve housekeeping logging and list management

Author: mergify[bot]

src/modules/job-manager/housekeeping.c

@@ -116,6 +116,7 @@ struct allocation {
     int free_count;         // number of releases
     double t_start;
     struct bulk_exec *bulk_exec;
+    void *list_handle;
 };
 
 struct housekeeping {
@@ -258,15 +259,13 @@ static void allocation_release (struct allocation *a)
 
 static void allocation_remove (struct allocation *a)
 {
-    void *cursor;
-    if (!(cursor = zlistx_find (a->hk->allocations, a))) {
+    if (!a->list_handle
+        || zlistx_delete (a->hk->allocations, a->list_handle) < 0) {
         flux_log (a->hk->ctx->h,
-                  LOG_ERR,
+                  LOG_CRIT,
                   "housekeeping: internal error removing allocation for %s",
                   idf58 (a->id));
-        return;
     }
-    zlistx_delete (a->hk->allocations, cursor);
 }
 
 static void allocation_timeout (flux_reactor_t *r,
@@ -318,6 +317,14 @@ static void set_failed_reason (const char **s, const char *reason)
         *s = "multiple failure modes";
 }
 
+static void bulk_start (struct bulk_exec *bulk_exec, void *arg)
+{
+    struct allocation *a = arg;
+    flux_t *h = a->hk->ctx->h;
+
+    flux_log (h, LOG_DEBUG, "housekeeping: %s started", idf58 (a->id));
+}
+
 static void bulk_exit (struct bulk_exec *bulk_exec,
                        void *arg,
                        const struct idset *ids)
@@ -377,7 +384,9 @@ static void bulk_exit (struct bulk_exec *bulk_exec,
 static void bulk_complete (struct bulk_exec *bulk_exec, void *arg)
 {
     struct allocation *a = arg;
+    flux_t *h = a->hk->ctx->h;
 
+    flux_log (h, LOG_DEBUG, "housekeeping: %s complete", idf58 (a->id));
     allocation_remove (a);
 }
 
@@ -430,34 +439,26 @@ int housekeeping_start (struct housekeeping *hk,
 {
     flux_t *h = hk->ctx->h;
     struct allocation *a;
-    void *list_handle;
 
     /* Housekeeping is not configured
      */
     if (!hk->cmd)
         goto skip;
 
-    /* Create the 'allocation' and put it in our list.
+    /* Create and start the 'allocation' and put it in our list.
+     * N.B. bulk_exec_start() starts watchers but does not send RPCs.
      */
     if (!(a = allocation_create (hk, R, id, userid))
-        || !(list_handle = zlistx_insert (hk->allocations, a, false))) {
+        || bulk_exec_start (h, a->bulk_exec) < 0
+        || !(a->list_handle = zlistx_insert (hk->allocations, a, false))) {
         flux_log (h,
                   LOG_ERR,
-                  "housekeeping: %s error saving alloc object (skipping)",
+                  "housekeeping: %s error creating alloc object"
+                  " - returning resources to the scheduler",
                   idf58 (id));
         allocation_destroy (a);
         goto skip;
     }
-    /* Start bulk execution.
-     */
-    if (bulk_exec_start (h, a->bulk_exec) < 0) {
-        flux_log (h,
-                  LOG_ERR,
-                  "housekeeping: %s error starting housekeeping tasks",
-                  idf58 (id));
-        zlistx_delete (hk->allocations, list_handle);
-        goto skip;
-    }
     return 0;
 skip:
     return alloc_send_free_request (hk->ctx->alloc, R, id, true);
@@ -548,7 +549,7 @@ int housekeeping_hello_respond (struct housekeeping *hk, const flux_msg_t *msg)
                 flux_future_destroy (f);
 
             // delete the allocation to avoid sending frees later
-            zlistx_delete (hk->allocations, zlistx_cursor (hk->allocations));
+            allocation_remove (a);
         }
         a = zlistx_next (hk->allocations);
     }
@@ -839,7 +840,7 @@ struct housekeeping *housekeeping_ctx_create (struct job_manager *ctx)
 }
 
 static struct bulk_exec_ops bulk_ops = {
-    .on_start = NULL,
+    .on_start = bulk_start,
     .on_exit = bulk_exit,
     .on_complete = bulk_complete,
     .on_output = bulk_output,