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

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