On 04/07/2015 05:59 AM, Michael Paquier wrote:
On Mon, Apr 6, 2015 at 9:10 PM, Fujii Masao <masao.fu...@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
