housekeeping: improve internal list management

Problem: the hk->allocations list is manipulated directly in
several places, and deletion is a bit awkward.

Store the list cursor in the list item so it doesn't have to be
looked up using zlistx_find() for deletion.

Streamline error handling code in housekeeping_start() by starting
bulk execution before adding the allocation to the list.

Log unexpected internal errors at LOG_CRIT.

Author: garlick

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,
@@ -430,34 +429,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 +539,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);
     }