Re: [bug fix] pg_ctl always uses the same event source

From: "MauMau" <maumau307(at)gmail(dot)com>
To: "Magnus Hagander" <magnus(at)hagander(dot)net>, "Amit Kapila" <amit(dot)kapila16(at)gmail(dot)com>
Cc: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Robert Haas" <robertmhaas(at)gmail(dot)com>, "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [bug fix] pg_ctl always uses the same event source
Date: 2014-07-16 11:48:31
Message-ID: CF44CED6C57D4CBC91551AFF1C452A68@maumau
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: "Magnus Hagander" <magnus(at)hagander(dot)net>
> There's also the change to throw an error if the source is already
> registered, which is potentially a bigger problem. Since the default
> will be the same everywhere, do we really want to throw an error when
> you install a second version, now that this is the normal state?
>
> There's also definitely a problem in that that codepath fires up a
> MessageBox, but it's just a function called in a DLL. It might just as
> well be called from a background service or from within an installer
> with no visible desktop, at which point the process will appear to be
> hung... I'm pretty sure you're not allowed to do that.

I got what you mean. I removed changes in pgevent.c except for the default
name. I attached the revised patch.

>>> More importantly, isn't it wrong to claim it will only be used for
>>> register and unregister? If we get an early failure in start, for
>>> example, there are numerous codepaths that will end up calling
>>> write_stderr(), which will use the eventlog when running as a service.
>>> Shouldn't the "-e" parameter be moved under "common options"?
>>
>>
>> Yes, you are right. -e is effective only if pg_ctl is invoked as a
>> Windows
>> service. So it is written at register mode. That is, -e specifies the
>> event source used by the Windows service which is registered by "pg_ctl
>> register".
>
> Oh, right. I see what you mean now. That goes for all parameters
> though, including -D, and we don't specify those as register mode
> only, so I still think it's wrong to place it there. It is now grouped
> with all other parameters that we specifically *don't* write to the
> commandline of the service.

Sorry, but I'm probably not understanding your comment. This may be due to
my English capability. -e is effective only on Windows, so it is placed in
section "Options for Windows". And I could not find a section named "Common
options". -e is currently meangful only with register mode, so it is placed
at register mode in Synopsis section. For example, -D is not attached to
kill mode.

Do you suggest that -e should be attached to all modes in Synopsis section,
or -e should be placed in the section "Options" instead of "Options for
Windows"?

Regards
MauMau

Attachment Content-Type Size
pg_ctl_eventsrc_v11.patch application/octet-stream 7.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-07-16 11:54:54 Re: [TODO] Process pg_hba.conf keywords as case-insensitive
Previous Message Amit Kapila 2014-07-16 11:30:37 Re: [bug fix] pg_ctl always uses the same event source