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

On 31.8.2015 8:51, Michael Paquier wrote:
> On Fri, Aug 28, 2015 at 7:46 PM, <ekocjan(at)gmail(dot)com> wrote:
>> 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?
>

I tested with these two changes:

diff -ur postgresql-9.6devel.orig\src\bin\pg_ctl\pg_ctl.c
postgresql-9.6devel\src\bin\pg_ctl\pg_ctl.c
--- postgresql-9.6devel.orig\src\bin\pg_ctl\pg_ctl.c Mon Aug 31
13:16:51 2015
+++ postgresql-9.6devel\src\bin\pg_ctl\pg_ctl.c Mon Aug 31 15:12:54 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 (!pgwin32_is_service()) /* Running as a service */
+ if (pgwin32_is_service()) /* Running as a service */
{
char errbuf[2048]; /* Arbitrary size? */

@@ -2131,6 +2131,9 @@
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_ctl"));
start_time = time(NULL);
+
+ write_stderr("%s: test test\n",
+ progname);

/*
* save argv[0] so do_start() can look for the postmaster if
necessary. we

1) It now works ok for development under regular account.

2) However there is no output on stderr when pg_ctl is running as a
subprocess of a service. So I would still have to patch pg_ctl if I want
stderr. I haven't looked into elog.c yet, maybe stderr from there would
be good to have as well? So far, pg_ctl output and pg_logs\* was enough
for me to debug the installations. Technically, I could also monitor
Windows event log and dump events on the fly into my report file. But it
seems simpler to just patch PostgreSQL locally to force stderr and reuse
all the logic from the Unix port...

Thank you for looking into this,
Egon