From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_rewind and log messages |
Date: | 2015-04-07 20:39:04 |
Message-ID: | 55244068.6070708@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 04/07/2015 05:59 AM, Michael Paquier wrote:
> On Mon, Apr 6, 2015 at 9:10 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> I eliminated a bunch of newlines in the log messages that seemed
>>> really unnecessary to me, simplifying a bit the whole.
So the patch removed the newlines from the error messages, and added the
newline into pg_fatal/log instead. Perhaps that's good idea, but it's
not actually consistent with what we do in other utilities in src/bin.
See write_msg() and exit_horribly() in pg_dump, write_stderr() in
pg_ctl, and psql_error() in psql. If we want to change that in pg_rewind
(IMHO we shouldn't), let's do that as a completely separate patch.
> While hacking this stuff, I noticed as
>>> well that pg_rewind could be called as root on non-Windows platform,
>>> that's dangerous from a security point of view as process manipulates
>>> files in PGDATA. Hence let's block that. On Windows, a restricted
>>> token should be used.
>>
>> Good catch! I think it's better to implement it as a separate patch
>> because it's very different from log message problem.
>
> Attached are new patches:
> - 0001 fixes the restriction issues: restricted token on Windows, and
> forbid non-root user on non-Windows.
Committed this one.
> - 0002 includes the improvements for logging, addressing the comments above.
This is a bit of a mixed bag. I'll comment on the three points from the
commit message separately:
> Fix inconsistent handling of logs in pg_rewind
>
> pg_rewind was handling a couple of things differently compared to the
> other src/bin utilities:
> - Logging output needs to be flushed on stderr, not stdout
Agreed in general. But there's also precedent in printing some stuff to
stdout: pg_ctl does that for the status message, like "server starting".
As does initdb.
I'm pretty unclear on what the rule here is.
> - Logging messages should be prefixed with the application name
We tend to do that for error messages, but not for other messages. Seems
inappropriate for the progress messages.
> - pg_fatal exits with error code of 1, but it was sometimes followed by
> subsequent logs, making this information lost in the process.
Fixed.
- Heikki
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2015-04-07 20:53:20 | Re: pg_rewind and log messages |
Previous Message | Tom Lane | 2015-04-07 20:33:52 | Re: pg_restore -t should match views, matviews, and foreign tables |