Re: logfile subprocess and Fancy File Functions - Mailing list pgsql-patches

Andreas didn't ask for a full file API.  I suggested it because we were
already going to have some of the functionality.  If rename/unlink are
new problems, we can skip them and just add what Andreas needs right
now.

---------------------------------------------------------------------------

Andreas Pflug wrote:
> 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
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073