Re: Close stdout and stderr in syslogger

From: Святослав Ермилин <munakoiso(at)yandex-team(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Дмитрий Сарафанников <dsarafan(at)yandex-team(dot)ru>, Андрей Бородин <x4mmm(at)yandex-team(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Close stdout and stderr in syslogger
Date: 2019-09-14 07:27:38
Message-ID: 961051568446048@sas1-fc7737ec834f.qloud-c.yandex.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Tom,

Thank you for quick reply.

> I'm quite certain that the current behavior is intentional, if only
> because closing the syslogger's stderr would make it impossible to
> debug problems inside the syslogger.
Developer who debugs syslogger, probably, can remove fclose() calls.
Maybe we can have a switch for this?
As far as I understand there is no stdout printing in syslogger?

> Why is it a problem that we leave it open?

For us it's problem because logfiles are rotated by size.
The size of a file could be on order of few Gbs.
We can unlink logfile, logrotate is doing so, but the file will not be delete
form file system. We can use copy-truncate log rotation, this solves the problem,
but such solution seems outdated.

>  I don't believe either that the file will grow much (in normal cases anyway)

We are not in control of what user write to logs. We provide managed PostgreSQL.
Some users can set log_statement = 'all' and get tons of logs.
Our goal is to make logging system accountable to free space monitoring and
efficient (no log copy, no extra occupied space).

We could also avoid pointing pg_ctl log to same logfile as syslogger writes.
But that's how postgresql-common works and it is easier for our users to see
this output among usual logs to understand what was goining on with database.

> In any case, the proposed patch almost certainly introduces new
> problems, in that you dropped the fcloses's into code that
> executes repeatedly.

Probably, I should place fclose() right after successfull syslogger start?

_____

To reproduce problem on can follow this steps:

1) Use this settings:
log_destination = 'stderr,csvlog'
logging_collector = on
log_filename = 'postgresql-11-data.log'
log_file_mode = '0640' 
log_truncate_on_rotation = off

2) pg_ctl -D DemoDb --log=/private/tmp/pg/postgresql-11-data.log start

3) Check out open descriptors (look for types 1w - stdout and 2w - stderr)
lsof | grep postgres | grep log
postgres  7968 munakoiso    1w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log
postgres  7968 munakoiso    2w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log
postgres  7968 munakoiso  12w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log

4) delete file /private/tmp/pg/postgresql-11-data.log to imitate logrotate, then
psql postgres -c "select pg_rotate_logfile();"

5) Check out open descriptors
without using this patch you shold find here lines with 1w and 2w
with using should not

lsof | grep postgres | grep log
postgres  8082 munakoiso    5w      REG                1,4        2451 3074156 /private/tmp/pg/postgresql-11-data.log

______________
Sviatoslav Ermilin
Yandex

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2019-09-14 07:39:55 Re: Psql patch to show access methods info
Previous Message Michael Paquier 2019-09-14 06:03:57 Re: [HACKERS] [PATCH] pageinspect function to decode infomasks