Refactor ps_status.c API
authorPeter Eisentraut <peter@eisentraut.org>
Wed, 11 Mar 2020 15:36:40 +0000 (16:36 +0100)
committerPeter Eisentraut <peter@eisentraut.org>
Wed, 11 Mar 2020 15:38:31 +0000 (16:38 +0100)
The init_ps_display() arguments were mostly lies by now, so to match
typical usage, just use one argument and let the caller assemble it
from multiple sources if necessary.  The only user of the additional
arguments is BackendInitialize(), which was already doing string
assembly on the caller side anyway.

Remove the second argument of set_ps_display() ("force") and just
handle that in init_ps_display() internally.

BackendInitialize() also used to set the initial status as
"authentication", but that was very far from where authentication
actually happened.  So now it's set to "initializing" and then
"authentication" just before the actual call to
ClientAuthentication().

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com

19 files changed:
src/backend/access/transam/xlog.c
src/backend/bootstrap/bootstrap.c
src/backend/commands/async.c
src/backend/postmaster/autovacuum.c
src/backend/postmaster/bgworker.c
src/backend/postmaster/pgarch.c
src/backend/postmaster/pgstat.c
src/backend/postmaster/postmaster.c
src/backend/postmaster/syslogger.c
src/backend/replication/basebackup.c
src/backend/replication/syncrep.c
src/backend/replication/walreceiver.c
src/backend/replication/walsender.c
src/backend/storage/ipc/standby.c
src/backend/storage/lmgr/lock.c
src/backend/tcop/postgres.c
src/backend/utils/init/postinit.c
src/backend/utils/misc/ps_status.c
src/include/utils/ps_status.h

index 614a25242b5cd3f763c8ebe8f4ec2972cd017a57..4fa446ffa42d3de3f2ba70ba9b44e5fa8514d766 100644 (file)
@@ -3648,7 +3648,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
            /* Report recovery progress in PS display */
            snprintf(activitymsg, sizeof(activitymsg), "waiting for %s",
                     xlogfname);
-           set_ps_display(activitymsg, false);
+           set_ps_display(activitymsg);
 
            restoredFromArchive = RestoreArchivedFile(path, xlogfname,
                                                      "RECOVERYXLOG",
@@ -3691,7 +3691,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
        /* Report recovery progress in PS display */
        snprintf(activitymsg, sizeof(activitymsg), "recovering %s",
                 xlogfname);
-       set_ps_display(activitymsg, false);
+       set_ps_display(activitymsg);
 
        /* Track source of data in assorted state variables */
        readSource = source;
index 657b18ecc8949eabe6922dd63baca925ae8b3504..7923d1ec9b2053ed897ad423b121b0499274610a 100644 (file)
@@ -342,7 +342,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
                statmsg = "??? process";
                break;
        }
-       init_ps_display(statmsg, "", "", "");
+       init_ps_display(statmsg);
    }
 
    /* Acquire configuration parameters, unless inherited from postmaster */
index dae939a4ab8f9d8c4d0004d7986b69bd40fe6b51..0c9d20ebfc9720161be16061ebc488fc7e61072e 100644 (file)
@@ -2225,7 +2225,7 @@ ProcessIncomingNotify(void)
    if (Trace_notify)
        elog(DEBUG1, "ProcessIncomingNotify");
 
-   set_ps_display("notify interrupt", false);
+   set_ps_display("notify interrupt");
 
    /*
     * We must run asyncQueueReadAllNotifications inside a transaction, else
@@ -2242,7 +2242,7 @@ ProcessIncomingNotify(void)
     */
    pq_flush();
 
-   set_ps_display("idle", false);
+   set_ps_display("idle");
 
    if (Trace_notify)
        elog(DEBUG1, "ProcessIncomingNotify: done");
index e3a43d329662e178cf0de19d67ce0aed9327b006..a6499fc3da637ae31df313ecbb9f7e79f44a5efc 100644 (file)
@@ -434,7 +434,7 @@ AutoVacLauncherMain(int argc, char *argv[])
    am_autovacuum_launcher = true;
 
    /* Identify myself via ps */
-   init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_LAUNCHER), "", "", "");
+   init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_LAUNCHER));
 
    ereport(DEBUG1,
            (errmsg("autovacuum launcher started")));
@@ -1507,7 +1507,7 @@ AutoVacWorkerMain(int argc, char *argv[])
    am_autovacuum_worker = true;
 
    /* Identify myself via ps */
-   init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_WORKER), "", "", "");
+   init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_WORKER));
 
    SetProcessingMode(InitProcessing);
 
@@ -1680,7 +1680,7 @@ AutoVacWorkerMain(int argc, char *argv[])
         */
        InitPostgres(NULL, dbid, NULL, InvalidOid, dbname, false);
        SetProcessingMode(NormalProcessing);
-       set_ps_display(dbname, false);
+       set_ps_display(dbname);
        ereport(DEBUG1,
                (errmsg("autovacuum: processing database \"%s\"", dbname)));
 
index 75fc0d5d33f8b34579951f68070c3e17b7634863..684250984d4f32ea14d71f7f94cece9f424b5bd1 100644 (file)
@@ -689,7 +689,7 @@ StartBackgroundWorker(void)
    IsBackgroundWorker = true;
 
    /* Identify myself via ps */
-   init_ps_display(worker->bgw_name, "", "", "");
+   init_ps_display(worker->bgw_name);
 
    /*
     * If we're not supposed to have shared memory access, then detach from
index 3ca30badb2bc5533694574d41cc4beaf5fb6070a..58f54544f60e67bf1621d658a42302673a585e26 100644 (file)
@@ -241,7 +241,7 @@ PgArchiverMain(int argc, char *argv[])
    /*
     * Identify myself via ps
     */
-   init_ps_display("archiver", "", "", "");
+   init_ps_display("archiver");
 
    pgarch_MainLoop();
 
@@ -584,7 +584,7 @@ pgarch_archiveXlog(char *xlog)
 
    /* Report archive activity in PS display */
    snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
-   set_ps_display(activitymsg, false);
+   set_ps_display(activitymsg);
 
    rc = system(xlogarchcmd);
    if (rc != 0)
@@ -634,14 +634,14 @@ pgarch_archiveXlog(char *xlog)
        }
 
        snprintf(activitymsg, sizeof(activitymsg), "failed on %s", xlog);
-       set_ps_display(activitymsg, false);
+       set_ps_display(activitymsg);
 
        return false;
    }
    elog(DEBUG1, "archived write-ahead log file \"%s\"", xlog);
 
    snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
-   set_ps_display(activitymsg, false);
+   set_ps_display(activitymsg);
 
    return true;
 }
index 462b4d7e069bbaf9050c6950e1c4a07699c4d638..107c96533668b8f41c49ceebe2d4f9404db439a8 100644 (file)
@@ -4450,7 +4450,7 @@ PgstatCollectorMain(int argc, char *argv[])
    /*
     * Identify myself via ps
     */
-   init_ps_display("stats collector", "", "", "");
+   init_ps_display("stats collector");
 
    /*
     * Read in existing stats files or initialize the stats to zero.
index 55187eb910d20d8aba9d65749c65238cea9f9423..46be78aadbf38b5a9d5730cefaa2eb68e231e671 100644 (file)
@@ -4282,7 +4282,7 @@ BackendInitialize(Port *port)
    int         ret;
    char        remote_host[NI_MAXHOST];
    char        remote_port[NI_MAXSERV];
-   char        remote_ps_data[NI_MAXHOST];
+   StringInfoData ps_data;
 
    /* Save port etc. for ps status */
    MyProcPort = port;
@@ -4346,10 +4346,6 @@ BackendInitialize(Port *port)
        ereport(WARNING,
                (errmsg_internal("pg_getnameinfo_all() failed: %s",
                                 gai_strerror(ret))));
-   if (remote_port[0] == '\0')
-       snprintf(remote_ps_data, sizeof(remote_ps_data), "%s", remote_host);
-   else
-       snprintf(remote_ps_data, sizeof(remote_ps_data), "%s(%s)", remote_host, remote_port);
 
    /*
     * Save remote_host and remote_port in port structure (after this, they
@@ -4423,21 +4419,21 @@ BackendInitialize(Port *port)
    /*
     * Now that we have the user and database name, we can set the process
     * title for ps.  It's good to do this as early as possible in startup.
-    *
-    * For a walsender, the ps display is set in the following form:
-    *
-    * postgres: walsender <user> <host> <activity>
-    *
-    * To achieve that, we pass "walsender" as username and username as dbname
-    * to init_ps_display(). XXX: should add a new variant of
-    * init_ps_display() to avoid abusing the parameters like this.
     */
+   initStringInfo(&ps_data);
    if (am_walsender)
-       init_ps_display(pgstat_get_backend_desc(B_WAL_SENDER), port->user_name, remote_ps_data,
-                       update_process_title ? "authentication" : "");
-   else
-       init_ps_display(port->user_name, port->database_name, remote_ps_data,
-                       update_process_title ? "authentication" : "");
+       appendStringInfo(&ps_data, "%s ", pgstat_get_backend_desc(B_WAL_SENDER));
+   appendStringInfo(&ps_data, "%s ", port->user_name);
+   if (!am_walsender)
+       appendStringInfo(&ps_data, "%s ", port->database_name);
+   appendStringInfo(&ps_data, "%s", port->remote_host);
+   if (port->remote_port[0] != '\0')
+       appendStringInfo(&ps_data, "(%s)", port->remote_port);
+
+   init_ps_display(ps_data.data);
+   pfree(ps_data.data);
+
+   set_ps_display("initializing");
 
    /*
     * Disable the timeout, and prevent SIGTERM/SIGQUIT again.
index cf7b535e4e359ab855e94b8eeb99517c76d4d334..b394599236f004be1349a16b3023296164c5bab3 100644 (file)
@@ -179,7 +179,7 @@ SysLoggerMain(int argc, char *argv[])
 
    am_syslogger = true;
 
-   init_ps_display("logger", "", "", "");
+   init_ps_display("logger");
 
    /*
     * If we restarted, our stderr is already redirected into our own input
index f66cbc2428a47ae1939abc0a09aeadd06c16da9e..806d013108d4b7d8634759509cffe9f19c25e252 100644 (file)
@@ -828,7 +828,7 @@ SendBaseBackup(BaseBackupCmd *cmd)
 
        snprintf(activitymsg, sizeof(activitymsg), "sending backup \"%s\"",
                 opt.label);
-       set_ps_display(activitymsg, false);
+       set_ps_display(activitymsg);
    }
 
    perform_base_backup(&opt);
index c284103b54887391b4b1461d8c4b09463971696e..ffd5b31eb23e8608edfb3a1c89a2a6e39f012197 100644 (file)
@@ -209,7 +209,7 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
        memcpy(new_status, old_status, len);
        sprintf(new_status + len, " waiting for %X/%X",
                (uint32) (lsn >> 32), (uint32) lsn);
-       set_ps_display(new_status, false);
+       set_ps_display(new_status);
        new_status[len] = '\0'; /* truncate off " waiting ..." */
    }
 
@@ -311,7 +311,7 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
    if (new_status)
    {
        /* Reset ps display */
-       set_ps_display(new_status, false);
+       set_ps_display(new_status);
        pfree(new_status);
    }
 }
index ab59a86c62ef698e312f79d3e66897e5600bdf60..25e0333c9e1fc8d9d47b499d1b70b4521e99e40f 100644 (file)
@@ -666,8 +666,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
    walrcv->receiveStartTLI = 0;
    SpinLockRelease(&walrcv->mutex);
 
-   if (update_process_title)
-       set_ps_display("idle", false);
+   set_ps_display("idle");
 
    /*
     * nudge startup process to notice that we've stopped streaming and are
@@ -715,7 +714,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
        snprintf(activitymsg, sizeof(activitymsg), "restarting at %X/%X",
                 (uint32) (*startpoint >> 32),
                 (uint32) *startpoint);
-       set_ps_display(activitymsg, false);
+       set_ps_display(activitymsg);
    }
 }
 
@@ -1028,7 +1027,7 @@ XLogWalRcvFlush(bool dying)
            snprintf(activitymsg, sizeof(activitymsg), "streaming %X/%X",
                     (uint32) (LogstreamResult.Write >> 32),
                     (uint32) LogstreamResult.Write);
-           set_ps_display(activitymsg, false);
+           set_ps_display(activitymsg);
        }
 
        /* Also let the master know that we made some progress */
index 594a60e883fecfa579b6eeb780e9115a2334385c..3f74bc84939380443a2e7484d87cd8401605c3a4 100644 (file)
@@ -2769,7 +2769,7 @@ retry:
 
        snprintf(activitymsg, sizeof(activitymsg), "streaming %X/%X",
                 (uint32) (sentPtr >> 32), (uint32) sentPtr);
-       set_ps_display(activitymsg, false);
+       set_ps_display(activitymsg);
    }
 }
 
index c083b82d98316582a80121c9d06f2c06164c67dc..08f695a98053002d6dd13d73a9162258fa582769 100644 (file)
@@ -259,7 +259,7 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
                new_status = (char *) palloc(len + 8 + 1);
                memcpy(new_status, old_status, len);
                strcpy(new_status + len, " waiting");
-               set_ps_display(new_status, false);
+               set_ps_display(new_status);
                new_status[len] = '\0'; /* truncate off " waiting" */
            }
 
@@ -290,7 +290,7 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
    /* Reset ps display if we changed it */
    if (new_status)
    {
-       set_ps_display(new_status, false);
+       set_ps_display(new_status);
        pfree(new_status);
    }
 }
index 56dba09299d560b2c4ce639a553b2cf3ebc9a771..1df7b8e2ab7d780c53d7de2cbab24a0d6a4ef00d 100644 (file)
@@ -1737,7 +1737,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
        new_status = (char *) palloc(len + 8 + 1);
        memcpy(new_status, old_status, len);
        strcpy(new_status + len, " waiting");
-       set_ps_display(new_status, false);
+       set_ps_display(new_status);
        new_status[len] = '\0'; /* truncate off " waiting" */
    }
 
@@ -1789,7 +1789,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
        /* Report change to non-waiting status */
        if (update_process_title)
        {
-           set_ps_display(new_status, false);
+           set_ps_display(new_status);
            pfree(new_status);
        }
 
@@ -1803,7 +1803,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
    /* Report change to non-waiting status */
    if (update_process_title)
    {
-       set_ps_display(new_status, false);
+       set_ps_display(new_status);
        pfree(new_status);
    }
 
index 9dba3b0566a403fc23ecb9a01ead2273835bd605..00c77b66c74a415f10769556382b46c1fbfb53c7 100644 (file)
@@ -1081,7 +1081,7 @@ exec_simple_query(const char *query_string)
         */
        commandTag = CreateCommandTag(parsetree->stmt);
 
-       set_ps_display(GetCommandTagName(commandTag), false);
+       set_ps_display(GetCommandTagName(commandTag));
 
        BeginCommand(commandTag, dest);
 
@@ -1365,7 +1365,7 @@ exec_parse_message(const char *query_string,  /* string to execute */
 
    pgstat_report_activity(STATE_RUNNING, query_string);
 
-   set_ps_display("PARSE", false);
+   set_ps_display("PARSE");
 
    if (save_log_statement_stats)
        ResetUsage();
@@ -1656,7 +1656,7 @@ exec_bind_message(StringInfo input_message)
 
    pgstat_report_activity(STATE_RUNNING, psrc->query_string);
 
-   set_ps_display("BIND", false);
+   set_ps_display("BIND");
 
    if (save_log_statement_stats)
        ResetUsage();
@@ -2099,7 +2099,7 @@ exec_execute_message(const char *portal_name, long max_rows)
 
    pgstat_report_activity(STATE_RUNNING, sourceText);
 
-   set_ps_display(GetCommandTagName(portal->commandTag), false);
+   set_ps_display(GetCommandTagName(portal->commandTag));
 
    if (save_log_statement_stats)
        ResetUsage();
@@ -4175,7 +4175,7 @@ PostgresMain(int argc, char *argv[],
        {
            if (IsAbortedTransactionBlockState())
            {
-               set_ps_display("idle in transaction (aborted)", false);
+               set_ps_display("idle in transaction (aborted)");
                pgstat_report_activity(STATE_IDLEINTRANSACTION_ABORTED, NULL);
 
                /* Start the idle-in-transaction timer */
@@ -4188,7 +4188,7 @@ PostgresMain(int argc, char *argv[],
            }
            else if (IsTransactionOrTransactionBlock())
            {
-               set_ps_display("idle in transaction", false);
+               set_ps_display("idle in transaction");
                pgstat_report_activity(STATE_IDLEINTRANSACTION, NULL);
 
                /* Start the idle-in-transaction timer */
@@ -4215,7 +4215,7 @@ PostgresMain(int argc, char *argv[],
 
                pgstat_report_stat(false);
 
-               set_ps_display("idle", false);
+               set_ps_display("idle");
                pgstat_report_activity(STATE_IDLE, NULL);
            }
 
@@ -4365,7 +4365,7 @@ PostgresMain(int argc, char *argv[],
 
                /* Report query to various monitoring facilities. */
                pgstat_report_activity(STATE_FASTPATH, NULL);
-               set_ps_display("<FASTPATH>", false);
+               set_ps_display("<FASTPATH>");
 
                /* start an xact for this function invocation */
                start_xact_command();
index 8a47dcdcb18a6a841193830f4fe7d89e88ec014a..f4247ea70d557e4d482b068245b335fc0ef880fe 100644 (file)
@@ -236,6 +236,7 @@ PerformAuthentication(Port *port)
    /*
     * Now perform authentication exchange.
     */
+   set_ps_display("authentication");
    ClientAuthentication(port); /* might not return, if failure */
 
    /*
@@ -303,7 +304,7 @@ PerformAuthentication(Port *port)
        }
    }
 
-   set_ps_display("startup", false);
+   set_ps_display("startup");
 
    ClientAuthInProgress = false;   /* client_min_messages is active now */
 }
index ed23c840e955d7d88a61cde85ce9e8632ffd1122..8b160c0b405c7aca3d2aecb8e02abdd576c9c90b 100644 (file)
@@ -250,12 +250,11 @@ save_ps_display_args(int argc, char **argv)
  * values.  At this point, the original argv[] array may be overwritten.
  */
 void
-init_ps_display(const char *username, const char *dbname,
-               const char *host_info, const char *initial_str)
+init_ps_display(const char *fixed_part)
 {
-   Assert(username);
-   Assert(dbname);
-   Assert(host_info);
+   bool        save_update_process_title;
+
+   Assert(fixed_part);
 
 #ifndef PS_USE_NONE
    /* no ps display for stand-alone backend */
@@ -309,19 +308,25 @@ init_ps_display(const char *username, const char *dbname,
    if (*cluster_name == '\0')
    {
        snprintf(ps_buffer, ps_buffer_size,
-                PROGRAM_NAME_PREFIX "%s %s %s ",
-                username, dbname, host_info);
+                PROGRAM_NAME_PREFIX "%s ",
+                fixed_part);
    }
    else
    {
        snprintf(ps_buffer, ps_buffer_size,
-                PROGRAM_NAME_PREFIX "%s: %s %s %s ",
-                cluster_name, username, dbname, host_info);
+                PROGRAM_NAME_PREFIX "%s: %s ",
+                cluster_name, fixed_part);
    }
 
    ps_buffer_cur_len = ps_buffer_fixed_size = strlen(ps_buffer);
 
-   set_ps_display(initial_str, true);
+   /*
+    * On the first run, force the update.
+    */
+   save_update_process_title = update_process_title;
+   update_process_title = true;
+   set_ps_display("");
+   update_process_title = save_update_process_title;
 #endif                         /* not PS_USE_NONE */
 }
 
@@ -332,11 +337,11 @@ init_ps_display(const char *username, const char *dbname,
  * indication of what you're currently doing passed in the argument.
  */
 void
-set_ps_display(const char *activity, bool force)
+set_ps_display(const char *activity)
 {
 #ifndef PS_USE_NONE
-   /* update_process_title=off disables updates, unless force = true */
-   if (!force && !update_process_title)
+   /* update_process_title=off disables updates */
+   if (!update_process_title)
        return;
 
    /* no ps display for stand-alone backend */
index 23f1e59ffcd600de388187712660b5b0a3741bca..9f43e1fdf0aa1d2e53c321b8cebb139c20e4fd38 100644 (file)
@@ -16,10 +16,9 @@ extern bool update_process_title;
 
 extern char **save_ps_display_args(int argc, char **argv);
 
-extern void init_ps_display(const char *username, const char *dbname,
-                           const char *host_info, const char *initial_str);
+extern void init_ps_display(const char *fixed_part);
 
-extern void set_ps_display(const char *activity, bool force);
+extern void set_ps_display(const char *activity);
 
 extern const char *get_ps_display(int *displen);