On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> 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.
>
Thanks for pushing!
> 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.
It was intentionally "noisy" with dashes to make it impossible to
accidentally overlook dry-run mode in the logs. But if that's contrary
to the way Postgres does things I am fine if you want to remove the
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?
>
I took a quick look at your follow-up patches and below are some comments:
//////////
Patch 0001 - USECS_PER_SEC
//////////
+1 to use USECS_PER_SEC from timestamps.h
1.
-#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);
Maybe that static assert might be better placed adjacent to the
#define to enforce that comment about "divide evenly".
SUGGESTION
#define WAITS_PER_SEC 10
/* WAITS_PER_SEC should divide USECS_PER_SEC evenly */
StatiAssertDecl(USECS_PER_SEC % WAITS_PER_SEC == 0);
//////////
Patch 0002 -- boolean
//////////
LGTM.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
