Re: vacuum verbose detail logs are unclear; log at *start* of each stage

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: vacuum verbose detail logs are unclear; log at *start* of each stage
Date: 2020-01-26 05:36:29
Message-ID: 20200126053629.GO13621@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 22, 2020 at 02:34:57PM +0900, Michael Paquier wrote:
> From patch 0003:
> /*
> + * Indent multi-line DETAIL if being sent to client (verbose)
> + * We don't know if it's sent to the client (client_min_messages);
> + * Also, that affects output to the logfile, too; assume that it's more
> + * important to format messages requested by the client than to make
> + * verbose logs pretty when also sent to the logfile.
> + */
> + msgprefix = elevel==INFO ? "!\t" : "";
> Such stuff gets a -1 from me. This is not project-like, and you make
> the translation of those messages much harder than they should be.

I don't see why its harder to translate ? Do you mean because it changes the
strings by adding %s ?

- appendStringInfo(&sbuf, ngettext("%u page is entirely empty.\n",
- "%u pages are entirely empty.\n",
+ appendStringInfo(&sbuf, ngettext("%s%u page is entirely empty.\n",
+ "%s%u pages are entirely empty.\n",
...

I did raise two questions regarding translation:

I'm not sure why this one doesn't use get ngettext() ? Seems to have been
missed at a8d585c0.
|appendStringInfo(&buf, _("There were %.0f unused item identifiers.\n"),

Or why this one does use _/gettext() ? (580ddcec suggests that I'm missing
something, but I just experimented, and it really seems to do nothing, since
"%s" shouldn't be translated).
|appendStringInfo(&buf, _("%s."), pg_rusage_show(&ru0));

Also, I realized it's possible to write different strings to the log vs the
client (with and without a prefix) by calling errdetail_internal() and
errdetail_log().

Here's a version rebased on top of f942dfb9, and making use of errdetail_log.
I'm not sure if it address your concern about translation, but it doesn't
change strings.

I think it's not needed or desirable to change what's written to the logfile,
since CSV logs have a separate "detail" field, and text logs are indented. The
server log is unchanged:

> 2020-01-25 23:08:40.451 CST [13971] INFO: "t": removed 0, found 160 nonremovable row versions in 1 out of 888 pages
> 2020-01-25 23:08:40.451 CST [13971] DETAIL: 0 dead row versions cannot be removed yet, oldest xmin: 781
> There were 0 unused item identifiers.
> Skipped 0 pages due to buffer pins, 444 frozen pages.
> 0 pages are entirely empty.
> CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.01 s.

If VERBOSE, then the client log has ! prefixes, with the style borrowed from
ShowUsage:

> INFO: "t": removed 0, found 160 nonremovable row versions in 1 out of 888 pages
> DETAIL: ! 0 dead row versions cannot be removed yet, oldest xmin: 781
> ! There were 0 unused item identifiers.
> ! Skipped 0 pages due to buffer pins, 444 frozen pages.
> ! 0 pages are entirely empty.
> ! CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.01 s.

I mentioned before that maybe the client's messages with newlines should be
indented similarly to how the they're done in the text logfile. I looked,
that's append_with_tabs() in elog.c. So that's a different possible
implementation, which would apply to any message with newlines (or possibly
just DETAIL).

I'll also fork the allvisible/frozen/hintbits patches to a separate thread.

Thanks,
Justin

Attachment Content-Type Size
v3-0001-Remove-gettext-erronously-readded-at-580ddce.patch text/x-diff 957 bytes
v3-0002-vacuum-verbose-use-ngettext-everywhere-possible.patch text/x-diff 1.4 KB
v3-0003-vacuum-verbose-prefix-write-multi-line-output-to-.patch text/x-diff 4.8 KB
v3-0004-reduce-to-DEBUG-status-logged-from-vacuum_heap-in.patch text/x-diff 4.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Vik Fearing 2020-01-26 05:52:05 Re: Greatest Common Divisor
Previous Message Tom Lane 2020-01-26 00:49:28 Re: t/010_pg_basebackup.pl checksum verify fails with RELSEG_SIZE 1