pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Álvaro Herrera <alvherre@kurilemu.de>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>
Cc: Euler Taveira <euler@eulerto.com>
Cc: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_createsubscriber --dry-run logging concerns
Date: Fri, 31 Oct 2025 19:02:32 +0100
Message-ID: <202510311750.mxiykx3tp4mx@alvherre.pgsql> (raw )
In-Reply-To: <CAHut+Puhm2GRazE6zHvYUckQL3MqqSPrAwovuC0xS6bWO2w3_g@mail.gmail.com >
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)
Attachments:
0001-pg_ctl-use-USECS_PER_SEC-from-datatype-timestamp.h-i.patch (text/x-diff, 3.1K) download
From 6ca39215858f0bfdc2ef4676f4f4baf7fc0176c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
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
0002-Remove-WaitPMResult-enum-in-pg_createsubscriber.patch (text/x-diff, 1.8K) download
From ef79cc503adc6b2154a068b3298e03b494cd46a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
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
view thread (27+ messages) latest in thread
Message-ID: <202510311750.mxiykx3tp4mx@alvherre.pgsql>
Permalink: ../202510311750.mxiykx3tp4mx@alvherre.pgsql/
Also on: postgresql.org/message-id/202510311750.mxiykx3tp4mx@alvherre.pgsql
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de, smithpb2250@gmail.com, li.evan.chao@gmail.com, euler@eulerto.com, kuroda.hayato@fujitsu.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_createsubscriber --dry-run logging concerns
In-Reply-To: <202510311750.mxiykx3tp4mx@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox