Re: Patch to implement pg_current_logfile() function

Le 03/10/2016 à 16:09, Christoph Berg a écrit :
> 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.

I've changed the OID and some other things in this v5 of the patch, see
bellow.

> 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.

The file has been renamed into current_logfile and is now removed at
startup if logging_collector is not active. The file can be excluded
from a backup but otherwise if it is restored it will be removed or
overridden at startup. Perhaps the file can give a useful information in
a backup to know the last log file active at backup time, but not sure
it has any interest.

I'm not sure which thread is talking about reshuffling the directory
layout, please give me a pointer if this is not the thread talking about
renaming of pg_xlog and pg_clog. In the future if we have a directory to
store files that must be excluded from backup or status files, it will
be easy to move this file here. I will follow such a change.

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

> 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?)

I've totally missed that we can have log_destination set to stderr and
csvlog at the same time, so pg_current_logfile() might return two
filenames in this case. I've changed the function to return a setof
record to report the last stderr or csv log file or both. One another
major change is that the current log filename list is also updated after
a configuration reload and not just after a startup or a log rotation.
So in the case of you are switching from stderr to csvlog or both you
will see immediately the change in current_logfile instead of waiting
for the next log rotation.

* log_destination set to csvlog only:

postgres=# select * from pg_current_logfile();
pg_current_logfile
---------------------------------------
pg_log/postgresql-2016-10-07_1646.csv
(1 row)

* log_destination set to stderr only:

postgres=# select pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)

postgres=# select * from pg_current_logfile();
pg_current_logfile
---------------------------------------
pg_log/postgresql-2016-10-07_1647.log
(1 row)

* log_destination set to both stderr,csvlog:

postgres=# select pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)

postgres=# select * from pg_current_logfile();
pg_current_logfile
---------------------------------------
pg_log/postgresql-2016-10-07_1648.log
pg_log/postgresql-2016-10-07_1648.csv
(2 rows)

When logging_collector is disabled, this function return null.

As the return type has changed to a setof, the query to read the file
need to be change too:

postgres=# SELECT pg_read_file(log) FROM pg_current_logfile() b(log);

pg_read_file
--------------------------------------------------------------------------------------------------------------------------------
LOG: duration: 0.182 ms statement: select pg_read_file(log) from
pg_current_logfile() file(log);+

(1 row)

I can change the return type to a single text[] if that's looks better.

Thanks

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