Re: Patch to implement pg_current_logfile() function

On Sat, 29 Oct 2016 22:00:08 +0200
Gilles Darold <gilles(dot)darold(at)dalibo(dot)com> wrote:

> The attached v10 of the current_logfiles patch include your last
> changes on documentation but not the patch on v9 about the
> user-supplied GUC value. I think the v10 path is ready for committers
> and that the additional patch to add src/include/utils/guc_values.h
> to define user GUC values is something that need to be taken outside
> this one. Imo, thoses GUC values (stderr, csvlog) are not expected to
> change so often to require a global definition, but why not, if
> committers think this must be done I can add it to a v11 patch.

I agree that the guc_values.h patches should be submitted separately
to the committers, when your patch is submitted. Creating symbols
for these values is a matter of coding style they should resolve.
(IMO it's not whether the values will change, it's whether
someone reading the code can read the letters "stdout" and know
to what they refer and where to find related usage elsewhere in
the code.)

I'll keep up the guc_values.h patches and have them ready for
submission along with your patch.

I don't think your patch is quite ready for submission to
the committers.

Attached is a patch to be applied on top of your v10 patch
which does basic fixup to logfile_writename().

I have some questions about logfile_writename():

Why does the logfile_open() call fail silently?
Why not use ereport() here? (With a WARNING level.)

Why do the ereport() invocations all have a LOG level?
You're not recovering and the errors are unexpected
so I'd think WARNING would be the right level.
(I previously, IIRC, suggested LOG level -- only if
you are retrying and recovering from an error.)

Have you given any thought to my proposal to change
CURRENT_LOG_FILENAME to LOG_METAINFO_FILE?

I'm not sure I've looked at every bit of your patch
yet. I won't have much time to do more real work
until after Tuesday morning.

Regards,

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein