From: | Andreas Pflug <pgadmin(at)pse-consulting(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, PostgreSQL Patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: logfile subprocess and Fancy File Functions |
Date: | 2004-07-23 23:10:25 |
Message-ID: | 41019AE1.2090302@pse-consulting.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Tom Lane wrote:
> Bruce Momjian wrote:
>
>>Are we done?
>
>
> Nope, the syslogger part of this is still a mess. I don't want any
> pg_logfile_rotate() function in there at all: its presence is a hangover
> from a different design philosophy.
No. As I mentioned earlier, there might be cases when a superuser wants
to issue the rotation to keep some stuff in a single log file.
> Nor pg_reload_conf(); where did
> that come from? (Hint: if you can edit postgresql.conf you do not need
> a helper function to signal the postmaster.)
Wrong. The generic file functions allow editing postgresql.conf without
having file access, and consequently you'd like to make that active
without consoling to the server.
> Nor the pg_logdir_ls view,
> as that will malfunction completely if we aren't actually using the
> syslogger facility, yet there's no graceful way to make it go away.
? It will show nothing if there are no files, so what's the problem?
>
> I also find the Log_destination setup to be less than carefully thought
> out: what in the world does it mean to specify stderr and file as
> distinct log destinations?
stderr is simply untouched. Actually it works parallel.
This design cannot support that, and doesn't
> need to AFAICS.
I didn't want to wipe out the default logging method right away. Of
course, even log_destination=syslog might be redirected.
> What we probably want instead is a separate
> redirect_stderr_to_files boolean (I'm sure a better name could be
> thought of).
>
> Also, while I'm aware that a superuser can persuade the backend to write
> on anything, it doesn't follow that we should invent pg_file_write(),
> pg_file_rename(), or pg_file_unlink(). Those are not needed for the
> originally intended purpose of this patch
I proposed to separate them, they're indeed non-related.
What I'd like is
SELECT pg_file_unlink('postgresql.conf.bak');
SELECT pg_file_write('postgresql.conf.tmp', 'listen_addresses=...');
SELECT pg_file_rename('postgresql.conf.tmp', 'postgresql.conf',
'postgresql.conf.bak');
SELECT pg_reload_conf();
> and I think that they are just
> invitations to trouble. If you are aware that there are burglars out
> there who know how to pick your door lock, do you then post directions
> and tools to help on your door?
These are superuser only, and executed in the postgres user context.
We're offering a superuser to shoot himself into the foot wherever he
likes regarding system catalog etc. I wouldn't have a problem if paths
may only be relative to PG_DATA and .. is disallowed.
> Finally, I can tell without even trying it that the present syslogger
> code will fail miserably in EXEC_BACKEND case.
I don't have an EXEC_BACKEND environment, I already pointed out that
this has to be tested.
> It's expecting
> realStdErr to be inherited which it will not be.
Yes, this is probably one source of problems for EXEC_BACKEND.
> I don't think the
> notion of respawning the logger will work; we're just going to have to
> assume it is as reliable as the postmaster is, and we only need launch
> it once.
It works.
(BTW, did Magnus ever verify for us that redirecting stderr
> into a pipe will work at all on Windows?
Actually, dup2 etc is documented perfectly for win32. This certainly
doesn't mean anything...
win32 system error messages are retrieved using GetLastError, not from
stderr, so problems are a bit different anyway.
Regards,
Andreas
From | Date | Subject | |
---|---|---|---|
Next Message | Matthew T. O'Connor | 2004-07-24 01:58:06 | Re: autovauum integration patch: Attempt #4 |
Previous Message | Peter Eisentraut | 2004-07-23 22:43:37 | Re: autovauum integration patch: Attempt #4 |