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-13 06:41:11
Message-ID: 664881568356861@iva5-c4dd0484b46b.qloud-c.yandex.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

<div><div>Hi Tom,</div><div> </div><div>Thank you for quick reply.</div><div> </div><div>&gt; I'm quite certain that the current behavior is intentional, if only</div><div>&gt; because closing the syslogger's stderr would make it impossible to</div><div>&gt; debug problems inside the syslogger.</div><div>Developer who debugs syslogger, probably, can remove fclose() calls.</div><div>Maybe we can have a switch for this?</div><div>As far as I understand there is no stdout printing in syslogger?</div><div> </div><div>&gt; Why is it a problem that we leave it open?</div><div> </div><div>For us it's problem because logfiles are rotated by size.</div><div>The size of a file could be on order of few Gbs.</div><div>We can unlink logfile, logrotate is doing so, but the file will not be delete</div><div>form file system. We can use copy-truncate log rotation, this solves the problem,</div><div>but such solution seems outdated.</div><div> </div><div>&gt;  I don't believe either that the file will grow much (in normal cases anyway)</div><div> </div><div>We are not in control of what user write to logs. We provide managed PostgreSQL.</div><div>Some users can set log_statement = 'all' and get tons of logs.</div><div>Our goal is to make logging system accountable to free space monitoring and</div><div>efficient (no log copy, no extra occupied space).</div><div> </div><div>We could also avoid pointing pg_ctl log to same logfile as syslogger writes.</div><div>But that's how postgresql-common works and it is easier for our users to see</div><div>this output among usual logs to understand what was goining on with database.</div><div> </div><div>&gt; In any case, the proposed patch almost certainly introduces new</div><div>&gt; problems, in that you dropped the fcloses's into code that</div><div>&gt; executes repeatedly.</div><div> </div><div>Probably, I should place fclose() right after successfull syslogger start?</div><div> </div><div>_____</div><div> </div><div>To reproduce problem on can follow this steps:</div><div> </div><div>1) Use this settings:</div><div>log_destination = 'stderr,csvlog'</div><div>logging_collector = on </div><div>log_filename = 'postgresql-11-data.log'</div><div>log_file_mode = '0640'   </div><div>log_truncate_on_rotation = off</div><div> </div><div>2) pg_ctl -D DemoDb --log=/private/tmp/pg/postgresql-11-data.log start </div><div> </div><div>3) Check out open descriptors (look for types 1w - stdout and 2w - stderr)</div><div>lsof | grep postgres | grep log</div><div>postgres   7968 munakoiso    1w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log</div><div>postgres   7968 munakoiso    2w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log</div><div>postgres   7968 munakoiso   12w      REG                1,4        1170 3074156 /private/tmp/pg/postgresql-11-data.log</div><div> </div><div>4) delete file /private/tmp/pg/postgresql-11-data.log to imitate logrotate, then</div><div>psql postgres -c "select pg_rotate_logfile();"</div><div> </div><div>5) Check out open descriptors</div><div>without using this patch you shold find here lines with 1w and 2w</div><div>with using should not</div><div> </div><div>lsof | grep postgres | grep log</div><div>postgres   8082 munakoiso    5w      REG                1,4        2451 3074156 /private/tmp/pg/postgresql-11-data.log</div><div> </div><div> </div><div>______________</div><div>Sviatoslav Ermilin</div><div>Yandex</div><div> </div></div><div> </div><div>12.09.2019, 18:33, "Tom Lane" &lt;tgl(at)sss(dot)pgh(dot)pa(dot)us&gt;:</div><blockquote><p>=?utf-8?B?0KHQstGP0YLQvtGB0LvQsNCyINCV0YDQvNC40LvQuNC9?= &lt;<a href="mailto:munakoiso(at)yandex-team(dot)ru">munakoiso(at)yandex-team(dot)ru</a>&gt; writes:</p><blockquote> &lt;div&gt;&lt;div&gt;Hi!&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;Few months ago we have encountered situation when some quite big open log files were open by Postres despite being deleted.&lt;/div&gt;&lt;div&gt;This affects free space caluculation in out managed PostgreSQL instances.&lt;/div&gt;&lt;div&gt;Currently I'm investigating this issue.&lt;/div&gt;&lt;div&gt;We traced some roots to unclosed descriptors in Perl code of postgresql-common [0], but we still face some unclosed descriptors pointing to log file.&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;Debian tools from postgresql-common starts pg_ctl piped to logfile. Descriptor is piped to logfile and block it for delete.&lt;/div&gt;&lt;div&gt;That is why we can't delete logfile.1 after logrotate.&lt;/div&gt;&lt;div&gt;And after second logrotate logfile.1 is in "deleted" status, but can't be deleted in fact.&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;Here I apply path with possible solution. In this patch stdout and stderr pipes are just closed in syslogger.&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;--&lt;/div&gt;&lt;div&gt;Sviatoslav Ermilin&lt;/div&gt;&lt;div&gt;Yandex&lt;/div&gt;&lt;div&gt; &lt;/div&gt;&lt;div&gt;[0] <a href="https://salsa.debian.org/postgresql/postgresql-common/commit/580aa0677edc222ebaf6e1031cf3929f847f27fb">https://salsa.debian.org/postgresql/postgresql-common/commit/580aa0677edc222ebaf6e1031cf3929f847f27fb</a>&lt;/div&gt;&lt;/div&gt;</blockquote><p><br />I'm quite certain that the current behavior is intentional, if only<br />because closing the syslogger's stderr would make it impossible to<br />debug problems inside the syslogger. Why is it a problem that we<br />leave it open? I don't believe either that the file will grow much<br />(in normal cases anyway), or that it's impossible to unlink it<br />(except on Windows, but you didn't say anything about Windows).<br /><br />In any case, the proposed patch almost certainly introduces new<br />problems, in that you dropped the fcloses's into code that<br />executes repeatedly.<br /><br />                        regards, tom lane</p></blockquote>

Attachment Content-Type Size
unknown_filename text/html 5.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2019-09-13 06:47:06 Re: pgbench - allow to create partitioned tables
Previous Message Noah Misch 2019-09-13 06:20:28 Re: pgsql: Use data directory inode number, not port, to select SysV resour