[v9.3] Extra Daemons (Re: elegant and effective way for running jobs inside a database)

2012/3/10 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On Fri, Mar 9, 2012 at 6:51 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>>
>> On 03/09/2012 01:40 PM, Robert Haas wrote:
>>>
>>> On Fri, Mar 9, 2012 at 12:02 PM, David E. Wheeler<david(at)justatheory(dot)com>
>>>  wrote:
>>>>
>>>> On Mar 9, 2012, at 7:55 AM, Merlin Moncure wrote:
>>>>>
>>>>> 100% agree  (having re-read the thread and Alvaro's idea having sunk
>>>>> in).  Being able to set up daemon processes side by side with the
>>>>> postmaster would fit the bill nicely.  It's pretty interesting to
>>>>> think of all the places you could go with it.
>>>>
>>>> pgAgent could use it *right now*. I keep forgetting to restart it after
>>>> restarting PostgreSQL and finding after a day or so that no jobs have run.
>>>
>>> That can and should be fixed by teaching pgAgent that failing to
>>> connect to the server, or getting disconnected, is not a fatal error,
>>> but a reason to sleep and retry.
>>
>>
>> Yeah. It's still not entirely clear to me what a postmaster-controlled
>> daemon is going to be able to do that an external daemon can't.
>
> Start and stop at the same time as postmaster, without any pain.
>
> It's a considerable convenience to be able to design this aspect once
> and then have all things linked to the postmaster follow that. It
> means people will be able to write code that runs on all OS easily,
> without everybody having similar but slightly different code about
> starting up, reading parameters, following security rules etc.. Tight
> integration, with good usability.
>
I tried to implement a patch according to the idea. It allows extensions
to register an entry point of the self-managed daemon processes,
then postmaster start and stop them according to the normal manner.

[kaigai(at)iwashi patch]$ ps ax | grep postgres
27784 pts/0 S 0:00 /usr/local/pgsql/bin/postgres
27786 ? Ss 0:00 postgres: writer process
27787 ? Ss 0:00 postgres: checkpointer process
27788 ? Ss 0:00 postgres: wal writer process
27789 ? Ss 0:00 postgres: autovacuum launcher process
27790 ? Ss 0:00 postgres: stats collector process
27791 ? Ss 0:00 postgres: auth_counter <== (*)

The auth_counter being included in this patch is just an example of
this functionality. It does not have significant meanings. It just logs
number of authentication success and fails every intervals.

I'm motivated to define an extra daemon that attach shared memory
segment of PostgreSQL as a computing server to avoid limitation of
number of GPU code that we can load concurrently.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Wed, Apr 25, 2012 at 10:40 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> I tried to implement a patch according to the idea. It allows extensions
> to register an entry point of the self-managed daemon processes,
> then postmaster start and stop them according to the normal manner.

I've got a provisional version of this as well, that I was expecting
to submit for 9.3CF1

Best thing is probably to catch up at PGCon on this, so we can merge
the proposals and code.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 25 April 2012 10:40, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> I tried to implement a patch according to the idea. It allows extensions
> to register an entry point of the self-managed daemon processes,
> then postmaster start and stop them according to the normal manner.

The patch needs much work yet, but has many good ideas.

There doesn't seem to be a place where we pass the parameter to say
which one of the multiple daemons a particular process should become.
It would be helpful for testing to make the example module call 2
daemons each with slightly different characteristics or parameters, so
we can test the full function of the patch.

I think its essential that we allow these processes to execute SQL, so
we must correctly initialise them as backends and set up signalling.
Which also means we need a parameter to limit the number of such
processes.

Also, I prefer to call these bgworker processes, which is more similar
to auto vacuum worker and bgwriter naming. That also gives a clue as
to how to set up signalling etc..

I don't think we should allow these processes to override sighup and
sigterm. Signal handling should be pretty standard, just as it is with
normal backends.

I have a prototype that has some of these characteristics, so I see
our work as complementary.

At present, I don't think this patch would be committable in CF1, but
I'd like to make faster progress with it than that. Do you want to
work on this more, or would you like me to merge our prototypes into a
more likely candidate?

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

2012/6/8 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On 25 April 2012 10:40, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> I tried to implement a patch according to the idea. It allows extensions
>> to register an entry point of the self-managed daemon processes,
>> then postmaster start and stop them according to the normal manner.
>
> The patch needs much work yet, but has many good ideas.
>
> There doesn't seem to be a place where we pass the parameter to say
> which one of the multiple daemons a particular process should become.
> It would be helpful for testing to make the example module call 2
> daemons each with slightly different characteristics or parameters, so
> we can test the full function of the patch.
>
This patch intended to register a daemon multiple times with different
name such as "auth-counter-1" or "auth-counter-2".
But, I agree with the suggestion to take a parameter to identify each
daemon makes interface better than the original one.

> I think its essential that we allow these processes to execute SQL, so
> we must correctly initialise them as backends and set up signalling.
> Which also means we need a parameter to limit the number of such
> processes.
>
It should be controllable with a flag of RegisterExtraDaemon().
Although it helps to reduce code duplication in case when extra daemons
execute SQL, but some other use-cases may not need SQL execution.

> Also, I prefer to call these bgworker processes, which is more similar
> to auto vacuum worker and bgwriter naming. That also gives a clue as
> to how to set up signalling etc..
>
> I don't think we should allow these processes to override sighup and
> sigterm. Signal handling should be pretty standard, just as it is with
> normal backends.
>
Hmm. CHECK_FOR_INTERRUPTS() might be sufficient to handle
signaling behavior according to the standard.

> I have a prototype that has some of these characteristics, so I see
> our work as complementary.
>
> At present, I don't think this patch would be committable in CF1, but
> I'd like to make faster progress with it than that. Do you want to
> work on this more, or would you like me to merge our prototypes into a
> more likely candidate?
>
I'm not favor in duplicate similar efforts. If available, could you merge
some ideas in my patch into your prototypes?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Sun, Jun 10, 2012 at 4:15 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2012/6/8 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>
>> I have a prototype that has some of these characteristics, so I see
>> our work as complementary.
>>
>> At present, I don't think this patch would be committable in CF1, but
>> I'd like to make faster progress with it than that. Do you want to
>> work on this more, or would you like me to merge our prototypes into a
>> more likely candidate?
>>
> I'm not favor in duplicate similar efforts. If available, could you merge
> some ideas in my patch into your prototypes?
>

so, we are waiting for a new patch? is it coming from Simon or Kohei?

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

On 21 June 2012 19:13, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
> On Sun, Jun 10, 2012 at 4:15 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> 2012/6/8 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>
>>> I have a prototype that has some of these characteristics, so I see
>>> our work as complementary.
>>>
>>> At present, I don't think this patch would be committable in CF1, but
>>> I'd like to make faster progress with it than that. Do you want to
>>> work on this more, or would you like me to merge our prototypes into a
>>> more likely candidate?
>>>
>> I'm not favor in duplicate similar efforts. If available, could you merge
>> some ideas in my patch into your prototypes?
>>
>
> so, we are waiting for a new patch? is it coming from Simon or Kohei?

There is an updated patch coming from me. I thought I would focus on
review of other work first.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

2012-06-21 23:53 keltezéssel, Simon Riggs írta:
> On 21 June 2012 19:13, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
>> On Sun, Jun 10, 2012 at 4:15 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> 2012/6/8 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>>
>>>> I have a prototype that has some of these characteristics, so I see
>>>> our work as complementary.
>>>>
>>>> At present, I don't think this patch would be committable in CF1, but
>>>> I'd like to make faster progress with it than that. Do you want to
>>>> work on this more, or would you like me to merge our prototypes into a
>>>> more likely candidate?
>>>>
>>> I'm not favor in duplicate similar efforts. If available, could you merge
>>> some ideas in my patch into your prototypes?
>>>
>> so, we are waiting for a new patch? is it coming from Simon or Kohei?
> There is an updated patch coming from me. I thought I would focus on
> review of other work first.

We have some use cases for this patch, when can you post
a new version? I would test and review it.

Thanks in advance,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

2012-04-25 11:40 keltezéssel, Kohei KaiGai írta:
> 2012/3/10 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>> On Fri, Mar 9, 2012 at 6:51 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>>
>>> On 03/09/2012 01:40 PM, Robert Haas wrote:
>>>> On Fri, Mar 9, 2012 at 12:02 PM, David E. Wheeler<david(at)justatheory(dot)com>
>>>> wrote:
>>>>> On Mar 9, 2012, at 7:55 AM, Merlin Moncure wrote:
>>>>>> 100% agree (having re-read the thread and Alvaro's idea having sunk
>>>>>> in). Being able to set up daemon processes side by side with the
>>>>>> postmaster would fit the bill nicely. It's pretty interesting to
>>>>>> think of all the places you could go with it.
>>>>> pgAgent could use it *right now*. I keep forgetting to restart it after
>>>>> restarting PostgreSQL and finding after a day or so that no jobs have run.
>>>> That can and should be fixed by teaching pgAgent that failing to
>>>> connect to the server, or getting disconnected, is not a fatal error,
>>>> but a reason to sleep and retry.
>>>
>>> Yeah. It's still not entirely clear to me what a postmaster-controlled
>>> daemon is going to be able to do that an external daemon can't.
>> Start and stop at the same time as postmaster, without any pain.
>>
>> It's a considerable convenience to be able to design this aspect once
>> and then have all things linked to the postmaster follow that. It
>> means people will be able to write code that runs on all OS easily,
>> without everybody having similar but slightly different code about
>> starting up, reading parameters, following security rules etc.. Tight
>> integration, with good usability.
>>
> I tried to implement a patch according to the idea. It allows extensions
> to register an entry point of the self-managed daemon processes,
> then postmaster start and stop them according to the normal manner.
>
> [kaigai(at)iwashi patch]$ ps ax | grep postgres
> 27784 pts/0 S 0:00 /usr/local/pgsql/bin/postgres
> 27786 ? Ss 0:00 postgres: writer process
> 27787 ? Ss 0:00 postgres: checkpointer process
> 27788 ? Ss 0:00 postgres: wal writer process
> 27789 ? Ss 0:00 postgres: autovacuum launcher process
> 27790 ? Ss 0:00 postgres: stats collector process
> 27791 ? Ss 0:00 postgres: auth_counter <== (*)
>
> The auth_counter being included in this patch is just an example of
> this functionality. It does not have significant meanings. It just logs
> number of authentication success and fails every intervals.
>
> I'm motivated to define an extra daemon that attach shared memory
> segment of PostgreSQL as a computing server to avoid limitation of
> number of GPU code that we can load concurrently.
>
> Thanks,

I have tested this original version. The patch has a single trivial reject,
after fixing it, it compiled nicely.

After adding shared_preload_libraries='$libdir/auth_counter', the extra
daemon start and stops nicely with pg_ctl start/stop. The auth_counter.c
code is a fine minimalistic example on writing one's own daemon.

Thanks,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

2012/6/29 Boszormenyi Zoltan <zb(at)cybertec(dot)at>:
> 2012-04-25 11:40 keltezéssel, Kohei KaiGai írta:
>
>> 2012/3/10 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>>
>>> On Fri, Mar 9, 2012 at 6:51 PM, Andrew Dunstan <andrew(at)dunslane(dot)net>
>>> wrote:
>>>>
>>>>
>>>> On 03/09/2012 01:40 PM, Robert Haas wrote:
>>>>>
>>>>> On Fri, Mar 9, 2012 at 12:02 PM, David E.
>>>>> Wheeler<david(at)justatheory(dot)com>
>>>>>  wrote:
>>>>>>
>>>>>> On Mar 9, 2012, at 7:55 AM, Merlin Moncure wrote:
>>>>>>>
>>>>>>> 100% agree  (having re-read the thread and Alvaro's idea having sunk
>>>>>>> in).  Being able to set up daemon processes side by side with the
>>>>>>> postmaster would fit the bill nicely.  It's pretty interesting to
>>>>>>> think of all the places you could go with it.
>>>>>>
>>>>>> pgAgent could use it *right now*. I keep forgetting to restart it
>>>>>> after
>>>>>> restarting PostgreSQL and finding after a day or so that no jobs have
>>>>>> run.
>>>>>
>>>>> That can and should be fixed by teaching pgAgent that failing to
>>>>> connect to the server, or getting disconnected, is not a fatal error,
>>>>> but a reason to sleep and retry.
>>>>
>>>>
>>>> Yeah. It's still not entirely clear to me what a postmaster-controlled
>>>> daemon is going to be able to do that an external daemon can't.
>>>
>>> Start and stop at the same time as postmaster, without any pain.
>>>
>>> It's a considerable convenience to be able to design this aspect once
>>> and then have all things linked to the postmaster follow that. It
>>> means people will be able to write code that runs on all OS easily,
>>> without everybody having similar but slightly different code about
>>> starting up, reading parameters, following security rules etc.. Tight
>>> integration, with good usability.
>>>
>> I tried to implement a patch according to the idea. It allows extensions
>> to register an entry point of the self-managed daemon processes,
>> then postmaster start and stop them according to the normal manner.
>>
>> [kaigai(at)iwashi patch]$ ps ax | grep postgres
>> 27784 pts/0    S      0:00 /usr/local/pgsql/bin/postgres
>> 27786 ?        Ss     0:00 postgres: writer process
>> 27787 ?        Ss     0:00 postgres: checkpointer process
>> 27788 ?        Ss     0:00 postgres: wal writer process
>> 27789 ?        Ss     0:00 postgres: autovacuum launcher process
>> 27790 ?        Ss     0:00 postgres: stats collector process
>> 27791 ?        Ss     0:00 postgres: auth_counter              <== (*)
>>
>> The auth_counter being included in this patch is just an example of
>> this functionality. It does not have significant meanings. It just logs
>> number of authentication success and fails every intervals.
>>
>> I'm motivated to define an extra daemon that attach shared memory
>> segment of PostgreSQL as a computing server to avoid limitation of
>> number of GPU code that we can load concurrently.
>>
>> Thanks,
>
>
> I have tested this original version. The patch has a single trivial reject,
> after fixing it, it compiled nicely.
>
> After adding shared_preload_libraries='$libdir/auth_counter', the extra
> daemon start and stops nicely with pg_ctl start/stop. The auth_counter.c
> code is a fine minimalistic example on writing one's own daemon.
>
Thanks for your testing.

According to Simon's comment, I'm waiting for his integration of this patch
with another implementation by him.

The auth_counter is just an proof-of-concept patch, so, it is helpful if you
could provide another use case that can make sense.

Best regards,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

2012-06-29 16:44 keltezéssel, Kohei KaiGai írta:
> 2012/6/29 Boszormenyi Zoltan <zb(at)cybertec(dot)at>:
>> 2012-04-25 11:40 keltezéssel, Kohei KaiGai írta:
>>
>>> 2012/3/10 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>>> On Fri, Mar 9, 2012 at 6:51 PM, Andrew Dunstan <andrew(at)dunslane(dot)net>
>>>> wrote:
>>>>>
>>>>> On 03/09/2012 01:40 PM, Robert Haas wrote:
>>>>>> On Fri, Mar 9, 2012 at 12:02 PM, David E.
>>>>>> Wheeler<david(at)justatheory(dot)com>
>>>>>> wrote:
>>>>>>> On Mar 9, 2012, at 7:55 AM, Merlin Moncure wrote:
>>>>>>>> 100% agree (having re-read the thread and Alvaro's idea having sunk
>>>>>>>> in). Being able to set up daemon processes side by side with the
>>>>>>>> postmaster would fit the bill nicely. It's pretty interesting to
>>>>>>>> think of all the places you could go with it.
>>>>>>> pgAgent could use it *right now*. I keep forgetting to restart it
>>>>>>> after
>>>>>>> restarting PostgreSQL and finding after a day or so that no jobs have
>>>>>>> run.
>>>>>> That can and should be fixed by teaching pgAgent that failing to
>>>>>> connect to the server, or getting disconnected, is not a fatal error,
>>>>>> but a reason to sleep and retry.
>>>>>
>>>>> Yeah. It's still not entirely clear to me what a postmaster-controlled
>>>>> daemon is going to be able to do that an external daemon can't.
>>>> Start and stop at the same time as postmaster, without any pain.
>>>>
>>>> It's a considerable convenience to be able to design this aspect once
>>>> and then have all things linked to the postmaster follow that. It
>>>> means people will be able to write code that runs on all OS easily,
>>>> without everybody having similar but slightly different code about
>>>> starting up, reading parameters, following security rules etc.. Tight
>>>> integration, with good usability.
>>>>
>>> I tried to implement a patch according to the idea. It allows extensions
>>> to register an entry point of the self-managed daemon processes,
>>> then postmaster start and stop them according to the normal manner.
>>>
>>> [kaigai(at)iwashi patch]$ ps ax | grep postgres
>>> 27784 pts/0 S 0:00 /usr/local/pgsql/bin/postgres
>>> 27786 ? Ss 0:00 postgres: writer process
>>> 27787 ? Ss 0:00 postgres: checkpointer process
>>> 27788 ? Ss 0:00 postgres: wal writer process
>>> 27789 ? Ss 0:00 postgres: autovacuum launcher process
>>> 27790 ? Ss 0:00 postgres: stats collector process
>>> 27791 ? Ss 0:00 postgres: auth_counter <== (*)
>>>
>>> The auth_counter being included in this patch is just an example of
>>> this functionality. It does not have significant meanings. It just logs
>>> number of authentication success and fails every intervals.
>>>
>>> I'm motivated to define an extra daemon that attach shared memory
>>> segment of PostgreSQL as a computing server to avoid limitation of
>>> number of GPU code that we can load concurrently.
>>>
>>> Thanks,
>>
>> I have tested this original version. The patch has a single trivial reject,
>> after fixing it, it compiled nicely.
>>
>> After adding shared_preload_libraries='$libdir/auth_counter', the extra
>> daemon start and stops nicely with pg_ctl start/stop. The auth_counter.c
>> code is a fine minimalistic example on writing one's own daemon.
>>
> Thanks for your testing.
>
> According to Simon's comment, I'm waiting for his integration of this patch
> with another implementation by him.
>
> The auth_counter is just an proof-of-concept patch, so, it is helpful if you
> could provide another use case that can make sense.

Well, we have two use cases that are more complex:

- an SQL-driven scheduler, similar to pgAgent, it's generic enough,
we might port it to this scheme and publish it
- a huge volume importer daemon, it was written for a very specific
purpose and for a single client, we cannot publish it.

Both need database connections, the second needs more than one,
so they need to link to the client side libpq, the way it was done for
walreceiver can be done here as well.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

On Fri, Jun 29, 2012 at 9:44 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
> The auth_counter is just an proof-of-concept patch, so, it is helpful if you
> could provide another use case that can make sense.
>

what about pgbouncer?

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

2012/6/21 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On 21 June 2012 19:13, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
>> On Sun, Jun 10, 2012 at 4:15 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> 2012/6/8 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>>
>>>> I have a prototype that has some of these characteristics, so I see
>>>> our work as complementary.
>>>>
>>>> At present, I don't think this patch would be committable in CF1, but
>>>> I'd like to make faster progress with it than that. Do you want to
>>>> work on this more, or would you like me to merge our prototypes into a
>>>> more likely candidate?
>>>>
>>> I'm not favor in duplicate similar efforts. If available, could you merge
>>> some ideas in my patch into your prototypes?
>>>
>>
>> so, we are waiting for a new patch? is it coming from Simon or Kohei?
>
> There is an updated patch coming from me. I thought I would focus on
> review of other work first.
>
Simon, what about the current status of this patch?

Do I have something to help for the integration by the upcoming CF?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:

> We have some use cases for this patch, when can you post
> a new version? I would test and review it.

What use cases do you have in mind?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
>
>> We have some use cases for this patch, when can you post
>> a new version? I would test and review it.
>
> What use cases do you have in mind?
>
I'm motivated with this feature to implement background calculation server
to handle accesses to GPU device; to avoid limitation of number of processes
that can use GPU device simultaneously.

Probably, other folks have their use cases.
For example, Zoltan introduced his use case in the upthread as follows:
> - an SQL-driven scheduler, similar to pgAgent, it's generic enough,
> we might port it to this scheme and publish it
> - a huge volume importer daemon, it was written for a very specific
> purpose and for a single client, we cannot publish it.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Excerpts from Kohei KaiGai's message of mar sep 11 12:46:34 -0300 2012:
> 2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> > Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
> >
> >> We have some use cases for this patch, when can you post
> >> a new version? I would test and review it.
> >
> > What use cases do you have in mind?
> >
> I'm motivated with this feature to implement background calculation server
> to handle accesses to GPU device; to avoid limitation of number of processes
> that can use GPU device simultaneously.

Hmm, okay, so basically a worker would need a couple of LWLocks, a
shared memory area, and not much else? Not a database connection.

> Probably, other folks have their use cases.
> For example, Zoltan introduced his use case in the upthread as follows:
> > - an SQL-driven scheduler, similar to pgAgent, it's generic enough,
> > we might port it to this scheme and publish it

Hm, this would benefit from a direct backend connection to get the
schedule data (SPI interface I guess).

> > - a huge volume importer daemon, it was written for a very specific
> > purpose and for a single client, we cannot publish it.

This one AFAIR requires more than one connection, so a direct data
connection is no good -- hence link libpq like walreceiver.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Excerpts from Kohei KaiGai's message of mar sep 11 12:46:34 -0300 2012:
>> 2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
>> > Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
>> >
>> >> We have some use cases for this patch, when can you post
>> >> a new version? I would test and review it.
>> >
>> > What use cases do you have in mind?
>> >
>> I'm motivated with this feature to implement background calculation server
>> to handle accesses to GPU device; to avoid limitation of number of processes
>> that can use GPU device simultaneously.
>
> Hmm, okay, so basically a worker would need a couple of LWLocks, a
> shared memory area, and not much else? Not a database connection.
>
Right. It needs shared memory area to communicate with each backend
and locking mechanism, but my case does not take database accesses
right now.

>> Probably, other folks have their use cases.
>> For example, Zoltan introduced his use case in the upthread as follows:
>> > - an SQL-driven scheduler, similar to pgAgent, it's generic enough,
>> > we might port it to this scheme and publish it
>
> Hm, this would benefit from a direct backend connection to get the
> schedule data (SPI interface I guess).
>
I also think SPI interface will be first candidate for the daemons that
needs database access. Probably, lower layer interfaces (such as
heap_open and heap_beginscan) are also available if SPI interface
can be used.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Excerpts from Kohei KaiGai's message of mar sep 11 13:25:18 -0300 2012:
> 2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> >> > - an SQL-driven scheduler, similar to pgAgent, it's generic enough,
> >> > we might port it to this scheme and publish it
> >
> > Hm, this would benefit from a direct backend connection to get the
> > schedule data (SPI interface I guess).
> >
> I also think SPI interface will be first candidate for the daemons that
> needs database access. Probably, lower layer interfaces (such as
> heap_open and heap_beginscan) are also available if SPI interface
> can be used.

Well, as soon as you have a database connection on which you can run
SPI, you need a lot of stuff to ensure your transaction is aborted in
case of trouble and so on. At that point you can do direct access as
well.

I think it would be a good design to provide different cleanup routes
for the different use cases: for those that need database connections we
nede to go through AbortOutOfAnyTransaction() or something similar; for
others we can probably get away with much less than that. Not 100% sure
at this point.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2012-09-11 17:58 keltezéssel, Alvaro Herrera írta:
> Excerpts from Kohei KaiGai's message of mar sep 11 12:46:34 -0300 2012:
>> 2012/9/11 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
>>> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
>>>
>>>> We have some use cases for this patch, when can you post
>>>> a new version? I would test and review it.
>>> What use cases do you have in mind?
>>>
>> I'm motivated with this feature to implement background calculation server
>> to handle accesses to GPU device; to avoid limitation of number of processes
>> that can use GPU device simultaneously.
> Hmm, okay, so basically a worker would need a couple of LWLocks, a
> shared memory area, and not much else? Not a database connection.
>
>> Probably, other folks have their use cases.
>> For example, Zoltan introduced his use case in the upthread as follows:
>>> - an SQL-driven scheduler, similar to pgAgent, it's generic enough,
>>> we might port it to this scheme and publish it
> Hm, this would benefit from a direct backend connection to get the
> schedule data (SPI interface I guess).

Indeed. And the advantage is that the scheduler's lifetime is exactly
the server's lifetime so there is no need to try reconnecting as soon
as the server goes away and wait until it comes back.

>>> - a huge volume importer daemon, it was written for a very specific
>>> purpose and for a single client, we cannot publish it.
> This one AFAIR requires more than one connection, so a direct data
> connection is no good -- hence link libpq like walreceiver.

Yes.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:

>> We have some use cases for this patch, when can you post
>> a new version? I would test and review it.

> What use cases do you have in mind?

Wouldn't it be helpful for some features like parallel query in future?

With Regards,
Amit Kapila.

Excerpts from Amit Kapila's message of mié sep 12 00:30:40 -0300 2012:
> On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
>
> >> We have some use cases for this patch, when can you post
> >> a new version? I would test and review it.
>
> > What use cases do you have in mind?
>
> Wouldn't it be helpful for some features like parallel query in future?

Maybe, maybe not -- but I don't think it's a wise idea to include too
much complexity just to support such a thing. I would vote to leave
that out for now and just concentrate on getting external stuff working.
There are enough use cases that it's already looking nontrivial.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 12 September 2012 04:30, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
> On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012:
>
>>> We have some use cases for this patch, when can you post
>>> a new version? I would test and review it.
>
>> What use cases do you have in mind?
>
> Wouldn't it be helpful for some features like parallel query in future?

Trying to solve that is what delayed this patch, so the scope of this
needs to be "permanent daemons" rather than dynamically spawned worker
tasks.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Thursday, September 20, 2012 1:44 AM Simon Riggs wrote:
On 12 September 2012 04:30, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
> On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400
2012:
>
>>>> We have some use cases for this patch, when can you post
>>>> a new version? I would test and review it.
>
>>> What use cases do you have in mind?
>
>> Wouldn't it be helpful for some features like parallel query in future?

> Trying to solve that is what delayed this patch, so the scope of this
> needs to be "permanent daemons" rather than dynamically spawned worker
> tasks.

Why can't worker tasks be also permanent, which can be controlled through
configuration. What I mean to say is that if user has need for parallel
operations
he can configure max_worker_tasks and those many worker tasks will get
created.
Otherwise without having such parameter, we might not be sure whether such
deamons
will be of use to database users who don't need any background ops.

The dynamism will come in to scene when we need to allocate such daemons
for particular ops(query), because
might be operation need certain number of worker tasks, but no such task
is available, at that time it need
to be decided whether to spawn a new task or change the parallelism in
operation such that it can be executed with
available number of worker tasks.

Although I understood and agree that such "permanent daemons" will be
useful for usecases other than
parallel operations. However my thinking is that having "permanent
daemons" can also be useful for parallel ops.
So even currently it is getting developed for certain usecases but the
overall idea can be enhanced to have them for
parallel ops as well.

With Regards,
Amit Kapila.

Excerpts from Amit Kapila's message of jue sep 20 02:10:23 -0300 2012:

> Why can't worker tasks be also permanent, which can be controlled through
> configuration. What I mean to say is that if user has need for parallel
> operations
> he can configure max_worker_tasks and those many worker tasks will get
> created.
> Otherwise without having such parameter, we might not be sure whether such
> deamons
> will be of use to database users who don't need any background ops.
>
> The dynamism will come in to scene when we need to allocate such daemons
> for particular ops(query), because
> might be operation need certain number of worker tasks, but no such task
> is available, at that time it need
> to be decided whether to spawn a new task or change the parallelism in
> operation such that it can be executed with
> available number of worker tasks.

Well, there is a difficulty here which is that the number of processes
connected to databases must be configured during postmaster start
(because it determines the size of certain shared memory structs). So
you cannot just spawn more tasks if all max_worker_tasks are busy.
(This is a problem only for those workers that want to be connected as
backends. Those that want libpq connections do not need this and are
easier to handle.)

The design we're currently discussing actually does not require a new
GUC parameter at all. This is why: since the workers must be registered
before postmaster start anyway (in the _PG_init function of a module
that's listed in shared_preload_libraries) then we have to run a
registering function during postmaster start. So postmaster can simply
count how many it needs and size those structs from there. Workers that
do not need a backend-like connection don't have a shmem sizing
requirement so are not important for this. Configuration is thus
simplified.

BTW I am working on this patch and I think I have a workable design in
place; I just couldn't get the code done before the start of this
commitfest. (I am missing handling the EXEC_BACKEND case though, but I
will not even look into that until the basic Unix case is working).

One thing I am not going to look into is how is this new capability be
used for parallel query. I feel we have enough use cases without it,
that we can develop a fairly powerful feature. After that is done and
proven (and committed) we can look into how we can use this to implement
these short-lived workers for stuff such as parallel query.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2012/9/20 Amit Kapila <amit(dot)kapila(at)huawei(dot)com>:
> On Thursday, September 20, 2012 1:44 AM Simon Riggs wrote:
> On 12 September 2012 04:30, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
>> On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
>> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400
> 2012:
>>
>>>>> We have some use cases for this patch, when can you post
>>>>> a new version? I would test and review it.
>>
>>>> What use cases do you have in mind?
>>
>>> Wouldn't it be helpful for some features like parallel query in future?
>
>> Trying to solve that is what delayed this patch, so the scope of this
>> needs to be "permanent daemons" rather than dynamically spawned worker
>> tasks.
>
> Why can't worker tasks be also permanent, which can be controlled through
> configuration. What I mean to say is that if user has need for parallel
> operations
> he can configure max_worker_tasks and those many worker tasks will get
> created.
> Otherwise without having such parameter, we might not be sure whether such
> deamons
> will be of use to database users who don't need any background ops.
>
> The dynamism will come in to scene when we need to allocate such daemons
> for particular ops(query), because
> might be operation need certain number of worker tasks, but no such task
> is available, at that time it need
> to be decided whether to spawn a new task or change the parallelism in
> operation such that it can be executed with
> available number of worker tasks.
>
> Although I understood and agree that such "permanent daemons" will be
> useful for usecases other than
> parallel operations. However my thinking is that having "permanent
> daemons" can also be useful for parallel ops.
> So even currently it is getting developed for certain usecases but the
> overall idea can be enhanced to have them for
> parallel ops as well.
>
I'm also not sure why "permanent daemons" is more difficult than dynamically
spawned daemons, because I guess all the jobs needed for permanent
daemons are quite similar to what we're now doing to implement bgwriter
or others in postmaster.c, except for it is implemented with loadable modules.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote:
Excerpts from Amit Kapila's message of jue sep 20 02:10:23 -0300 2012:

>> Why can't worker tasks be also permanent, which can be controlled through
>> configuration. What I mean to say is that if user has need for parallel
>> operations
>> he can configure max_worker_tasks and those many worker tasks will get
>> created.
>> Otherwise without having such parameter, we might not be sure whether such
>> deamons
>> will be of use to database users who don't need any background ops.
>
>> The dynamism will come in to scene when we need to allocate such daemons
>> for particular ops(query), because
>> might be operation need certain number of worker tasks, but no such task
>> is available, at that time it need
>> to be decided whether to spawn a new task or change the parallelism in
>> operation such that it can be executed with
>> available number of worker tasks.

> Well, there is a difficulty here which is that the number of processes
> connected to databases must be configured during postmaster start
> (because it determines the size of certain shared memory structs). So
> you cannot just spawn more tasks if all max_worker_tasks are busy.
> (This is a problem only for those workers that want to be connected as
> backends. Those that want libpq connections do not need this and are
> easier to handle.)

Are you telling about shared memory structs that need to be allocated for each worker task?
I am not sure if they can be shared across multiple slaves or will be required for each slave.
However even if that is not possible, other mechanism can be used to get the work done by existing slaves.

If not above then where there is a need of dynamic worker tasks as mentioned by Simon?

> The design we're currently discussing actually does not require a new
> GUC parameter at all. This is why: since the workers must be registered
> before postmaster start anyway (in the _PG_init function of a module
> that's listed in shared_preload_libraries) then we have to run a
>registering function during postmaster start. So postmaster can simply
> count how many it needs and size those structs from there. Workers that
> do not need a backend-like connection don't have a shmem sizing
> requirement so are not important for this. Configuration is thus
> simplified.

> BTW I am working on this patch and I think I have a workable design in
> place; I just couldn't get the code done before the start of this
> commitfest. (I am missing handling the EXEC_BACKEND case though, but I
> will not even look into that until the basic Unix case is working).

> One thing I am not going to look into is how is this new capability be
> used for parallel query. I feel we have enough use cases without it,
> that we can develop a fairly powerful feature. After that is done and
> proven (and committed) we can look into how we can use this to implement
> these short-lived workers for stuff such as parallel query.

Agreed and I also meant to say the same as you are saying.

With Regards,
Amit Kapila.

On Thursday, September 20, 2012 7:35 PM Kohei KaiGai wrote:
2012/9/20 Amit Kapila <amit(dot)kapila(at)huawei(dot)com>:
> On Thursday, September 20, 2012 1:44 AM Simon Riggs wrote:
> On 12 September 2012 04:30, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
>> On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote:
>> Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400
> 2012:
>>
>>>>>> We have some use cases for this patch, when can you post
>>>>>> a new version? I would test and review it.
>>
>>>>> What use cases do you have in mind?
>>
>>>> Wouldn't it be helpful for some features like parallel query in
future?
>
>>> Trying to solve that is what delayed this patch, so the scope of this
>>> needs to be "permanent daemons" rather than dynamically spawned worker
>>> tasks.
>
>> Why can't worker tasks be also permanent, which can be controlled
through
>> configuration. What I mean to say is that if user has need for parallel
>> operations
>> he can configure max_worker_tasks and those many worker tasks will get
>> created.
>> Otherwise without having such parameter, we might not be sure whether
such
>> deamons
>> will be of use to database users who don't need any background ops.
>
>> The dynamism will come in to scene when we need to allocate such
daemons
>> for particular ops(query), because
>> might be operation need certain number of worker tasks, but no such
task
>> is available, at that time it need
>> to be decided whether to spawn a new task or change the parallelism in
>> operation such that it can be executed with
>> available number of worker tasks.
>

> I'm also not sure why "permanent daemons" is more difficult than
dynamically
> spawned daemons,

I think Alvaro and Simon also felt "permanent daemons" is not difficult and
is the right way to go,
that’s why the feature is getting developed on those lines.

With Regards,
Amit Kapila.

Excerpts from Amit Kapila's message of vie sep 21 02:26:49 -0300 2012:
> On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote:

> > Well, there is a difficulty here which is that the number of processes
> > connected to databases must be configured during postmaster start
> > (because it determines the size of certain shared memory structs). So
> > you cannot just spawn more tasks if all max_worker_tasks are busy.
> > (This is a problem only for those workers that want to be connected as
> > backends. Those that want libpq connections do not need this and are
> > easier to handle.)
>
> Are you telling about shared memory structs that need to be allocated for each worker task?
> I am not sure if they can be shared across multiple slaves or will be required for each slave.
> However even if that is not possible, other mechanism can be used to get the work done by existing slaves.

I mean stuff like PGPROC entries and such. Currently, they are
allocated based on max_autovacuum_workers + max_connections +
max_prepared_transactions IIRC. So by following identical reasoning we
would just have to add an hypothetical new max_bgworkers to the mix;
however as I said above, we don't really need that because we can count
the number of registered workers at postmaster start time and use that
to size PGPROC.

Shared memory used by each worker (or by a group of workers) that's not
part of core structs should be allocated by the worker itself via
RequestAddInShmemSpace.

> If not above then where there is a need of dynamic worker tasks as mentioned by Simon?

Well, I think there are many uses for dynamic workers, or short-lived
workers (start, do one thing, stop and not be restarted).

In my design, a worker is always restarted if it stops; otherwise there
is no principled way to know whether it should be running or not (after
a crash, should we restart a registered worker? We don't know whether
it stopped before the crash.) So it seems to me that at least for this
first shot we should consider workers as processes that are going to be
always running as long as postmaster is alive. On a crash, if they have
a backend connection, they are stopped and then restarted.

> > One thing I am not going to look into is how is this new capability be
> > used for parallel query. I feel we have enough use cases without it,
> > that we can develop a fairly powerful feature. After that is done and
> > proven (and committed) we can look into how we can use this to implement
> > these short-lived workers for stuff such as parallel query.
>
> Agreed and I also meant to say the same as you are saying.

Great.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Friday, September 21, 2012 6:50 PM Alvaro Herrera wrote:
Excerpts from Amit Kapila's message of vie sep 21 02:26:49 -0300 2012:
> On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote:

> > > Well, there is a difficulty here which is that the number of processes
> >> connected to databases must be configured during postmaster start
> >> (because it determines the size of certain shared memory structs). So
> >> you cannot just spawn more tasks if all max_worker_tasks are busy.
> >> (This is a problem only for those workers that want to be connected as
> >> backends. Those that want libpq connections do not need this and are
> >> easier to handle.)
>

>> If not above then where there is a need of dynamic worker tasks as mentioned by Simon?

> Well, I think there are many uses for dynamic workers, or short-lived
> workers (start, do one thing, stop and not be restarted).

> In my design, a worker is always restarted if it stops; otherwise there
> is no principled way to know whether it should be running or not (after
> a crash, should we restart a registered worker? We don't know whether
> it stopped before the crash.) So it seems to me that at least for this
> first shot we should consider workers as processes that are going to be
> always running as long as postmaster is alive. On a crash, if they have
> a backend connection, they are stopped and then restarted.

a. Is there a chance that it would have made shared memory inconsitent after crash like by having lock on some structure and crash before releasing it?
If such is case, do we need reinitialize the shared memory as well with worker restart?

b. do these worker tasks be able to take any new jobs, or whatever they are started with they will do only those jobs?

With Regards,
Amit Kapila.

With Regards,
Amit Kapila.

Excerpts from Amit kapila's message of sáb sep 22 01:14:40 -0300 2012:
> On Friday, September 21, 2012 6:50 PM Alvaro Herrera wrote:
> Excerpts from Amit Kapila's message of vie sep 21 02:26:49 -0300 2012:
> > On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote:
>
> > > > Well, there is a difficulty here which is that the number of processes
> > >> connected to databases must be configured during postmaster start
> > >> (because it determines the size of certain shared memory structs). So
> > >> you cannot just spawn more tasks if all max_worker_tasks are busy.
> > >> (This is a problem only for those workers that want to be connected as
> > >> backends. Those that want libpq connections do not need this and are
> > >> easier to handle.)
> >
>
> >> If not above then where there is a need of dynamic worker tasks as mentioned by Simon?
>
> > Well, I think there are many uses for dynamic workers, or short-lived
> > workers (start, do one thing, stop and not be restarted).
>
> > In my design, a worker is always restarted if it stops; otherwise there
> > is no principled way to know whether it should be running or not (after
> > a crash, should we restart a registered worker? We don't know whether
> > it stopped before the crash.) So it seems to me that at least for this
> > first shot we should consider workers as processes that are going to be
> > always running as long as postmaster is alive. On a crash, if they have
> > a backend connection, they are stopped and then restarted.
>
> a. Is there a chance that it would have made shared memory inconsitent after crash like by having lock on some structure and crash before releasing it?
> If such is case, do we need reinitialize the shared memory as well with worker restart?

Any worker that requires access to shared memory will have to be stopped
and restarted on a crash (of any other postmaster child process).
Conversely, if a worker requires shmem access, it will have to cause the
whole system to be stopped/restarted if it crashes in some ugly way.
Same as any current process that's connected to shared memory, I think.

So, to answer your question, yes. We need to take the safe route and
consider that a crashed process might have corrupted shmem. (But if it
dies cleanly, then there is no need for this.)

> b. do these worker tasks be able to take any new jobs, or whatever
> they are started with they will do only those jobs?

Not sure I understand this question. If a worker connects to a
database, it will stay connected to that database until it dies;
changing DBs is not allowed. If you want a worker that connects to
database A, does stuff there, and then connects to database B, it could
connect to A, do its deed, then set up database=B in shared memory and
stop, which will cause postmaster to restart it; next time it starts, it
reads shmem and knows to connect to the other DB.

My code has the ability to connect to no particular database -- what
autovac launcher does (this lets it read shared catalogs). So you could
do useful things like have the first invocation of your worker connect
to that on the first invocation and read pg_database to determine what
DB to connect next, then terminate.

You could also have worker groups commanded by one process: one queen
bee, one or more worker bees. The queen determines what to do, sets
tasklist info in shmem, signals worker bees. While the tasklist is
empty, workers would sleep.

As you can see there are many things that can be done with this.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

> On Monday, September 24, 2012 12:24 AM Alvaro Herrera wrote:
> Excerpts from Amit kapila's message of sáb sep 22 01:14:40 -0300 2012:
> > On Friday, September 21, 2012 6:50 PM Alvaro Herrera wrote:
> > Excerpts from Amit Kapila's message of vie sep 21 02:26:49 -0300
> 2012:
> > > On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote:
> >
>
> You could also have worker groups commanded by one process: one queen
> bee, one or more worker bees. The queen determines what to do, sets
> tasklist info in shmem, signals worker bees. While the tasklist is
> empty, workers would sleep.
>
> As you can see there are many things that can be done with this.

Yes, this really is a good feature which can be used for many different functionalaties.

With Regards,
Amit Kapila.

Excerpts from Kohei KaiGai's message of mié abr 25 06:40:23 -0300 2012:

> I tried to implement a patch according to the idea. It allows extensions
> to register an entry point of the self-managed daemon processes,
> then postmaster start and stop them according to the normal manner.

Here's my attempt at this. This is loosely based on your code, as well
as parts of what Simon sent me privately.

One thing *not* implemented in this patch is EXEC_BACKEND support. I
haven't looked at that at all, but it looks rather involved to
implement. If we agree that what I propose here is a reasonable
interface I will start looking at that.

I ported your auth_counter to my framework. It works in the sense that
it runs, but I observed that it always report zeroes. I didn't debug
that.

Please let me know your thoughts. Some ideas below. (I thought about
putting the text below as src/backend/postmaster/README.bgworker).

A background worker is a secondary process that takes advantage of postmaster
to start, stop, and receive signals during operation (e.g. for recovery
restart and similar conditions). It may optionally connect to a particular
database, like a regular backend; or it might "connect" to no database in
particular, giving it access to shared catalogs, similar to autovacuum
launcher. It may also request attachment to shared memory, which allows it to
acquire lwlocks, etc.

An extension or loadable module can request a bgworker to be loaded by calling
RegisterBackgroundWorker. This receives a function pointer to run when the
bgworker is started; a point in time at which to start it; a pointer to some
opaque data; and SIGHUP and SIGTERM handlers.

RegisterBackgroundWorker can be called multiple times by the same loadable
module; this can be used, for example, to have one worker to connect to each
database.

Once running, a worker can establish a database connection by calling
BackgroundWorkerInitializeConnection. This receives a database name to
connect to (NULL for a shared-catalog-only connection), and a username (NULL
to mean bootstrap superuser).

Registration flags
------------------

The following flags currently exist:

BGWORKER_SHMEM_ACCESS --- this determines that the worker will have access to
shared memory; in particular it'll be able to acquire lwlocks. It will also
be killed in case of child crash, etc.

BGWORKER_BACKEND_DATABASE_CONNECTION --- this allows a worker to request a
database connection. This also determines stop time on postmaster shutdown,
etc. If this flag is passed, BGWORKER_SHMEM_ACCESS must also be passed.
If a worker with such a flag dies with an exit code other than 0/1, it will
cause HandleChildCrash to be called, just like a backend that dies.

XXX --- do we need more flags?

XXX --- the signal timing bits depending on flags are not yet set in stone.

Postmaster behavior
-------------------

Each RegisterBackgroundWorker(BGWORKER_SHMEM_ACCESS) call increases the number
of PGPROC elements required. Such bgworkers can only be registered during
shared_preload_libraries; this is a useful limitation because it means the
total number of PGPROC entries needed can be determined at postmaster start
time, and thus we do not need a new GUC to control the max number of workers
(Whether we want such a limit for other reasons, such as protecting against
broken modules that register thousands of workers, is TBD). XXX not
implemented is the capability that workers that do *not* request shared memory
access can be registered when postmaster is running. It's not clear that this
is really useful.

There are three states in the postmaster state machine upon entering which
bgworkers can be started. 1. PM_INIT (postmaster start); 2. PM_HOT_STANDBY
(consistent state reached); 3. PM_RUN (recovery finished). Workers specify
their start time during registration. A worker cannot request database
connection if the start time is PM_INIT.

XXX It is unclear whether we need to allow for shutdown timing tweaks.

Postmaster coding
-----------------

Postmaster keeps track of background workers in BackgroundWorkerList. Each
list element is a RegisteredBgWorker; this list, and each element, are allocated at
postmaster start time (during RegisterBackgroundWorker) and never destroyed.
Each element has a pointer to a BackgroundWorker element, plus a PID (zero if
the worker is not running), a PMChildSlot, a cancel_key, and possibly a
pointer to a Backend entry (which is also present in the BackendList).

This arrangement makes it easy to
1. find workers that need to be started
2. find workers that need to be signalled in various conditions
3. execute cancel signals (even though the cancel key is presumably hard to
find)

Note that since the Backend entry for a worker is in BackendList, it's
important to search the bgworkers list (CleanupBackgroundWorker) before the
backends list (CleanupBackend) during signalling.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Excerpts from Alvaro Herrera's message of mié sep 26 13:04:34 -0300 2012:
> Excerpts from Kohei KaiGai's message of mié abr 25 06:40:23 -0300 2012:
>
> > I tried to implement a patch according to the idea. It allows extensions
> > to register an entry point of the self-managed daemon processes,
> > then postmaster start and stop them according to the normal manner.
>
> Here's my attempt at this. This is loosely based on your code, as well
> as parts of what Simon sent me privately.

Actually please consider this version instead, in which the support to
connect to a database and run transactions actually works. I have also
added a new sample module (worker_spi) which talks to the server using
the SPI interface.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hi Alvaro,

Let me volunteer for reviewing, of course, but now pgsql_fdw is in my queue...

If some other folks can also volunteer it soon, it is welcome.

2012/9/26 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Excerpts from Alvaro Herrera's message of mié sep 26 13:04:34 -0300 2012:
>> Excerpts from Kohei KaiGai's message of mié abr 25 06:40:23 -0300 2012:
>>
>> > I tried to implement a patch according to the idea. It allows extensions
>> > to register an entry point of the self-managed daemon processes,
>> > then postmaster start and stop them according to the normal manner.
>>
>> Here's my attempt at this. This is loosely based on your code, as well
>> as parts of what Simon sent me privately.
>
> Actually please consider this version instead, in which the support to
> connect to a database and run transactions actually works. I have also
> added a new sample module (worker_spi) which talks to the server using
> the SPI interface.
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Excerpts from Kohei KaiGai's message of jue sep 27 01:06:41 -0300 2012:
> Hi Alvaro,
>
> Let me volunteer for reviewing, of course, but now pgsql_fdw is in my queue...

Sure, thanks -- keep in mind I entered this patch in the next
commitfest, so please do invest more effort in the ones in the
commitfest now in progress.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Here's an updated version of this patch, which also works in
an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
but I don't see anything that would create a portability problem there.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 23.10.2012 00:29, Alvaro Herrera wrote:
> Here's an updated version of this patch, which also works in
> an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
> but I don't see anything that would create a portability problem there.)

Looks good at first glance. Fails on Windows, though:

"C:\postgresql\pgsql.sln" (default target) (1) ->
"C:\postgresql\auth_counter.vcxproj" (default target) (29) ->
(Link target) ->
auth_counter.obj : error LNK2001: unresolved external symbol
UnBlockSig [C:\p
ostgresql\auth_counter.vcxproj]
.\Release\auth_counter\auth_counter.dll : fatal error LNK1120: 1
unresolved externals [C:\postgresql\auth_counter.vcxproj]

"C:\postgresql\pgsql.sln" (default target) (1) ->
"C:\postgresql\worker_spi.vcxproj" (default target) (77) ->
worker_spi.obj : error LNK2001: unresolved external symbol UnBlockSig
[C:\pos
tgresql\worker_spi.vcxproj]
.\Release\worker_spi\worker_spi.dll : fatal error LNK1120: 1
unresolved externals [C:\postgresql\worker_spi.vcxproj]

Marking UnBlockSig with PGDLLIMPORT fixes that. But I wonder if it's a
good idea to leave unblocking signals the responsibility of the user
code in the first place? That seems like the kind of low-level stuff
that you want to hide from extension writers.

Oh, and this needs docs.

- Heikki

Heikki Linnakangas escribió:
> On 23.10.2012 00:29, Alvaro Herrera wrote:
> >Here's an updated version of this patch, which also works in
> >an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
> >but I don't see anything that would create a portability problem there.)
>
> Looks good at first glance.

Thanks.

> Fails on Windows, though:
>
> "C:\postgresql\pgsql.sln" (default target) (1) ->
> "C:\postgresql\auth_counter.vcxproj" (default target) (29) ->
> (Link target) ->
> auth_counter.obj : error LNK2001: unresolved external symbol
> UnBlockSig [C:\p
> ostgresql\auth_counter.vcxproj]
> .\Release\auth_counter\auth_counter.dll : fatal error LNK1120: 1
> unresolved externals [C:\postgresql\auth_counter.vcxproj]

Wow. If that's the only problem it has on Windows, I am extremely
pleased.

Were you able to test the provided test modules? Only now I realise
that they aren't very friendly because there's a hardcoded database name
in there ("alvherre", not the wisest choice I guess), but they should at
least be able to run and not turn into a fork bomb due to being unable
to connect, for instance.

> Marking UnBlockSig with PGDLLIMPORT fixes that. But I wonder if it's
> a good idea to leave unblocking signals the responsibility of the
> user code in the first place? That seems like the kind of low-level
> stuff that you want to hide from extension writers.

Sounds sensible.

I am unsure about the amount of pre-cooked stuff we need to provide.
For instance, do we want some easy way to let the user code run
transactions?

> Oh, and this needs docs.

Hmm, yes it does.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 15.11.2012 17:10, Alvaro Herrera wrote:
> Heikki Linnakangas escribió:
>> On 23.10.2012 00:29, Alvaro Herrera wrote:
>>> Here's an updated version of this patch, which also works in
>>> an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
>>> but I don't see anything that would create a portability problem there.)
>>
>> Looks good at first glance.
>
> Thanks.
>
>> Fails on Windows, though:
>>
>> "C:\postgresql\pgsql.sln" (default target) (1) ->
>> "C:\postgresql\auth_counter.vcxproj" (default target) (29) ->
>> (Link target) ->
>> auth_counter.obj : error LNK2001: unresolved external symbol
>> UnBlockSig [C:\p
>> ostgresql\auth_counter.vcxproj]
>> .\Release\auth_counter\auth_counter.dll : fatal error LNK1120: 1
>> unresolved externals [C:\postgresql\auth_counter.vcxproj]
>
> Wow. If that's the only problem it has on Windows, I am extremely
> pleased.
>
> Were you able to test the provided test modules?

I tested the auth_counter module, seemed to work. It counted all
connections as "successful", though, even when I tried to log in with an
invalid username/database. Didn't try with an invalid password. And I
didn't try worker_spi.

> I am unsure about the amount of pre-cooked stuff we need to provide.
> For instance, do we want some easy way to let the user code run
> transactions?

Would be nice, of course. I guess it depends on how much work would it
be provide that. But we can leave that for later, once the base patch is in.

- Heikki

On 15 November 2012 10:10, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:

> I am unsure about the amount of pre-cooked stuff we need to provide.
> For instance, do we want some easy way to let the user code run
> transactions?

That sounds like a basic requirement. There will be a few
non-transactional bgworkers but most will be just user code, probably
PL/pgSQL.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2012/10/22 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Here's an updated version of this patch, which also works in
> an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
> but I don't see anything that would create a portability problem there.)
>
I also tried to check the latest patch "briefly".
Let me comment on several points randomly.

Once bgworker process got crashed, postmaster tries to restart it about 60
seconds later. My preference is this interval being configurable with initial
registration parameter; that includes "never restart again" option.
Probably, some extensions don't want to restart again on unexpected crash.

Regarding to process restart, this interface allows to specify the timing to
start bgworker using BgWorkerStart_* label. On the other hand,
bgworker_should_start_now() checks correspondence between pmState
and the configured start-time using equal matching. It can make such a
scenarios, for instance, a bgworker launched at PM_INIT state, then it got
crashed. Author expected it shall be restarted, however, pmState was
already progressed to PM_RUN, so nobody can restart this bgworker again.
How about an idea to provide the start time with bitmap? It allows extensions
to specify multiple candidates to launch bgworker.

Stop is one other significant event for bgworker, not only start-up.
This interface has no way to specify when we want to stop bgworker.
Probably, we can offer two options. Currently, bgworkers are simultaneously
terminated with other regular backends. In addition, I want an option to make
sure bgworker being terminated after all the regular backends exit.
It is critical issue for me, because I try to implement parallel
calculation server
with bgworker, thus, it should not be terminated earlier than regular backend.

do_start_bgworker() initializes process state according to normal manner,
then it invokes worker->bgw_main(). Once ERROR is raised inside of the
main routine, the control is backed to the sigsetjmp() on do_start_bgworker(),
then the bgworker is terminated.
I'm not sure whether it is suitable for bgworker that tries to connect database,
because it may raise an error everywhere, such as zero division and so on...
I think it is helpful to provide a utility function in the core; that
handles to abort
transactions in progress, clean-up resources, and so on.
spi_worker invokes BackgroundWorkerInitializeConnection() at the head of
main routine. In a similar fashion, is it available to support a
utility function
that initialize the process as transaction-aware background-worker?
I don't think it is a good manner each extension has to implement its own
sigsetjmp() block and rollback logic. I'd also like to investigate the necessary
logic for transaction abort and resource cleanup here....

Some other misc comments below.

It ensures bgworker must be registered within shared_preload_libraries.
Why not raise an error if someone try to register bgworker later?

How about to move bgw_name field into tail of BackgroundWorker structure?
It makes simplifies the logic in RegisterBackgroundWorker(), if it is
located as:
typedef struct BackgroundWorker
{
int bgw_flags;
BgWorkerStartTime bgw_start_time;
bgworker_main_type bgw_main;
void *bgw_main_arg;
bgworker_sighdlr_type bgw_sighup;
bgworker_sighdlr_type bgw_sigterm;
char bgw_name[1]; <== (*)
} BackgroundWorker;

StartOneBackgroundWorker always scan the BackgroundWorkerList from
the head. Isn't it available to save the current position at static variable?
If someone tries to manage thousand of bgworkers, it makes a busy loop. :(

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai escribió:
> 2012/10/22 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> > Here's an updated version of this patch, which also works in
> > an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
> > but I don't see anything that would create a portability problem there.)
> >
> I also tried to check the latest patch "briefly".
> Let me comment on several points randomly.

Thanks.

> Once bgworker process got crashed, postmaster tries to restart it about 60
> seconds later. My preference is this interval being configurable with initial
> registration parameter; that includes "never restart again" option.
> Probably, some extensions don't want to restart again on unexpected crash.

The main issue with specifying a crash-restart policy, I thought, was
that after a postmaster restart cycle there is no reasonable way to know
whether any bgworker should restart or not: had any particular bgworker
crashed? Remember that any backend crash, and some bgworker crashes,
will cause the postmaster to reinitialize the whole system. So I don't
see that it will be handled consistently; if we offer the option, module
developers might be led into thinking that a daemon that stops is never
to run again, but that's not going to be the case.

But I'm not really wedded to this behavior.

> Stop is one other significant event for bgworker, not only start-up.
> This interface has no way to specify when we want to stop bgworker.
> Probably, we can offer two options. Currently, bgworkers are simultaneously
> terminated with other regular backends. In addition, I want an option to make
> sure bgworker being terminated after all the regular backends exit.
> It is critical issue for me, because I try to implement parallel
> calculation server
> with bgworker, thus, it should not be terminated earlier than regular backend.

I can do that. I considered stop time as well, but couldn't think of
any case in which I would want the worker to persist beyond backends
exit, so I ended up not adding the option. But I think it's reasonably
easy to add.

> Regarding to process restart, this interface allows to specify the timing to
> start bgworker using BgWorkerStart_* label. On the other hand,
> bgworker_should_start_now() checks correspondence between pmState
> and the configured start-time using equal matching. It can make such a
> scenarios, for instance, a bgworker launched at PM_INIT state, then it got
> crashed. Author expected it shall be restarted, however, pmState was
> already progressed to PM_RUN, so nobody can restart this bgworker again.
> How about an idea to provide the start time with bitmap? It allows extensions
> to specify multiple candidates to launch bgworker.

No, I think you must be misreading the code. If a daemon specified
BgWorkerStart_PostmasterStart then it will start whenever pmState is
PM_INIT *or any later state*. This is why the switch has all those
"fall throughs".

> do_start_bgworker() initializes process state according to normal manner,
> then it invokes worker->bgw_main(). Once ERROR is raised inside of the
> main routine, the control is backed to the sigsetjmp() on do_start_bgworker(),
> then the bgworker is terminated.
> I'm not sure whether it is suitable for bgworker that tries to connect database,
> because it may raise an error everywhere, such as zero division and so on...
> I think it is helpful to provide a utility function in the core; that
> handles to abort
> transactions in progress, clean-up resources, and so on.

Yeah, I considered this too --- basically this is the question I posted
elsewhere, "what else do we need to provide for modules"? A sigsetjmp()
standard block or something is probably part of that.

> It ensures bgworker must be registered within shared_preload_libraries.
> Why not raise an error if someone try to register bgworker later?

Hm, sure, we can do that I guess.

> How about to move bgw_name field into tail of BackgroundWorker structure?
> It makes simplifies the logic in RegisterBackgroundWorker(), if it is
> located as:
> typedef struct BackgroundWorker
> {
> int bgw_flags;
> BgWorkerStartTime bgw_start_time;
> bgworker_main_type bgw_main;
> void *bgw_main_arg;
> bgworker_sighdlr_type bgw_sighup;
> bgworker_sighdlr_type bgw_sigterm;
> char bgw_name[1]; <== (*)
> } BackgroundWorker;

Makes sense.

> StartOneBackgroundWorker always scan the BackgroundWorkerList from
> the head. Isn't it available to save the current position at static variable?
> If someone tries to manage thousand of bgworkers, it makes a busy loop. :(

We could try that.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Kohei KaiGai escribió:
> 2012/10/22 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> > Here's an updated version of this patch, which also works in
> > an EXEC_BACKEND environment. (I haven't tested this at all on Windows,
> > but I don't see anything that would create a portability problem there.)
> >
> I also tried to check the latest patch "briefly".
> Let me comment on several points randomly.
>
> Once bgworker process got crashed, postmaster tries to restart it about 60
> seconds later. My preference is this interval being configurable with initial
> registration parameter; that includes "never restart again" option.
> Probably, some extensions don't want to restart again on unexpected crash.

I changed this.

> Stop is one other significant event for bgworker, not only start-up.
> This interface has no way to specify when we want to stop bgworker.
> Probably, we can offer two options. Currently, bgworkers are simultaneously
> terminated with other regular backends. In addition, I want an option to make
> sure bgworker being terminated after all the regular backends exit.
> It is critical issue for me, because I try to implement parallel
> calculation server
> with bgworker, thus, it should not be terminated earlier than regular backend.

I am not really sure about this. Each new behavior we want to propose
requires careful attention, because we need to change postmaster's
shutdown sequence in pmdie(), and also reaper() and
PostmasterStateMachine(). After looking into it, I am hesitant to
change this too much unless we can reach a very detailed agreement of
exactly what we want to happen.

Also note that there are two types of workers: those that require a
database connection, and those that do not. The former are signalled
via SignalSomeChildren(BACKEND_TYPE_BGWORKER); the latter are signalled
via SignalUnconnectedWorker(). Would this distinction be enough for
you?

Delivering more precise shutdown signals is tricky; we would have to
abandon SignalSomeChildren() and code something new to scan the workers
list checking the stop time for each one. (Except in emergency
situations, of course, in which we would continue to rely on
SignalSomeChildren).

> How about to move bgw_name field into tail of BackgroundWorker structure?
> It makes simplifies the logic in RegisterBackgroundWorker(), if it is
> located as:
> typedef struct BackgroundWorker
> {
> int bgw_flags;
> BgWorkerStartTime bgw_start_time;
> bgworker_main_type bgw_main;
> void *bgw_main_arg;
> bgworker_sighdlr_type bgw_sighup;
> bgworker_sighdlr_type bgw_sigterm;
> char bgw_name[1]; <== (*)
> } BackgroundWorker;

This doesn't work; or rather, it works and it makes
RegisterBackgroundWorker() code somewhat nicer, but according to Larry
Wall's Conservation of Cruft Principle, the end result is that the
module code to register a new worker is a lot messier; it can no longer
use a struct in the stack, but requires malloc() or similar. I don't
see that this is a win overall.

> StartOneBackgroundWorker always scan the BackgroundWorkerList from
> the head. Isn't it available to save the current position at static variable?
> If someone tries to manage thousand of bgworkers, it makes a busy loop. :(

Seems messy; we would have to get into the guts of slist_foreach (unroll
the macro and make the iterator static). I prefer not to go that path,
at least not for now.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Kohei KaiGai escribi:
>> StartOneBackgroundWorker always scan the BackgroundWorkerList from
>> the head. Isn't it available to save the current position at static variable?
>> If someone tries to manage thousand of bgworkers, it makes a busy loop. :(

> Seems messy; we would have to get into the guts of slist_foreach (unroll
> the macro and make the iterator static). I prefer not to go that path,
> at least not for now.

Thousands of bgworkers seems like a pretty unsupportable scenario anyway
--- it'd presumably have most of the same problems as thousands of
backends.

regards, tom lane

FWIW I have pushed this to github; see
https://github.com/alvherre/postgres/compare/bgworker

It's also attached.

The UnBlockSig stuff is the main stumbling block as I see it because it
precludes compilation on Windows. Maybe we should fix that by providing
another function that the module is to call after initialization is done
and before it gets ready to work ... but having a function that only
calls PG_SETMASK() feels somewhat useless to me; and I don't see what
else we could do in there.

Maybe I could also add a stop time enum, with a single value, so that in
the future we can add more to tweak things without having to change
RegisterBackgroundWorker's signature.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera escribió:
> FWIW I have pushed this to github; see
> https://github.com/alvherre/postgres/compare/bgworker
>
> It's also attached.
>
> The UnBlockSig stuff is the main stumbling block as I see it because it
> precludes compilation on Windows. Maybe we should fix that by providing
> another function that the module is to call after initialization is done
> and before it gets ready to work ... but having a function that only
> calls PG_SETMASK() feels somewhat useless to me; and I don't see what
> else we could do in there.

I cleaned up some more stuff and here's another version. In particular
I added wrapper functions to block and unblock signals, so that this
doesn't need exported UnBlockSig.

I also changed ServerLoop so that it sleeps until a crashed bgworker
needs to be restarted -- if a worker terminates, and it has requested
(say) 2s restart time, don't have it wait until the full 60s postmaster
sleep time has elapsed.

The sample code has been propped up somewhat, too.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 21.11.2012 23:29, Alvaro Herrera wrote:
> Alvaro Herrera escribió:
>> FWIW I have pushed this to github; see
>> https://github.com/alvherre/postgres/compare/bgworker
>>
>> It's also attached.
>>
>> The UnBlockSig stuff is the main stumbling block as I see it because it
>> precludes compilation on Windows. Maybe we should fix that by providing
>> another function that the module is to call after initialization is done
>> and before it gets ready to work ... but having a function that only
>> calls PG_SETMASK() feels somewhat useless to me; and I don't see what
>> else we could do in there.
>
> I cleaned up some more stuff and here's another version. In particular
> I added wrapper functions to block and unblock signals, so that this
> doesn't need exported UnBlockSig.

Could you just unblock the signals before calling into the background
worker's main() function?

- Heikki

Heikki Linnakangas escribió:
> On 21.11.2012 23:29, Alvaro Herrera wrote:
> >Alvaro Herrera escribió:
> >>FWIW I have pushed this to github; see
> >>https://github.com/alvherre/postgres/compare/bgworker
> >>
> >>It's also attached.
> >>
> >>The UnBlockSig stuff is the main stumbling block as I see it because it
> >>precludes compilation on Windows. Maybe we should fix that by providing
> >>another function that the module is to call after initialization is done
> >>and before it gets ready to work ... but having a function that only
> >>calls PG_SETMASK() feels somewhat useless to me; and I don't see what
> >>else we could do in there.
> >
> >I cleaned up some more stuff and here's another version. In particular
> >I added wrapper functions to block and unblock signals, so that this
> >doesn't need exported UnBlockSig.
>
> Could you just unblock the signals before calling into the
> background worker's main() function?

Yes, but what if a daemon wants to block/unblock signals later?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 22.11.2012 19:18, Alvaro Herrera wrote:
> Heikki Linnakangas escribió:
>> On 21.11.2012 23:29, Alvaro Herrera wrote:
>>> Alvaro Herrera escribió:
>>>> The UnBlockSig stuff is the main stumbling block as I see it because it
>>>> precludes compilation on Windows. Maybe we should fix that by providing
>>>> another function that the module is to call after initialization is done
>>>> and before it gets ready to work ... but having a function that only
>>>> calls PG_SETMASK() feels somewhat useless to me; and I don't see what
>>>> else we could do in there.
>>>
>>> I cleaned up some more stuff and here's another version. In particular
>>> I added wrapper functions to block and unblock signals, so that this
>>> doesn't need exported UnBlockSig.
>>
>> Could you just unblock the signals before calling into the
>> background worker's main() function?
>
> Yes, but what if a daemon wants to block/unblock signals later?

Ok. Can you think of an example of a daemon that would like to do that?

Grepping the backend for "BlockSig", the only thing it seems to be
currenlty used for is to block nested signals in the SIGQUIT handler
(see bg_quickdie() for an example). The patch provides a built-in
SIGQUIT handler for the background workers, so I don't think you need
BlockSig for that. Or do you envision that it would be OK for a
background worker to replace the SIGQUIT handler with a custom one?

Even if we provide the BackgroundWorkerBlock/UnblockSignals() functions,
I think it would still make sense to unblock the signals before calling
the bgworker's main loop. One less thing for the background worker to
worry about that way. Or are there some operations that can't be done
safely after unblocking the signals? Also, I note that some worker
processes call sigdelset(&BlockSig, SIGQUITE); that remains impossible
to do in a background worker on Windows, the
BackgroundWorkerBlock/UnblockSignals() wrapper functions don't help with
that.

Some documentation on what a worker is allowed to do would be helpful here..

- Heikki

Heikki Linnakangas escribió:
> On 22.11.2012 19:18, Alvaro Herrera wrote:
> >Heikki Linnakangas escribió:
> >>On 21.11.2012 23:29, Alvaro Herrera wrote:
> >>>Alvaro Herrera escribió:
> >>>>The UnBlockSig stuff is the main stumbling block as I see it because it
> >>>>precludes compilation on Windows. Maybe we should fix that by providing
> >>>>another function that the module is to call after initialization is done
> >>>>and before it gets ready to work ... but having a function that only
> >>>>calls PG_SETMASK() feels somewhat useless to me; and I don't see what
> >>>>else we could do in there.
> >>>
> >>>I cleaned up some more stuff and here's another version. In particular
> >>>I added wrapper functions to block and unblock signals, so that this
> >>>doesn't need exported UnBlockSig.
> >>
> >>Could you just unblock the signals before calling into the
> >>background worker's main() function?
> >
> >Yes, but what if a daemon wants to block/unblock signals later?
>
> Ok. Can you think of an example of a daemon that would like to do that?

Not really, but I don't know what crazy stuff people might be able to
come up with. Somebody was talking about using a worker to do parallel
computation (of queries?) using FPUs, or something along those lines. I
don't have enough background on that sort of thing but it wouldn't
surprise me that they wanted to block signals for a while when the
daemons are busy talking to the FPU, say.

> Grepping the backend for "BlockSig", the only thing it seems to be
> currenlty used for is to block nested signals in the SIGQUIT handler
> (see bg_quickdie() for an example). The patch provides a built-in
> SIGQUIT handler for the background workers, so I don't think you
> need BlockSig for that. Or do you envision that it would be OK for a
> background worker to replace the SIGQUIT handler with a custom one?

I wasn't really considering that the SIGQUIT handler would be replaced.
Not really certain that the SIGTERM handler needs to fiddle with the
sigmask ...

> Even if we provide the BackgroundWorkerBlock/UnblockSignals()
> functions, I think it would still make sense to unblock the signals
> before calling the bgworker's main loop. One less thing for the
> background worker to worry about that way. Or are there some
> operations that can't be done safely after unblocking the signals?

Yes, that's probably a good idea. I don't see anything that would need
to run with signals blocked in the supplied sample code (but then they
are pretty simplistic).

> Also, I note that some worker processes call sigdelset(&BlockSig,
> SIGQUITE); that remains impossible to do in a background worker on
> Windows, the BackgroundWorkerBlock/UnblockSignals() wrapper
> functions don't help with that.

Hmm. Not really sure about that. Maybe we should keep SIGQUIT
unblocked at all times, so postmaster.c needs to remove it from BlockSig
before invoking the bgworker's main function.

The path of least resistance seems to be to export BlockSig and
UnBlockSig, but I hesitate to do it.

> Some documentation on what a worker is allowed to do would be helpful
> here..

I will see about it.

If the bgworker developer gets really tense about this stuff (or
anything at all, really), they can create a completely new sigmask and
do sigaddset() etc. Since this is all C code, we cannot keep them from
doing anything, really; I think what we need to provide here is just a
framework to ease development of simple cases.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> If the bgworker developer gets really tense about this stuff (or
> anything at all, really), they can create a completely new sigmask and
> do sigaddset() etc. Since this is all C code, we cannot keep them from
> doing anything, really; I think what we need to provide here is just a
> framework to ease development of simple cases.

An important point here is that if a bgworker does need to do its own
signal manipulation --- for example, installing custom signal handlers
--- it would be absolutely catastrophic for us to unblock signals before
reaching worker-specific code; signals might arrive before the process
had a chance to fix their handling. So I'm against Heikki's auto-unblock
proposal.

regards, tom lane

2012/11/21 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Alvaro Herrera escribió:
>> FWIW I have pushed this to github; see
>> https://github.com/alvherre/postgres/compare/bgworker
>>
>> It's also attached.
>>
>> The UnBlockSig stuff is the main stumbling block as I see it because it
>> precludes compilation on Windows. Maybe we should fix that by providing
>> another function that the module is to call after initialization is done
>> and before it gets ready to work ... but having a function that only
>> calls PG_SETMASK() feels somewhat useless to me; and I don't see what
>> else we could do in there.
>
> I cleaned up some more stuff and here's another version. In particular
> I added wrapper functions to block and unblock signals, so that this
> doesn't need exported UnBlockSig.
>
> I also changed ServerLoop so that it sleeps until a crashed bgworker
> needs to be restarted -- if a worker terminates, and it has requested
> (say) 2s restart time, don't have it wait until the full 60s postmaster
> sleep time has elapsed.
>
> The sample code has been propped up somewhat, too.
>
I checked the v7 patch.

I didn't find out some major problems being remained in this version.
Even though the timing to unblock signals is still under discussion,
it seems to me BackgroundWorkerUnblockSignals() is a reasonable
solution. At least, no tangible requirement to override signal handlers
except for SIGTERM & SIGINT supported by framework.

Some minor comments here:

If we provide background worker a support routine of sigsetjmp()
block with transaction rollback, does it make sense to mimic the
block in PostgresMain()? It reports the raised error and calls
AbortCurrentTransaction() to release overall resources.

How about to drop auth_counter module? I don't think we need
to have two different example modules in contrib.
My preference is worker_spi rather than auth_counter, because
it also provides example to handle transactions.

At SubPostmasterMain(), it checks whether argv[1] has
"--forkbgworker" using strncmp() on the first 14 byte. It should include
the "=" character to be placed on 15th bytes, to avoid unexpected
match.

Also, "cookie" is picked up using atoi(). In case when "--forkbgworker="
takes non-numerical characters, atoi() returns 0. Even though it might
be paranoia, the initial value of BackgroundWorkerCookie should be
1, in spite of 0.

At BackgroundWorkerInitializeConnection(),
+ /* XXX is this the right errcode? */
+ if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
+ ereport(FATAL,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("database connection requirement not indicated
during registration")));

It only happen when extension calls this function under incorrect
performing mode. So, it is a situation of Assert, not ereport.

So, I'd like to hand over committers this patch near future.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>