On 2025-Oct-09, Peter Smith wrote: > Please see the v3 patches.
Okay, I have pushed 0002 to 18 and master. I wanted to backpatch to 17 but there are too many conflicts there. I'm not opposed to 0001 (to master only), but I think the lines of dashes are a little excessively noisy. Are there other opinions on that? Note that vacuumlo also has it, with no dashes. I'll mark the commitfest item for version 19. While looking this over, I noticed that we define USEC_PER_SEC, but why? We already have USECS_PER_SEC, so let's use that. And WaitPMResult seems an overgrown boolean, so how about we remove it? Patch for these things attached. Thoughts? -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ Thou shalt study thy libraries and strive not to reinvent them without cause, that thy code may be short and readable and thy days pleasant and productive. (7th Commandment for C Programmers)
>From 6ca39215858f0bfdc2ef4676f4f4baf7fc0176c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Fri, 31 Oct 2025 16:51:22 +0100 Subject: [PATCH 1/2] pg_ctl: use USECS_PER_SEC from datatype/timestamp.h instead of reinventing the wheel --- src/bin/pg_basebackup/pg_createsubscriber.c | 5 ++--- src/bin/pg_ctl/pg_ctl.c | 12 ++++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index f59c293d875..52e7c9212a2 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -23,6 +23,7 @@ #include "common/logging.h" #include "common/pg_prng.h" #include "common/restricted_token.h" +#include "datatype/timestamp.h" #include "fe_utils/recovery_gen.h" #include "fe_utils/simple_list.h" #include "fe_utils/string_utils.h" @@ -129,7 +130,6 @@ static void drop_existing_subscription(PGconn *conn, const char *subname, static void get_publisher_databases(struct CreateSubscriberOptions *opt, bool dbnamespecified); -#define USEC_PER_SEC 1000000 #define WAIT_INTERVAL 1 /* 1 second */ static const char *progname; @@ -1610,8 +1610,7 @@ wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions } /* Keep waiting */ - pg_usleep(WAIT_INTERVAL * USEC_PER_SEC); - + pg_usleep(WAIT_INTERVAL * USECS_PER_SEC); timer += WAIT_INTERVAL; } diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 8a405ff122c..b270c0c0d82 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -26,6 +26,7 @@ #include "common/file_perm.h" #include "common/logging.h" #include "common/string.h" +#include "datatype/timestamp.h" #include "getopt_long.h" #include "utils/pidfile.h" @@ -68,9 +69,7 @@ typedef enum #define DEFAULT_WAIT 60 -#define USEC_PER_SEC 1000000 - -#define WAITS_PER_SEC 10 /* should divide USEC_PER_SEC evenly */ +#define WAITS_PER_SEC 10 /* should divide USECS_PER_SEC evenly */ static bool do_wait = true; static int wait_seconds = DEFAULT_WAIT; @@ -594,6 +593,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint) { int i; + static_assert(USECS_PER_SEC % WAITS_PER_SEC == 0); for (i = 0; i < wait_seconds * WAITS_PER_SEC; i++) { char **optlines; @@ -699,7 +699,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint) print_msg("."); } - pg_usleep(USEC_PER_SEC / WAITS_PER_SEC); + pg_usleep(USECS_PER_SEC / WAITS_PER_SEC); } /* out of patience; report that postmaster is still starting up */ @@ -738,7 +738,7 @@ wait_for_postmaster_stop(void) if (cnt % WAITS_PER_SEC == 0) print_msg("."); - pg_usleep(USEC_PER_SEC / WAITS_PER_SEC); + pg_usleep(USECS_PER_SEC / WAITS_PER_SEC); } return false; /* timeout reached */ } @@ -771,7 +771,7 @@ wait_for_postmaster_promote(void) if (cnt % WAITS_PER_SEC == 0) print_msg("."); - pg_usleep(USEC_PER_SEC / WAITS_PER_SEC); + pg_usleep(USECS_PER_SEC / WAITS_PER_SEC); } return false; /* timeout reached */ } -- 2.47.3
>From ef79cc503adc6b2154a068b3298e03b494cd46a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Fri, 31 Oct 2025 17:05:26 +0100 Subject: [PATCH 2/2] Remove WaitPMResult enum in pg_createsubscriber This is a simple bool --- src/bin/pg_basebackup/pg_createsubscriber.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index 52e7c9212a2..c1120d3643e 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -156,12 +156,6 @@ static char *subscriber_dir = NULL; static bool recovery_ended = false; static bool standby_running = false; -enum WaitPMResult -{ - POSTMASTER_READY, - POSTMASTER_STILL_STARTING -}; - /* * Cleanup objects that were created by pg_createsubscriber if there is an @@ -1584,7 +1578,7 @@ static void wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions *opt) { PGconn *conn; - int status = POSTMASTER_STILL_STARTING; + bool ready = false; int timer = 0; pg_log_info("waiting for the target server to reach the consistent state"); @@ -1596,7 +1590,7 @@ wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions /* Did the recovery process finish? We're done if so. */ if (dry_run || !server_is_in_recovery(conn)) { - status = POSTMASTER_READY; + ready = true; recovery_ended = true; break; } @@ -1616,7 +1610,7 @@ wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions disconnect_database(conn, false); - if (status == POSTMASTER_STILL_STARTING) + if (!ready) pg_fatal("server did not end recovery"); pg_log_info("target server reached the consistent state"); -- 2.47.3
