Horton

Thread

Commits

  1. Improve common/logging.c's support for multiple verbosity levels.

  2. Unified logging system for command-line programs

  1. "Unified logging system" breaks access to pg_dump debug outputs

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-14T20:50:25Z

    pg_dump et al have some low-level debug log messages that commit
cc8d41511 converted to pg_log_debug() calls, replacing the previous
one-off logging verbosity system that was there.  However, these
calls are dead code as things stand, because there is no way to set
__pg_log_level high enough to get them to print.

I propose the attached minimal patch to restore the previous
functionality.

Alternatively, we might consider inventing an additional logging.c
function pg_logging_increase_level() with the obvious semantics, and
make the various programs just call that when they see a -v switch.
That would be a slightly bigger patch, but it would more easily support
programs with a range of useful verbosities, so maybe that's a better
idea.

In a quick look around, I could not find any other unreachable
pg_log_debug calls.

(Note: it seems possible that the theoretical multiple verbosity
levels in pg_dump were already broken before cc8d41511, because
right offhand I do not see any code that that removed that would
have allowed invoking the higher levels either.  Nonetheless, there
is no point in carrying dead code --- and these messages *are*
of some interest.  I discovered this problem while trying to
debug parallel pg_restore behavior just now, and wondering
why "-v -v" didn't produce the messages I saw in the source code.)

			regards, tom lane

  2. Re: "Unified logging system" breaks access to pg_dump debug outputs

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-15T16:59:13Z

    I wrote:
> Alternatively, we might consider inventing an additional logging.c
> function pg_logging_increase_level() with the obvious semantics, and
> make the various programs just call that when they see a -v switch.
> That would be a slightly bigger patch, but it would more easily support
> programs with a range of useful verbosities, so maybe that's a better
> idea.

After further thought, I concluded that's a clearly superior solution,
so 0001 attached does it like that.  After noting that the enum values
are in the opposite direction from how I thought they went, I realized
that "increase_level" might be a bit ambiguous, so I now propose to
call it pg_logging_increase_verbosity.

> (Note: it seems possible that the theoretical multiple verbosity
> levels in pg_dump were already broken before cc8d41511, because
> right offhand I do not see any code that that removed that would
> have allowed invoking the higher levels either.

Closer inspection says this was almost certainly true, because
I discovered that pg_dump -v -v crashes if you don't specify
an output filename :-(.  So this has probably been unreachable
at least since we went over to using our own snprintf always;
before that, depending on platform, it might've been fine.
So we also need 0002 attached to fix that.

			regards, tom lane

  3. Re: "Unified logging system" breaks access to pg_dump debug outputs

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-09-15T21:38:15Z

    On 2020-Sep-15, Tom Lane wrote:

> I wrote:
> > Alternatively, we might consider inventing an additional logging.c
> > function pg_logging_increase_level() with the obvious semantics, and
> > make the various programs just call that when they see a -v switch.
> > That would be a slightly bigger patch, but it would more easily support
> > programs with a range of useful verbosities, so maybe that's a better
> > idea.
> 
> After further thought, I concluded that's a clearly superior solution,
> so 0001 attached does it like that.  After noting that the enum values
> are in the opposite direction from how I thought they went, I realized
> that "increase_level" might be a bit ambiguous, so I now propose to
> call it pg_logging_increase_verbosity.

I like this better too ... I've wished for extra-verbose messages from
pg_dump in the past, and this now allows me to add something should I
want them again.

> > (Note: it seems possible that the theoretical multiple verbosity
> > levels in pg_dump were already broken before cc8d41511, because
> > right offhand I do not see any code that that removed that would
> > have allowed invoking the higher levels either.
> 
> Closer inspection says this was almost certainly true, because
> I discovered that pg_dump -v -v crashes if you don't specify
> an output filename :-(.  So this has probably been unreachable
> at least since we went over to using our own snprintf always;
> before that, depending on platform, it might've been fine.
> So we also need 0002 attached to fix that.

Ugh.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: "Unified logging system" breaks access to pg_dump debug outputs

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-17T16:53:38Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Sep-15, Tom Lane wrote:
>> After further thought, I concluded that's a clearly superior solution,
>> so 0001 attached does it like that.  After noting that the enum values
>> are in the opposite direction from how I thought they went, I realized
>> that "increase_level" might be a bit ambiguous, so I now propose to
>> call it pg_logging_increase_verbosity.

> I like this better too ... I've wished for extra-verbose messages from
> pg_dump in the past, and this now allows me to add something should I
> want them again.

Pushed, thanks for reviewing.

			regards, tom lane