| 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
| 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 |