Re: BUG #13594: pg_ctl.exe redirects stderr to Windows Events Log if stderr is redirected to pipe

On Fri, Aug 28, 2015 at 7:46 PM, <ekocjan(at)gmail(dot)com> wrote:
>
> I run PostgreSQL as a part of a larger software package, pg_ctl is used to
> control PostgreSQL. I capture all subprocess stderr logs to make a complete
> report.
>
> Problem:
>
> On Windows (but not Unix!), PostgreSQL (any version) pg_ctl stops writing
> into stderr if stderr is not a character device. As a result, it's not
> possible to capture stderr of pg_ctl when using CreateProcess and
> STARTUPINFO.hStdError. It might be possible to create a workaround with a
> console buffer, but I think it's quite ugly:
> http://www.codeproject.com/Articles/16163/Real-Time-Console-Output-Redirection

Yep, that's ugly.

>
> Solution:
>
> Fix pg_ctl to write into stderr, if stderr is available and not just a
> character device. Possibly, add a flag to force stderr. I don't have the big
> picture, but this is the quick solution that I use right now:
>
> diff -ur postgresql-9.3.6.orig\src\bin\pg_ctl\pg_ctl.c
> postgresql-9.3.6\src\bin\pg_ctl\pg_ctl.c
> --- postgresql-9.3.6.orig\src\bin\pg_ctl\pg_ctl.c Mon Feb 02 22:43:50 2015
> +++ postgresql-9.3.6\src\bin\pg_ctl\pg_ctl.c Fri Aug 28 08:21:01 2015
> @@ -215,7 +215,7 @@
> * On Win32, we print to stderr if running on a console, or write to
> * eventlog if running as a service
> */
> - if (!isatty(fileno(stderr))) /* Running as a service */
> + if (getenv("PG_CTL_STDERR") == NULL && !isatty(fileno(stderr))) /* Running
> as a service */
> {
> char errbuf[2048]; /* Arbitrary size? */
>
>
> Then in my main process: _putenv("PG_CTL_STDERR=1")

Honestly I don't like that much.

>
> I also noticed, that write_stderr() in src/backend/utils/error/elog.c uses a
> different check (not isatty):
>
> if (pgwin32_is_service()) /* Running as a service */

The last time I poked at that, switching to use pgwin32_is_service was
not that straight-forward as it is a backend-only routine, relying on
write_stderr, write_eventlog and write_console, and pg_ctl has its own
concept of those routines:
http://www.postgresql.org/message-id/CAB7nPqSxWcUFpzL4LaOZAt4k-FPM7fy6Et7p50w8S5NbRfCwFw@mail.gmail.com
But actually looking at it with fresh eyes it is not that complicated
if we switch backend/port/win32/security.c to port/win32security.c to
make pgwin32_is_service available as well for frontends. And it would
obviously still be an improvement to switch isatty() to
pgwin32_is_service() in pg_ctl.c, the trick being to remove the
dependency to write_stderr when pgwin32_is_admin is called from
frontend so as it does not interact badly with the existing logging
infrastructure of elog.c and the definition of write_stderr in
pg_ctl.c. Note that in any case we cannot rely on ereport when calling
pgwin32_is_service on backend because it is called very early at
process startup, so there is no need to dance with ifdef FRONTEND
there.

This change is too invasive so I think that making it HEAD-only is
better, and that's a change of behavior. I should perhaps have split
the patch into two: one for the move to src/port, the other for pg_ctl
but it's not really a big deal. I'll add it to the next CF I think
that's worth considering it.

Egon, would the patch attached help in your case?
Regards,
--
Michael