Merge pull request #5256 from grondo/issue#5243

switch from decimal to f58 jobid encoding in most log messages and shell service name

Author: mergify[bot]

src/cmd/flux-job.c

@@ -43,6 +43,7 @@
 #include "src/common/libutil/jpath.h"
 #include "src/common/libjob/job.h"
 #include "src/common/libjob/unwrap.h"
+#include "src/common/libjob/idf58.h"
 #include "src/common/libutil/read_all.h"
 #include "src/common/libutil/monotime.h"
 #include "src/common/libutil/fsd.h"
@@ -1462,11 +1463,7 @@ size_t read_jobspec (const char *name, void **bufp)
 
 static void print_jobid (flux_jobid_t id)
 {
-    char buf[32];
-    if (flux_job_id_encode (id, "f58", buf, sizeof (buf)) < 0)
-        printf ("%ju\n", (uintmax_t) id);
-    else
-        printf ("%s\n", buf);
+    printf ("%s\n", idf58 (id));
 }
 
 int cmd_submit (optparse_t *p, int argc, char **argv)
@@ -3286,6 +3283,7 @@ void info_usage (void)
 }
 
 struct info_ctx {
+    const char *id_arg;
     flux_jobid_t id;
     json_t *keys;
     bool original;
@@ -3298,7 +3296,7 @@ void info_output (flux_future_t *f, const char *suffix, struct info_ctx *ctx)
     if (flux_rpc_get_unpack (f, "{s:s}", suffix, &s) < 0) {
         if (errno == ENOENT) {
             flux_future_destroy (f);
-            log_msg_exit ("job %ju id or key not found", (uintmax_t)ctx->id);
+            log_msg_exit ("job %s id or key not found", ctx->id_arg);
         }
         else
             log_err_exit ("flux_rpc_get_unpack");
@@ -3343,6 +3341,7 @@ void info_lookup (flux_t *h,
     flux_future_t *f;
     struct info_ctx ctx = {0};
 
+    ctx.id_arg = argv[optindex-1];
     ctx.id = id;
     if (!(ctx.keys = json_array ()))
         log_msg_exit ("json_array");
@@ -3426,13 +3425,6 @@ int cmd_stats (optparse_t *p, int argc, char **argv)
     return (0);
 }
 
-static char *to_f58 (flux_jobid_t id, char *buf, int len)
-{
-    if (flux_job_id_encode (id, "f58", buf, len) < 0)
-        (void) snprintf (buf, len, "%ju", (uintmax_t) id);
-    return buf;
-}
-
 int cmd_wait (optparse_t *p, int argc, char **argv)
 {
     flux_t *h;
@@ -3441,7 +3433,6 @@ int cmd_wait (optparse_t *p, int argc, char **argv)
     flux_jobid_t id = FLUX_JOBID_ANY;
     bool success;
     const char *errstr;
-    char buf[32];
     int rc = 0;
 
     if ((argc - optindex) > 1) {
@@ -3472,16 +3463,14 @@ int cmd_wait (optparse_t *p, int argc, char **argv)
                 log_msg_exit ("flux_job_wait_get_id: %s",
                               future_strerror (f, errno));
             if (!success) {
-                fprintf (stderr, "%s: %s\n",
-                         to_f58 (id, buf, sizeof (buf)),
-                         errstr);
+                fprintf (stderr, "%s: %s\n", idf58 (id), errstr);
                 rc = 1;
             }
             else {
                 if (optparse_hasopt (p, "verbose"))
                     fprintf (stderr,
                              "%s: job completed successfully\n",
-                             to_f58 (id, buf, sizeof (buf)));
+                             idf58 (id));
             }
             flux_future_destroy (f);
         }
@@ -3502,7 +3491,7 @@ int cmd_wait (optparse_t *p, int argc, char **argv)
         if (id == FLUX_JOBID_ANY) {
             if (flux_job_wait_get_id (f, &id) < 0)
                 log_err_exit ("flux_job_wait_get_id");
-            printf ("%s\n", to_f58 (id, buf, sizeof (buf)));
+            printf ("%s\n", idf58 (id));
         }
         if (!success)
             log_msg_exit ("%s", errstr);
@@ -3902,7 +3891,6 @@ int cmd_last (optparse_t *p, int argc, char **argv)
     int optindex = optparse_option_index (p);
     flux_future_t *f;
     flux_t *h;
-    char buf[32];
     json_t *jobs;
     size_t index;
     json_t *entry;
@@ -3941,12 +3929,8 @@ int cmd_last (optparse_t *p, int argc, char **argv)
     }
     if (json_array_size (jobs) == 0)
         log_msg_exit ("job history is empty");
-    json_array_foreach (jobs, index, entry) {
-        flux_jobid_t id = json_integer_value (entry);
-        if (flux_job_id_encode (id, "f58", buf, sizeof (buf)) < 0)
-            log_err_exit ("error encoding job ID");
-        printf ("%s\n", buf);
-    }
+    json_array_foreach (jobs, index, entry)
+        printf ("%s\n", idf58 (json_integer_value (entry)));
     flux_future_destroy (f);
     flux_close (h);
     return 0;

src/cmd/top/joblist_pane.c

@@ -18,6 +18,7 @@
 #include <jansson.h>
 
 #include "src/common/libutil/fsd.h"
+#include "src/common/libjob/idf58.h"
 #include "ccan/str/str.h"
 
 #include "top.h"
@@ -116,7 +117,7 @@ void joblist_pane_draw (struct joblist_pane *joblist)
 
     json_array_foreach (joblist->jobs, index, job) {
         char *uri = NULL;
-        char idstr[16];
+        const char *idstr;
         char run[16] = "";
         flux_jobid_t id;
         int userid;
@@ -143,8 +144,7 @@ void joblist_pane_draw (struct joblist_pane *joblist)
                              "uri", &uri) < 0)
             fatal (0, "error decoding a job record from job-list RPC");
 
-        if (flux_job_id_encode (id, "f58", idstr, sizeof (idstr)) < 0)
-            fatal (errno, "error encoding jobid as F58");
+        idstr = idf58 (id);
         (void)fsd_format_duration_ex (run, sizeof (run), fabs (now - t_run), 2);
         if (!(username = ucache_lookup (joblist->ucache, userid)))
             fatal (errno, "error looking up userid %d in ucache", (int)userid);
@@ -314,7 +314,6 @@ void joblist_pane_enter (struct joblist_pane *joblist)
     flux_jobid_t id;
     char *uri = NULL;
     char title [1024];
-    char jobid [24];
     flux_error_t error;
 
     json_t *job = get_current_job (joblist);
@@ -329,12 +328,11 @@ void joblist_pane_enter (struct joblist_pane *joblist)
         return;
     if (uri == NULL)
         return;
-    if (flux_job_id_encode (id, "f58", jobid, sizeof (jobid)) < 0
-        || snprintf (title,
-                     sizeof(title),
-                     "%s/%s",
-                     joblist->top->title,
-                     jobid) > sizeof (title))
+    if (snprintf (title,
+                  sizeof(title),
+                  "%s/%s",
+                  joblist->top->title,
+                  idf58 (id)) > sizeof (title))
         fatal (errno, "failed to build job title for job");
 
     /*  Lazily attempt to run top on jobid, but for now simply return to the

src/cmd/top/top.c

@@ -17,6 +17,7 @@
 
 #include "src/common/libutil/uri.h"
 #include "src/common/libutil/errprintf.h"
+#include "src/common/libjob/idf58.h"
 #include "ccan/str/str.h"
 #include "top.h"
 
@@ -224,18 +225,8 @@ static flux_jobid_t get_jobid (flux_t *h)
 
 static char * build_title (struct top *top, const char *title)
 {
-    if (!title) {
-        char jobid [24];
-        title = "";
-        if (top->id != FLUX_JOBID_ANY) {
-            if (flux_job_id_encode (top->id,
-                                    "f58",
-                                    jobid,
-                                    sizeof (jobid)) < 0)
-                fatal (errno, "failed to build jobid");
-            title = jobid;
-        }
-    }
+    if (!title)
+        title = idf58 (top->id);
     return strdup (title);
 }
 

src/common/libjob/Makefile.am

@@ -39,7 +39,8 @@ libjob_la_SOURCES = \
 	jj.c \
 	jj.h \
 	unwrap.c \
-	unwrap.h
+	unwrap.h \
+	idf58.h
 
 TESTS = \
 	test_job.t \

src/common/libjob/idf58.h

@@ -0,0 +1,35 @@
+/************************************************************\
+ * Copyright 2023 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, COPYING)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\************************************************************/
+
+#ifndef _IDF58_H
+#define _IDF58_H
+
+#include <flux/core.h>
+
+/*  Convenience function to convert a flux_jobid_t to F58 encoding
+ *  If the encode fails (unlikely), then the decimal encoding is returned.
+ */
+static inline const char *idf58 (flux_jobid_t id)
+{
+    static __thread char buf[21];
+    if (flux_job_id_encode (id, "f58", buf, sizeof (buf)) < 0) {
+        /* 64bit integer is guaranteed to fit in 21 bytes
+         * floor(log(2^64-1)/log(1)) + 1 = 20
+         */
+        (void) sprintf (buf, "%ju", (uintmax_t) id);
+    }
+    return buf;
+}
+
+#endif /* !_IDF58_H */
+
+/*
+ * vi:tabstop=4 shiftwidth=4 expandtab
+ */

src/common/libschedutil/hello.c

@@ -14,6 +14,7 @@
 #include <flux/core.h>
 #include <jansson.h>
 
+#include "src/common/libjob/idf58.h"
 #include "schedutil_private.h"
 #include "init.h"
 #include "hello.h"
@@ -25,14 +26,14 @@ static void raise_exception (flux_t *h, flux_jobid_t id, const char *note)
 
     flux_log (h,
               LOG_INFO,
-              "raising fatal exception on running job id=%ju",
-              (uintmax_t)id);
+              "raising fatal exception on running job id=%s",
+              idf58 (id));
 
     if (!(f = flux_job_raise (h, id, "scheduler-restart", 0, note))
         || flux_future_get (f, NULL) < 0) {
         flux_log_error (h,
-                        "error raising fatal exception on %ju: %s",
-                        (uintmax_t)id,
+                        "error raising fatal exception on %s: %s",
+                        idf58 (id),
                         future_strerror (f, errno));
     }
     flux_future_destroy (f);
@@ -64,8 +65,8 @@ static int schedutil_hello_job (schedutil_t *util,
     flux_future_destroy (f);
     return 0;
 error:
-    flux_log_error (util->h, "hello: error loading R for id=%ju",
-                    (uintmax_t)id);
+    flux_log_error (util->h, "hello: error loading R for id=%s",
+                    idf58 (id));
     flux_future_destroy (f);
     return -1;
 }

src/modules/job-archive/job-archive.c

@@ -28,6 +28,7 @@
 #include "src/common/libutil/fsd.h"
 #include "src/common/libutil/tstat.h"
 #include "src/common/libutil/monotime.h"
+#include "src/common/libjob/idf58.h"
 
 #define BUSY_TIMEOUT_DEFAULT 50
 #define BUFSIZE              1024
@@ -298,8 +299,8 @@ void job_info_lookup_continuation (flux_future_t *f, void *arg)
                         "t_run", &t_run,
                         "t_cleanup", &t_cleanup,
                         "t_inactive", &t_inactive) < 0) {
-        flux_log (ctx->h, LOG_ERR, "%s: parse job %ju error: %s",
-                  __FUNCTION__, (uintmax_t)id, error.text);
+        flux_log (ctx->h, LOG_ERR, "%s: parse job %s error: %s",
+                  __FUNCTION__, idf58 (id), error.text);
         goto out;
     }
 

src/modules/job-exec/bulk-exec.c

@@ -20,6 +20,7 @@
 
 #include "src/common/libczmqcontainers/czmq_containers.h"
 #include "src/common/libutil/aux.h"
+#include "src/common/libjob/idf58.h"
 #include "bulk-exec.h"
 
 struct exec_cmd {
@@ -491,8 +492,8 @@ void bulk_exec_kill_log_error (flux_future_t *f, flux_jobid_t id)
         if (flux_future_get (cf, NULL) < 0) {
             uint32_t rank = flux_rpc_get_nodeid (cf);
             flux_log_error (h,
-                            "%ju: exec_kill: %s (rank %lu)",
-                            (uintmax_t) id,
+                            "%s: exec_kill: %s (rank %lu)",
+                            idf58 (id),
                             flux_get_hostbyrank (h, rank),
                             (unsigned long)rank);
         }

src/modules/job-exec/exec.c

@@ -32,6 +32,7 @@
 
 #include <unistd.h>
 
+#include "src/common/libjob/idf58.h"
 #include "ccan/str/str.h"
 
 #include "job-exec.h"
@@ -457,18 +458,20 @@ static int exec_kill (struct jobinfo *job, int signum)
         f = bulk_exec_kill (exec, signum);
     if (!f) {
         if (errno != ENOENT)
-            flux_log_error (job->h, "%ju: bulk_exec_kill", job->id);
+            flux_log_error (job->h, "%s: bulk_exec_kill", idf58 (job->id));
         return 0;
     }
 
     flux_log (job->h, LOG_DEBUG,
-              "exec_kill: %ju: signal %d",
-              (uintmax_t) job->id,
+              "exec_kill: %s: signal %d",
+              idf58 (job->id),
               signum);
 
     jobinfo_incref (job);
     if (flux_future_then (f, 3., exec_kill_cb, job) < 0) {
-        flux_log_error (job->h, "%ju: exec_kill: flux_future_then", job->id);
+        flux_log_error (job->h,
+                        "%s: exec_kill: flux_future_then",
+                        idf58 (job->id));
         flux_future_destroy (f);
         return -1;
     }