Re: Patch to implement pg_current_logfile() function - Mailing list pgsql-hackers
From | Gilles Darold |
---|---|
Subject | Re: Patch to implement pg_current_logfile() function |
Date | |
Msg-id | 0731a353-8d2f-0f2f-fcd5-fde77114cb7e@dalibo.com Whole thread Raw |
In response to | Re: Patch to implement pg_current_logfile() function (Christoph Berg <myon@debian.org>) |
Responses |
Re: Patch to implement pg_current_logfile() function
(Christoph Berg <myon@debian.org>)
|
List | pgsql-hackers |
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
Attachment
pgsql-hackers by date: