Horton

Thread

  1. Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-17T12:58:28Z

    Sorry.
I forget to attach the patch file.

Begin forwarded message:

Date: Mon, 16 Aug 2010 19:49:20 +0800
From: Quan Zongliang <quanzongliang@gmail.com>
To: pgsql-hackers@postgresql.org
Subject: patch for pg_ctl.c to add windows service start-type


Hi, all

I modified pg_ctl.c to add a new option for Windows service start-type.
new option is -S [auto|demand]

For example, the command can be used under Windows:
pg_ctl register -N "s-name" -S auto
or
pg_ctl register -N "s-name" -S demand

The created service will be SERVICE_AUTO_START or SERVICE_DEMAND_START respectively.

regards

-- 
Quan Zongliang <quanzongliang@gmail.com>


-- 
Quan Zongliang <quanzongliang@gmail.com>

  2. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Magnus Hagander <magnus@hagander.net> — 2010-08-17T14:18:42Z

    On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang@gmail.com> wrote:
>
> Sorry.
> I forget to attach the patch file.

Without looking at the details of this patch, it looks reasonable - so
please put it on the commitfest page, if you haven't already.

It does, however, lack documentation updates - that needs to be done
before it can get applied.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

  3. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-19T14:24:54Z

    documents attached. html and man-page

On Tue, 17 Aug 2010 16:18:42 +0200
Magnus Hagander <magnus@hagander.net> wrote:

> On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang@gmail.com> wrote:
> >
> > Sorry.
> > I forget to attach the patch file.
> 
> Without looking at the details of this patch, it looks reasonable - so
> please put it on the commitfest page, if you haven't already.
> 
> It does, however, lack documentation updates - that needs to be done
> before it can get applied.
> 
> -- 
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/


-- 
Quan Zongliang <quanzongliang@gmail.com>

  4. Re: Fw: patch for pg_ctl.c to add windows service start-type

    David Fetter <david@fetter.org> — 2010-08-19T15:48:53Z

    On Thu, Aug 19, 2010 at 10:24:54PM +0800, Quan Zongliang wrote:
> documents attached. html and man-page

Thanks!

For future reference, the way to patch docs is by patching the SGML
source.  Please find enclosed a patch which incorporates the code
patch you sent with these docs.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  5. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Alvaro Herrera <alvherre@commandprompt.com> — 2010-08-19T19:47:43Z

    Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:

> +    <varlistentry>
> +     <term><option>-S <replaceable class="parameter"></replaceable></option></term>

You omitted the start-type inside the <replaceable> tag.  Also, the "a"
and "d" values seem to be accepted but not documented.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  6. Re: Fw: patch for pg_ctl.c to add windows service start-type

    David Fetter <david@fetter.org> — 2010-08-19T20:40:18Z

    On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> 
> > +    <varlistentry>
> > +     <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> 
> You omitted the start-type inside the <replaceable> tag.  Also, the "a"
> and "d" values seem to be accepted but not documented.

D'oh!  Changed patch enclosed.  Now in context format :)

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  7. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Alvaro Herrera <alvherre@commandprompt.com> — 2010-08-19T20:48:53Z

    Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> > 
> > > +    <varlistentry>
> > > +     <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> > 
> > You omitted the start-type inside the <replaceable> tag.  Also, the "a"
> > and "d" values seem to be accepted but not documented.
> 
> D'oh!  Changed patch enclosed.  Now in context format :)

Thanks.

Another thing -- why is there an enum at all?  Seems it'd be
simpler to assign the right value to the variable in the getopt() code
to start with.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  8. Re: Fw: patch for pg_ctl.c to add windows service start-type

    David Fetter <david@fetter.org> — 2010-08-19T20:51:08Z

    On Thu, Aug 19, 2010 at 04:48:53PM -0400, Alvaro Herrera wrote:
> Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> > On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> > > 
> > > > +    <varlistentry>
> > > > +     <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> > > 
> > > You omitted the start-type inside the <replaceable> tag.  Also,
> > > the "a" and "d" values seem to be accepted but not documented.
> > 
> > D'oh!  Changed patch enclosed.  Now in context format :)
> 
> Thanks.
> 
> Another thing -- why is there an enum at all?  Seems it'd be simpler
> to assign the right value to the variable in the getopt() code to
> start with.

That's a question for the patch author.  I was just cleaning up the
docs :)

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  9. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-19T23:01:54Z

    Because Windows's CreateService has serial start-type:
SERVICE_AUTO_START
SERVICE_BOOT_START
SERVICE_DEMAND_START
SERVICE_DISABLED
SERVICE_SYSTEM_START

Although all of them are not useful for pg service.
I think it is better to use enum.

On Thu, 19 Aug 2010 16:48:53 -0400
Alvaro Herrera <alvherre@commandprompt.com> wrote:

> Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> > On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> > > 
> > > > +    <varlistentry>
> > > > +     <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> > > 
> > > You omitted the start-type inside the <replaceable> tag.  Also, the "a"
> > > and "d" values seem to be accepted but not documented.
> > 
> > D'oh!  Changed patch enclosed.  Now in context format :)
> 
> Thanks.
> 
> Another thing -- why is there an enum at all?  Seems it'd be
> simpler to assign the right value to the variable in the getopt() code
> to start with.
> 
> -- 
> Álvaro Herrera <alvherre@commandprompt.com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support


-- 
Quan Zongliang <quanzongliang@gmail.com>

  10. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-19T23:04:13Z

    I don't know how to edit documents exactly before.

Thanks.

On Thu, 19 Aug 2010 08:48:53 -0700
David Fetter <david@fetter.org> wrote:

> On Thu, Aug 19, 2010 at 10:24:54PM +0800, Quan Zongliang wrote:
> > documents attached. html and man-page
> 
> Thanks!
> 
> For future reference, the way to patch docs is by patching the SGML
> source.  Please find enclosed a patch which incorporates the code
> patch you sent with these docs.
> 
> Cheers,
> David.
> -- 
> David Fetter <david@fetter.org> http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter      XMPP: david.fetter@gmail.com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
> 
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Quan Zongliang <quanzongliang@gmail.com>

  11. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Magnus Hagander <magnus@hagander.net> — 2010-08-20T09:21:18Z

    On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang@gmail.com> wrote:
> Because Windows's CreateService has serial start-type:
> SERVICE_AUTO_START
> SERVICE_BOOT_START
> SERVICE_DEMAND_START
> SERVICE_DISABLED
> SERVICE_SYSTEM_START
>
> Although all of them are not useful for pg service.
> I think it is better to use enum.

I don't see us ever using anything other than auto or demand. The
others aren't for "regular services", except for "disabled". And
adding a disabled service makes no sense :-) So I'm with Alvaro, I
think it's a good idea to simplify that.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

  12. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-22T14:03:32Z

    Sure, I agree.
New patch attached. How about this?

On Fri, 20 Aug 2010 11:21:18 +0200
Magnus Hagander <magnus@hagander.net> wrote:

> On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang@gmail.com> wrote:
> > Because Windows's CreateService has serial start-type:
> > SERVICE_AUTO_START
> > SERVICE_BOOT_START
> > SERVICE_DEMAND_START
> > SERVICE_DISABLED
> > SERVICE_SYSTEM_START
> >
> > Although all of them are not useful for pg service.
> > I think it is better to use enum.
> 
> I don't see us ever using anything other than auto or demand. The
> others aren't for "regular services", except for "disabled". And
> adding a disabled service makes no sense :-) So I'm with Alvaro, I
> think it's a good idea to simplify that.
> 
> 
> -- 
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/


-- 
Quan Zongliang <quanzongliang@gmail.com>

  13. Re: Fw: patch for pg_ctl.c to add windows service start-type

    David Fetter <david@fetter.org> — 2010-08-22T15:25:59Z

    On Sun, Aug 22, 2010 at 10:03:32PM +0800, Quan Zongliang wrote:
> Sure, I agree.
> New patch attached. How about this?

Docs re-added.  Please not to leave these out in future patches. :)

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  14. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Quan Zongliang <quanzongliang@gmail.com> — 2010-08-24T12:05:55Z

    Which files need be modified?
sgml, html, and man-page or only sgml?
I am not familiar with this.

On Sun, 22 Aug 2010 08:25:59 -0700
David Fetter <david@fetter.org> wrote:

> On Sun, Aug 22, 2010 at 10:03:32PM +0800, Quan Zongliang wrote:
> > Sure, I agree.
> > New patch attached. How about this?
> 
> Docs re-added.  Please not to leave these out in future patches. :)
> 
> Cheers,
> David.
> -- 
> David Fetter <david@fetter.org> http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter      XMPP: david.fetter@gmail.com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
> 
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Quan Zongliang <quanzongliang@gmail.com>

  15. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Magnus Hagander <magnus@hagander.net> — 2010-08-24T12:11:06Z

    On Tue, Aug 24, 2010 at 2:05 PM, Quan Zongliang <quanzongliang@gmail.com> wrote:
> Which files need be modified?
> sgml, html, and man-page or only sgml?
> I am not familiar with this.

Only SGML. HTML and man pages are generated from the SGML.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

  16. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Itagaki Takahiro <itagaki.takahiro@gmail.com> — 2010-09-30T02:40:10Z

    Hi, I have a question about the latest patch.

On Sun, Aug 22, 2010 at 11:03 PM, Quan Zongliang
<quanzongliang@gmail.com> wrote:
> New patch attached. How about this?
>> I don't see us ever using anything other than auto or demand. The
>> others aren't for "regular services"

+ set_starttype(char *starttypeopt)
+ {
+ 	if (strcmp(starttypeopt, "a") == 0 || strcmp(starttypeopt, "auto") == 0)
+ 		pgctl_start_type = SERVICE_AUTO_START;
+ 	else if (strcmp(starttypeopt, "d") == 0 || strcmp(starttypeopt,
"demand") == 0)
+ 		pgctl_start_type = SERVICE_DEMAND_START;

It accepts only "a" and "auto" for auto, but "au" or "aut" are rejected.
Is is an intended behavior?  I think we can use prefix match here because
we use the logic in some places. For example,

postgres=# SELECT 'tru'::bool, 'fal'::bool;
 bool | bool
------+------
 t    | f
(1 row)

-- 
Itagaki Takahiro

  17. Re: Fw: patch for pg_ctl.c to add windows service start-type

    Magnus Hagander <magnus@hagander.net> — 2010-09-30T08:12:15Z

    On Thu, Sep 30, 2010 at 04:40, Itagaki Takahiro
<itagaki.takahiro@gmail.com> wrote:
> Hi, I have a question about the latest patch.
>
> On Sun, Aug 22, 2010 at 11:03 PM, Quan Zongliang
> <quanzongliang@gmail.com> wrote:
>> New patch attached. How about this?
>>> I don't see us ever using anything other than auto or demand. The
>>> others aren't for "regular services"
>
> + set_starttype(char *starttypeopt)
> + {
> +       if (strcmp(starttypeopt, "a") == 0 || strcmp(starttypeopt, "auto") == 0)
> +               pgctl_start_type = SERVICE_AUTO_START;
> +       else if (strcmp(starttypeopt, "d") == 0 || strcmp(starttypeopt,
> "demand") == 0)
> +               pgctl_start_type = SERVICE_DEMAND_START;
>
> It accepts only "a" and "auto" for auto, but "au" or "aut" are rejected.
> Is is an intended behavior?  I think we can use prefix match here because
> we use the logic in some places. For example,

I think it's modeled on what we allow for the "-m" parameter, and ISTM
it's good to keep those consistent.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/