Re: Win32 testing needed

Andreas Pflug <pgadmin@pse-consulting.de> writes:
> Attached a patch with several issues resolved; only win32 checked.

> After your changes, the error from ReadFile is not ERROR_HANDLE_EOF any
> more, but ERROR_PIPE_BROKEN (which should be expected either), check is
> done for both now.

Got it.

> The logger should *not* use proc_exit but exit(0), because proc_exit
> might try to elog something, after we just closed the log file. IMHO
> there's nothing left to cleanup anyway.

No good.  If we can't survive exiting via proc_exit() then we're in deep
trouble anyway, because that is the path that any elog(ERROR) will take.
It might be best to just leave syslogFile open --- it should be properly
flushed and closed by exit() anyway, no?

> Changing setvbuf to use line buffered mode broke win32; apparently a
> line is not a line there.... changed back to _IONBUF which should be
> identical in result because we're always writing a complete line.

[ looks around ... ]  Oh, we dealt with this before: Windows treats
_IOLBF the same as _IOFBF, which we don't want.  Okay, ifdef time.
We do want _IOLBF on every other platform.

> An observation I didn't track down so far:
> Some LOG messages (e.g. the final logger shutdown, or "received fast
> shutdown request") don't have proper CRLF line ending in win32, but only
> LF.

Weird.  No ideas about that.  Can you determine whether the data coming
through the pipe has the problem?

> If the logger subprocess is killed, it will come up again ok, but
> redirecting to NULL_DEV doesn't work (open returns -1; that's what I had
> realStdErr for).

Why doesn't it work?  Do we just need a different spelling of
"/dev/null" for Windows?  Worst case, we could forcibly close
fileno(stderr) and just not have it pointing at anything if the
open of NULL_DEV doesn't work ... we never check ferror(stderr)
so it wouldn't really matter if output fails ...

> So what now? I'd propose inheriting the old stderr handle
> instead of redirection_done so it can be reused in this case, as in my
> original posting.

No, that was too much of a mess to solve a non-problem.  We do not
actually care whether stderr works in the logger, because everything it
has to say should go through elog anyway.  All we really care about is
that stderr is *not* still connected to the pipe.

I left stderr output enabled in the first instantation of the logger,
because it was easy to do and might provide a way to help debug
otherwise difficult logger problems.  But I don't think we need to go
out of our way to keep it enabled in re-instantiations, seeing that we
don't really expect the logger to crash anyway (see below).

> Finally, you don't seem to be a friend of a logfile rotation user
> trigger...

Nope, I'm not.  I think it's a bad solution to a nonexistent problem.
The logger's control parameters are more than sufficient.  Furthermore,
we'd really prefer that the logger doesn't ever crash (the restart
business is a bit ticklish) and so the fewer features it has, the better.

            regards, tom lane