Re: pg_upgrade failure on Windows Server

From: Asif Naeem <anaeem(dot)it(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: pg_upgrade failure on Windows Server
Date: 2015-03-17 20:15:07
Message-ID: CAEB4t-PgQJwN4kjYaQ61d1c_z=FFjVTM4DsrVt6cjwYLQ2DreQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Thank you for useful suggestions, PFA patch, for pg_ctl.c and pg_regress.c
it relies on CreateRestrictedProcess() function now.

src/common/restricted_token.c

> #define write_stderr(fmt, ...) fprintf(stderr, fmt, __VA_ARGS__)

src//bin/pg_ctl/pg_ctl.c

> static void
> write_stderr(const char *fmt,...)
> {
> ...
> /*
> * 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 */
> {
> ...
> }
>

src/backend/port/win32/security.c

> void
> write_stderr(const char *fmt,...)
> {
> ...
> /*
> * 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 */
> {
> ...
> }
>

security.c write_stderr() implementation seems correct, that relies on
pgwin32_is_service() function but it seems little expensive operation to
write error messages. pg_ctl.c write_stderr() implementation depend on
isatty() to detect if it is running as service, I doubt that if it is right
way to to do so. Any suggestion how to tackle this situation, along with
this can we make common routine of write_stderr() function that others use
it instead of defining their own ?.

Please do let me know if I missed something or more information is
required. Thanks.

On Mon, Mar 16, 2015 at 2:07 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)com> wrote:

> +1
>
> On Fri, Mar 13, 2015 at 4:32 AM, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com> wrote:
>
>> On Thu, Mar 12, 2015 at 11:45 PM, Alvaro Herrera
>> <alvherre(at)2ndquadrant(dot)com> wrote:
>> > Michael Paquier wrote:
>> >> On Thu, Mar 12, 2015 at 6:50 AM, Alvaro Herrera
>> >> <alvherre(at)2ndquadrant(dot)com> wrote:
>> >
>> >> > Is this a backpatchable bug fix, or are we considering this only for
>> the
>> >> > master branch?
>> >>
>> >> It would be good to get that backpatched, that's something we really
>> >> miss now IMO. Now it modifies libpgcommon, so Windows packagers (me
>> >> being one) will certainly need to patch a bit stuff but that's a
>> >> one-line changer so it's not a big deal. And I imagine that this is
>> >> actually the reason why Asif reported that as a bug as well.
>> >
>> > I think it'd be better to patch only pg_upgrade in back branches, so
>> > that there are no libpgcommon changes. Seems that would make life
>> > easier for packagers (See the \connect thread, where Robert opined that
>> > it'd be better to duplicate some routines in back branches rather than
>> > refactor libpq code and move the common code to pgcommon. I didn't
>> > completely agree with him at the time, but now that you mention
>> > packagers pain, maybe he has a point.)
>> >
>> > So let's do the refactoring in the master branch only, and duplicate
>> > the code in back branches. Nasty, but it seems the more robust
>> > approach.
>>
>> This plan sounds fine to me.
>> --
>> Michael
>>
>
>

Attachment Content-Type Size
restricted_token.v3.patch application/octet-stream 30.4 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Ján Máté 2015-03-17 22:09:30 Re: BUG #12872: Inconsistent processing of interval values
Previous Message Bruce Momjian 2015-03-17 19:45:45 Re: BUG #12872: Inconsistent processing of interval values