Patch to implement pg_current_logfile() function

On Wed, Mar 9, 2016 at 12:32 PM, Gilles Darold <gilles.darold@dalibo.com> wrote:

I choose to allow the log collector to write his current log file name
into the lock file 'postmaster.pid'. This allow simple access to this
information through system commands, for example:

postgres@W230ST:~$ tail -n1 /usr/local/pgql-devel/data/postmaster.pid
pg_log/postgresql-2016-03-09_152908.log

Log filename is written at the 8th line position when log collection
is active and all other information have been written to lock file.

The function pg_current_logfile() use in SQL mode read the lock file
to report the information.

I don't know if there's any limitation on using postmaster.pid file to
do that but it seems to me a bit weird to log this information to an
other file. My first attempt was to use a dedicated file and save it
to global/pg_current_logfile or pg_stat_tmp/pg_current_logfile but I
think it is better to use the postmaster.pid file for that.

Gosh, why? Piggybacking this on a file written for a specific purpose
by a different process seems like making life very hard for yourself,
and almost certainly a recipe for bugs.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 9, 2016 at 12:32 PM, Gilles Darold <gilles.darold@dalibo.com> wrote:

I choose to allow the log collector to write his current log file name
into the lock file 'postmaster.pid'.

Gosh, why? Piggybacking this on a file written for a specific purpose
by a different process seems like making life very hard for yourself,
and almost certainly a recipe for bugs.

That's a *complete* nonstarter. postmaster.pid has to be written by the
postmaster process and nobody else.

It's a particularly bad choice for the syslogger, which will exist
fractionally longer than the postmaster, and thus might be trying to write
into the file after the postmaster has removed it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Le 10/03/2016 16:26, Tom Lane a �crit :

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 9, 2016 at 12:32 PM, Gilles Darold <gilles.darold@dalibo.com> wrote:

I choose to allow the log collector to write his current log file name
into the lock file 'postmaster.pid'.

Gosh, why? Piggybacking this on a file written for a specific purpose
by a different process seems like making life very hard for yourself,
and almost certainly a recipe for bugs.

That's a *complete* nonstarter. postmaster.pid has to be written by the
postmaster process and nobody else.

It's a particularly bad choice for the syslogger, which will exist
fractionally longer than the postmaster, and thus might be trying to write
into the file after the postmaster has removed it.

I was thinking that the lock file was removed after all and that
postmaster was waiting for all childs die before removing it, but ok I
know why this was not my first choice, this was a very bad idea :-)
Then, should I have to use an alternate file to store the information or
implement a bidirectional communication with the syslogger? The last
solution looks like really too much for this very simple feature. With
an alternate file what is the best place where it has to be written? All
places have their special use, the global/ directory?

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Gilles Darold <gilles.darold@dalibo.com> writes:

Then, should I have to use an alternate file to store the information or
implement a bidirectional communication with the syslogger?

I'd just define a new single-purpose file $PGDATA/log_file_name
or some such.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Mar 10, 2016 at 9:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Gilles Darold <gilles.darold@dalibo.com> writes:

Then, should I have to use an alternate file to store the information or
implement a bidirectional communication with the syslogger?

I'd just define a new single-purpose file $PGDATA/log_file_name
or some such.

Would it make sense to have it as a symlink instead?

--
Alex

Le 11/03/2016 10:49, Shulgin, Oleksandr a écrit :

On Thu, Mar 10, 2016 at 9:05 PM, Tom Lane <tgl@sss.pgh.pa.us
<mailto:tgl@sss.pgh.pa.us>> wrote:

Gilles Darold <gilles.darold@dalibo.com
<mailto:gilles.darold@dalibo.com>> writes:

Then, should I have to use an alternate file to store the

information or

implement a bidirectional communication with the syslogger?

I'd just define a new single-purpose file $PGDATA/log_file_name
or some such.

Would it make sense to have it as a symlink instead?

The only cons I see is that it can be more "difficult" with some
language to gather the real path, but do we really need it? There is
also little time where the symlink doesn't exist, this is when it needs
to be removed before being recreated to point to the new log file. If
your external script try to reopen the log file at this moment it will
complain that file doesn't exists and looping until the file exists is
probably a bad idea.

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

Gilles Darold <gilles.darold@dalibo.com> writes:

Le 11/03/2016 10:49, Shulgin, Oleksandr a écrit :

Would it make sense to have it as a symlink instead?

The only cons I see is that it can be more "difficult" with some
language to gather the real path, but do we really need it? There is
also little time where the symlink doesn't exist, this is when it needs
to be removed before being recreated to point to the new log file.

Yeah, a symlink is impossible to update atomically (on most platforms
anyway). Plain file containing the pathname seems better.

Another point is that we might not necessarily want *only* the pathname in
there. postmaster.pid has accreted more stuff over time, and this file
might too. I can see wanting the syslogger PID in there, for example,
so that onlookers can detect a totally stale file. (Not proposing this
right now, just pointing out that it's a conceivable future feature.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Le 11/03/2016 15:22, Tom Lane a �crit :

Gilles Darold <gilles.darold@dalibo.com> writes:

Le 11/03/2016 10:49, Shulgin, Oleksandr a écrit :

Would it make sense to have it as a symlink instead?

The only cons I see is that it can be more "difficult" with some
language to gather the real path, but do we really need it? There is
also little time where the symlink doesn't exist, this is when it needs
to be removed before being recreated to point to the new log file.

Yeah, a symlink is impossible to update atomically (on most platforms
anyway). Plain file containing the pathname seems better.

Another point is that we might not necessarily want *only* the pathname in
there. postmaster.pid has accreted more stuff over time, and this file
might too. I can see wanting the syslogger PID in there, for example,
so that onlookers can detect a totally stale file. (Not proposing this
right now, just pointing out that it's a conceivable future feature.)

regards, tom lane

Here is the patch rewritten to use alternate file
$PGDATA/pg_log_filename to store the current log filename used by
syslogger. All examples used in the first mail of this thread work the
exact same way. If there's no other remarks, I will add the patch to the
next commit fest.

Here are some additional examples with this feature.

To obtain the filling percentage of the log file when log_rotation_size
is used:

postgres=# SELECT pg_current_logfile(), (select setting::int*1000 from
pg_settings where name='log_rotation_size'), a.size size,
((a.size*100)/(select setting::int*1000 from pg_settings where
name='log_rotation_size')) percent_used FROM
pg_stat_file(pg_current_logfile())
a(size,access,modification,change,creation,isdir);

-[ RECORD 1 ]------+----------------------------------------
pg_current_logfile | pg_log/postgresql-2016-03-11_160817.log
log_rotation_size | 10240000
size | 1246000
percent_used | 12

This can help to know if the file is near to be rotated. Or if you use
time based rotation:

postgres=# select pg_current_logfile(), (select setting::int*60 from
pg_settings where name='log_rotation_age')
log_rotation_age,a.access,a.modification, (((extract(epoch from
a.modification) - extract(epoch from a.access)) * 100) / (select
setting::int*60 from pg_settings where name='log_rotation_age'))
percent_used FROM pg_stat_file(pg_current_logfile())
a(size,access,modification,change,creation,isdir);
-[ RECORD 1 ]------+----------------------------------------
pg_current_logfile | pg_log/postgresql-2016-03-11_162143.log
log_rotation_age | 3600
access | 2016-03-11 16:21:43+01
modification | 2016-03-11 16:33:12+01
percent_used | 19.1388888888889

Best regards,

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

On Sat, Mar 12, 2016 at 12:46 AM, Gilles Darold
<gilles.darold@dalibo.com> wrote:

Here is the patch rewritten to use alternate file
$PGDATA/pg_log_filename to store the current log filename used by
syslogger. All examples used in the first mail of this thread work the
exact same way. If there's no other remarks, I will add the patch to the
next commit fest.

Please be sure to register this patch to the next CF:
https://commitfest.postgresql.org/10/
Things are hot enough with 9.6, so this will only be considered in 9.7.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Le 22/03/2016 14:44, Michael Paquier a écrit :

On Sat, Mar 12, 2016 at 12:46 AM, Gilles Darold
<gilles.darold@dalibo.com> wrote:

Here is the patch rewritten to use alternate file
$PGDATA/pg_log_filename to store the current log filename used by
syslogger. All examples used in the first mail of this thread work the
exact same way. If there's no other remarks, I will add the patch to the
next commit fest.

Please be sure to register this patch to the next CF:
https://commitfest.postgresql.org/10/
Things are hot enough with 9.6, so this will only be considered in 9.7.

Thanks for the reminder, here is the v3 of the patch after a deeper
review and testing. It is now registered to the next commit fest under
the System Administration topic.

Fixes in this patch are:

- Output file have been renamed as PGDATA/pg_log_file

- Log level of the warning when logging collector is not active has been
changed to NOTICE

postgres=# select pg_current_logfile();
NOTICE: current log can not be reported, log collection is not active
pg_current_logfile
--------------------

(1 row)

- Log level for file access errors in function
store_current_log_filename() of file src/backend/postmaster/syslogger.c
has been set to WARNING, using ERROR level forced the backend to stop
with a FATAL error.

- Add information about file PGDATA/pg_log_file in storage file layout
of doc/src/sgml/storage.sgml

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

Hi Gilles,

On Wed, 23 Mar 2016 23:22:26 +0100
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thanks for the reminder, here is the v3 of the patch after a deeper
review and testing. It is now registered to the next commit fest under
the System Administration topic.

I am going to try reviewing your patch. I don't feel entirely
confident, but should be able to provide at least some help.

I've not yet even tried building it, but the the first thing I notice
is that you're going to need to use pgrename() of src/port/dirmod.c
in order to get an atomic update of the pg_log_file file.

I believe this is the right approach here. Any other program
will always see a fully-formed file content.

The typical way this works is: you make a new file with new
content, then rename the new file to the old file name.
This makes the new file name go away and the old file
content go away and, effectively, replaces
the content of your file with new content.

You'll want to look at other places where pg uses pgrename()
to see what sort of name you should give to the new file.
If it was me, I'd just stick a dot in front, calling it
".pg_log_file" but we want to be consistent with existing
practice.

I'd imagine that you'd create the new file in the same directly
as pg_log_file, that being the usual way to ensure that both
files are in the same file system (a requirement). But when
you're looking at other uses of pgrename() in the existing code
it wouldn't hurt to see check what that code is doing regards
placement of the new file in the file system.

If you have any guidance/scripts/whatever which support
testing your patch please send it my way. Thanks.

Regards,

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, 6 Apr 2016 22:26:13 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 23 Mar 2016 23:22:26 +0100
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thanks for the reminder, here is the v3 of the patch after a deeper
review and testing. It is now registered to the next commit fest
under the System Administration topic.

I've not yet even tried building it,

Ok. I've built it (but not tested it). Some comments.

The docs don't build. You are missing a "<row"> at line
15197 of func.sgml. (Patched against current HEAD.)

Is there any rationale for the placement of your function
in that documentation table? I don't see any organizing principle
to the table so am wondering where it might best fit.
(http://www.postgresql.org/docs/devel/static/functions-info.html)

Perhaps you want to write?:

<para>
<function>pg_current_logfile</function> returns the name of the
current log file used by the logging collector, as
<type>text</type>. Log collection must be active or the
return value is undefined.
</para>

(Removed "a" before "text", and added "or..." to the end.)

Unless you want to define the return value to be NULL when
log collection is not active. This might be cleaner.
I say this because I'm tempted to add "This can be tested
for with current_setting('logging_collector')='on'." to
the end of the paragraph. But adding such text means
that the "logging_collector" setting is documented in multiple
places, in some sense, and such redundancy is best
avoided when possible.

I don't see a problem with defining the return value to be
NULL -- so long as it's defined to be NULL when there is
no current log file. This would be different from defining
it to be NULL when log collection is off. Although not
very different it should be clear that using pg_currrent_logfile()
to test whether log collection is on is not a good practice.
Perhaps some text like?:

<para>
<function>pg_current_logfile</function> returns the name of the
current log file used by the logging collector, as
<type>text</type>. <literal>NULL</literal> is returned
and a <literal>NOTICE</literal> raised when no log file exists.
</para>

(I'm going to have to think some more about the raising of the
notice, and of the other error handling in your code.
I've not paid it any attention and error handling is always
problematic.)

Regards,

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, 6 Apr 2016 23:37:09 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 6 Apr 2016 22:26:13 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 23 Mar 2016 23:22:26 +0100
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thanks for the reminder, here is the v3 of the patch after a
deeper review and testing. It is now registered to the next
commit fest under the System Administration topic.

Ok. I've built it (but not tested it). Some comments.

The logic in src/backend/postmaster/syslogger.c
looks good to me. (The log file is created before you
create the pg_curr_log file, so the worst thing to happen
is that the user gets the old log file, never a non-existent
file.)

In two places in src/backend/postmaster/syslogger.c,
before you call store_current_log_filename()
you have a comment that's more than 80 characters
long. Please fix. (But see below for more.)
(http://www.postgresql.org/docs/devel/static/source-format.html)

Given the name store_current_log_filename() the code
comment is not particularly useful. You might
consider removing the comment. You might also
consider changing store_currrent_log_filename()
to something like write_pg_log_file(). A little
shorter, and maybe more descriptive. Maybe enough
so that you can get rid of the comment.

(Are there other functions that create similar files?
Maybe there is a naming convention you can follow?)

(I like comments, but the pg coding style uses fewer
of them than I might use. Hence my notes above.)

Also, your patch seems to be using spaces, not tabs.
You want tabs.
See the formatting url above. (I forget whether
the docs use spaces or tabs. Check the code.)

Other thoughts:

You're going to have to do something for MS Windows
EOL conventions like in logfile_open() of
src/backend/postmaster/syslogger. You can't just
use a "\n".

The initial pg_log_file file is created by the postmaster.
Subsequent work is done by a logging subprocess.
Are there any permission implications?

src/backend/postmaster/syslogger.c expects to see
fopen() fail with ENFILE and EMFILE. What will you
do if you get these? Can you do the same thing
that the log rotation code does and try to update
pg_log_file the next time something logs? You'd
have to set a flag somewhere and test (in the regular
logging code) since presumably
the next time something is logged the log rotation
code (where all your code is) would no longer be
executed. This would leave the user with a "stale"
pg_log_file, but I'm not sure what else to do.

Regards the ereport() log level for when you can't
create pg_log_file at all, WARNING seems a bit severe
since LOG is used when the log stops rotating for some
reason. But there does not seem to be anything else that
fits so WARNING is ok with me. (See: include/utils/elog.h)

(I'd use LOG if you have to defer the updating of
pg_log_file. But logging at all here could be problematic.
You wouldn't want to trigger more even more logging
and some sort of tight little loop. It might be best
_not_ to log in this case.)

Have you given any thought as to when logfile_rotate()
is called? Since logfile_rotate() itself logs with ereport(),
it would _seem_ safe to ereport() from within your
store_current_log_filename(), called from within
logfile_rotate(). All the same, it wouldn't hurt to be
sure that calling ereport() from within your code
can't trigger another invocation of logfile_rotate()
leading to recursive awfulness.

The indentation of the ereport(), in the part that
continues over multiple lines, in store_current_log_filename()
seems too much. I think the continued lines should line up with
the "W" in WARNING.

This is what I see at the moment. I'll wait for replies
before looking further and writing more.

Regards,

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, 7 Apr 2016 01:13:51 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 6 Apr 2016 23:37:09 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 6 Apr 2016 22:26:13 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 23 Mar 2016 23:22:26 +0100
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thanks for the reminder, here is the v3 of the patch after a
deeper review and testing. It is now registered to the next
commit fest under the System Administration topic.

This is what I see at the moment. I'll wait for replies
before looking further and writing more.

Er, one more thing. Isn't it true that in logfile_rotate()
you only need to call store_current_log_filename() when
logfile_open() is called with a "w" mode, and never need to
call it when logfile_open() is called with an "a" mode?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Le 07/04/2016 08:30, Karl O. Pinc a �crit :

On Thu, 7 Apr 2016 01:13:51 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 6 Apr 2016 23:37:09 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 6 Apr 2016 22:26:13 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Wed, 23 Mar 2016 23:22:26 +0100
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thanks for the reminder, here is the v3 of the patch after a
deeper review and testing. It is now registered to the next
commit fest under the System Administration topic.

This is what I see at the moment. I'll wait for replies
before looking further and writing more.

Er, one more thing. Isn't it true that in logfile_rotate()
you only need to call store_current_log_filename() when
logfile_open() is called with a "w" mode, and never need to
call it when logfile_open() is called with an "a" mode?

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

Hi Karl,

Thank you very much for the patch review and please apologies this too
long response delay. I was traveling since end of April and totally
forgotten this patch. I have applied all your useful feedbacks on the
patch and attached a new one (v4) to this email :

- Fix the missing <row> in doc/src/sgml/func.sgml
- Rewrite pg_current_logfile descritption as suggested
- Remove comment in src/backend/postmaster/syslogger.c
- Use pgrename to first write the filename to a temporary file
before overriding the last log file.
- Rename store_current_log_filename() into logfile_writename() -
logfile_getname is used to retrieve the filename.
- Use logfile_open() to enable CRLF line endings on Windows
- Change log level for when it can't create pg_log_file, from
WARNING to LOG
- Remove NOTICE message when the syslogger is not enabled, the
function return null.

On other questions:

"src/backend/postmaster/syslogger.c expects to see fopen() fail with

ENFILE and EMFILE. What will you do if you get these?"

- Nothing, if the problem occurs during the log rotate call, then
log rotation file is disabled so logfile_writename() will not be called.
Case where the problem occurs between the rotation call and the
logfile_writename() call is possible but I don't think that it will be
useful to try again. In this last case log filename will be updated
during next rotation.

"Have you given any thought as to when logfile_rotate() is called?

Since logfile_rotate() itself logs with ereport(), it would _seem_ safe
to ereport() from within your store_current_log_filename(), called from
within logfile_rotate()."

- Other part of logfile_rotate() use ereport() so I guess it is safe
to use it.

"The indentation of the ereport(), in the part that continues over

multiple lines"

- This was my first thought but seeing what is done in the other
call to ereport() I think I have done it the right way.

"Isn't it true that in logfile_rotate() you only need to call

store_current_log_filename() when logfile_open() is called with a "w"
mode, and never need to call it when logfile_open() is called with an
"a" mode?"

- No, it is false, append mode is used when logfile_rotate used an
existing file but the filename still change.

Best regards,

--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

On Tue, 28 Jun 2016 11:06:24 +0200
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thank you very much for the patch review and please apologies this too
long response delay. I was traveling since end of April and totally
forgotten this patch. I have applied all your useful feedbacks on the
patch and attached a new one (v4) to this email :

Hi Gilles,

My schedule is really full at the moment. I'll get to this
when I have a bit of time. If you get anxious please write
and I'll see about making faster progress.

Regards,

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Jul 2, 2016 at 8:56 AM, Karl O. Pinc <kop@meme.com> wrote:

On Tue, 28 Jun 2016 11:06:24 +0200
Gilles Darold <gilles.darold@dalibo.com> wrote:

Thank you very much for the patch review and please apologies this too
long response delay. I was traveling since end of April and totally
forgotten this patch. I have applied all your useful feedbacks on the
patch and attached a new one (v4) to this email :

My schedule is really full at the moment. I'll get to this
when I have a bit of time. If you get anxious please write
and I'll see about making faster progress.

Moved to next CF, the patch still applies. Karl, you have registered
to review this patch a couple of months back but nothing happened. I
have removed your name for now. If you have time, don't hesitate to
come back to it.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, 3 Oct 2016 13:35:16 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

Moved to next CF, the patch still applies. Karl, you have registered
to review this patch a couple of months back but nothing happened. I
have removed your name for now. If you have time, don't hesitate to
come back to it.

Right. Hope to get back to it soon. Won't register until I'm ready
to look at it.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Gilles,

I've just tried v4 of the patch. The OID you picked for
pg_current_logfile doesn't work anymore, but after increasing it
randomly by 10000, it compiles. I like the added functionality,
especially that "select pg_read_file(pg_current_logfile());" just
works.

What bugs me is the new file "pg_log_file" in PGDATA. It clutters the
directory listing. I wouldn't know where else to put it, but you might
want to cross-check that with the thread that is trying to reshuffle
the directory layout to make it easier to exclude files from backups.
(Should this file be part of backups?)

It's probably correct to leave the file around on shutdown (given it's
still a correct pointer). But there might be a case for removing it on
startup if logging_collector isn't active anymore.

Also, pg_log_file is tab-completion-unfriendly, it conflicts with
pg_log/. Maybe name it current_logfile?

Another thing that might possibly be improved is csv logging:

# select pg_read_file(pg_current_logfile());
pg_read_file
───────────────────────────────────────────────────────────────
LOG: ending log output to stderr ↵
HINT: Future log output will go to log destination "csvlog".↵

-rw------- 1 cbe staff 1011 Okt 3 15:06 postgresql-2016-10-03_150602.csv
-rw------- 1 cbe staff 96 Okt 3 15:06 postgresql-2016-10-03_150602.log

... though it's unclear what to do if both stderr and csvlog are
selected.

Possibly NULL should be returned if only "syslog" is selected.
(Maybe remove pg_log_file once 'HINT: Future log output will go to
log destination "syslog".' is logged?)

Christoph

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers