Обсуждение: Re: [COMMITTERS] pgsql: Allow background workers to be started dynamically.

On Fri, Jul 19, 2013 at 7:24 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> The changes here make it impossible to write a bgworker which properly
> works in 9.3 and 9.4. Was that intended? If so, the commit message
> should mention the compatibility break...

Yeah, sorry, I probably should have mentioned that.  The structure
needs to be fixed size for us to store it in shared memory.

> If it was intended I propose changing the signature for 9.3 as
> well. There's just no point in releasing 9.3 when we already know which
> trivial but breaking change will be required for 9.4

I think that would be a good idea.  And I'd also propose getting rid
of bgw_sighup and bgw_sigterm in both branches, while we're at it.
AFAICT, they don't add any functionality, and they're basically
unusable for dynamically started background workers.  Probably better
not to get people to used to using them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jul 19, 2013 at 1:23 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jul 19, 2013 at 7:24 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> The changes here make it impossible to write a bgworker which properly
>> works in 9.3 and 9.4. Was that intended? If so, the commit message
>> should mention the compatibility break...
>
> Yeah, sorry, I probably should have mentioned that.  The structure
> needs to be fixed size for us to store it in shared memory.
>
>> If it was intended I propose changing the signature for 9.3 as
>> well. There's just no point in releasing 9.3 when we already know which
>> trivial but breaking change will be required for 9.4
>
> I think that would be a good idea.  And I'd also propose getting rid
> of bgw_sighup and bgw_sigterm in both branches, while we're at it.
> AFAICT, they don't add any functionality, and they're basically
> unusable for dynamically started background workers.  Probably better
> not to get people to used to using them.

+1. Much better to take that pain now, before we have made a production release.


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

Hi,

On 2013-07-19 08:23:25 -0400, Robert Haas wrote:
> On Fri, Jul 19, 2013 at 7:24 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> > The changes here make it impossible to write a bgworker which properly
> > works in 9.3 and 9.4. Was that intended? If so, the commit message
> > should mention the compatibility break...
>
> Yeah, sorry, I probably should have mentioned that.  The structure
> needs to be fixed size for us to store it in shared memory.

> > If it was intended I propose changing the signature for 9.3 as
> > well. There's just no point in releasing 9.3 when we already know which
> > trivial but breaking change will be required for 9.4
>
> I think that would be a good idea.

> And I'd also propose getting rid
> of bgw_sighup and bgw_sigterm in both branches, while we're at it.
> AFAICT, they don't add any functionality, and they're basically
> unusable for dynamically started background workers.  Probably better
> not to get people to used to using them.

I don't have a problem with getting rid of those, it's easy enough to
register them inside the worker and it's safe since we start with
blocked signals. I seem to remember some discussion about why they were
added but I can't find a reference anymore. Alvaro, do you remember?

Greetings,

Andres Freund

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

Andres Freund escribió:

> On 2013-07-19 08:23:25 -0400, Robert Haas wrote:

> > And I'd also propose getting rid
> > of bgw_sighup and bgw_sigterm in both branches, while we're at it.
> > AFAICT, they don't add any functionality, and they're basically
> > unusable for dynamically started background workers.  Probably better
> > not to get people to used to using them.
> 
> I don't have a problem with getting rid of those, it's easy enough to
> register them inside the worker and it's safe since we start with
> blocked signals. I seem to remember some discussion about why they were
> added but I can't find a reference anymore. Alvaro, do you remember?

I left them there because it was easy; but they were absolutely
necessary only until we decided that we would start the worker's main
function with signals blocked.  I don't think there's any serious reason
not to remove them now.

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

On Sat, Jul 20, 2013 at 12:39 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
>> I don't have a problem with getting rid of those, it's easy enough to
>> register them inside the worker and it's safe since we start with
>> blocked signals. I seem to remember some discussion about why they were
>> added but I can't find a reference anymore. Alvaro, do you remember?
>
> I left them there because it was easy; but they were absolutely
> necessary only until we decided that we would start the worker's main
> function with signals blocked.  I don't think there's any serious reason
> not to remove them now.

All right, done.  FWIW, I think starting the worker's main with
signals blocked was definitely the right decision.

I think we have consensus to back-patch the other API changes as well.I'll work up a patch for that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Jul 22, 2013 at 3:16 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> I think we have consensus to back-patch the other API changes as well.
>  I'll work up a patch for that.

Pushed that as well.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company