Re: pg_createsubscriber --dry-run logging concerns

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Álvaro Herrera <alvherre(at)kurilemu(dot)de>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_createsubscriber --dry-run logging concerns
Date: 2025-11-12 02:01:24
Message-ID: CAHut+PssSBRmrWLuBQ=gu1PdnefAB29CeC3nmYUDdF=eWFtfUg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 11, 2025 at 8:17 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2025-Nov-10, Peter Smith wrote:
>
> > Hi Alvaro,
> >
> > Here is patch v4-0001 modified as requested:
> > - dashes are removed
> > - the message is a single string
>
> Okay, thanks. I split the strings in two lines, as we customarily do
> when they contain embedded newlines. I also noticed pg_resetwal uses
> stdout rather than stderr and set out to change it, because I don't
> think it's sensible to have one program behave one way (print to stdout)
> when all others behave in another (to stderr). I wrote a commit message
> and was about ready to push.
>
> However, I then found out that the reason you used stdout instead of
> stderr in pg_resetwal is that with the latter, tests fail all over the
> place because of pg_resetwal -n being used for pg_upgrade internally via
> popen(), and making it write to stderr results in confusing pg_upgrade
> output as well as test failures. A very simple fix for this problem

Right.

> would be, of course, to add " 2>/dev/null" to the popen call, but that
> is not only cheating, it is also dangerous: if pg_resetwal ever finds
> reason to complain, we won't get very good information because of that
> redirection.
>
> (I also don't think this line belongs in stdout, in case you're thinking
> of changing it in the other direction for all other programs.)
>
> Maybe we should add a -q,--silent mode that suppresses the "Running in
> dry-run mode" line. I do wonder if this is getting too far into the
> weeds for such a small thing. I won't blame you if you want to just
> drop this whole idea, but I also won't stop you if you want to introduce
> --silent.
>

It is tempting to implement a "--silent" mode, but if I did that, I
would then feel obliged to document and test it. I don't want to go
further down this rabbit hole for what was originally supposed to be
trivial logging.

So, I am calling it quits for this 0001 patch.

Perhaps it's still of some use to push changes for everything except
the pg_resetwal? Or if you prefer to just abandon the whole patch,
that is OK too. Thanks for trying.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2025-11-12 02:05:02 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart
Previous Message Fujii Masao 2025-11-12 01:56:39 Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions