Thread

1. fix bgworkers in EXEC_BACKEND

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2012-12-27T17:15:27Z

   I committed background workers three weeks ago, claiming it worked on
   EXEC_BACKEND, and shortly thereafter I discovered that it didn't.  I
   noticed that the problem is the kludge to cause postmaster and children
   to recompute MaxBackends after shared_preload_libraries is processed; so
   the minimal fix is to duplicate this bit, from PostmasterMain() into
   SubPostmasterMain():
   
   @@ -4443,6 +4443,17 @@ SubPostmasterMain(int argc, char *argv[])
        */
       process_shared_preload_libraries();
    
   +   /*
   +    * If loadable modules have added background workers, MaxBackends needs to
   +    * be updated.  Do so now by forcing a no-op update of max_connections.
   +    * XXX This is a pretty ugly way to do it, but it doesn't seem worth
   +    * introducing a new entry point in guc.c to do it in a cleaner fashion.
   +    */
   +   if (GetNumShmemAttachedBgworkers() > 0)
   +       SetConfigOption("max_connections",
   +                       GetConfigOption("max_connections", false, false),
   +                       PGC_POSTMASTER, PGC_S_OVERRIDE);
   
   I considered this pretty ugly when I first wrote it, and as the comment
   says I tried to add something to guc.c to make it cleaner, but it was
   even uglier.
   
   So I now came up with a completely different idea: how about making
   MaxBackends a macro, i.e.
   
   +#define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \
   +                    GetNumShmemAttachedBgworkers())
   
   so that instead of having guc.c recompute it, each caller that needs to
   value obtains it up to date all the time?  This additionally means that
   assign_maxconnections and assign_autovacuum_max_workers go away (only
   the check routines remain).  Patch attached.
   
   The one problem I see as serious with this approach is that it'd be
   moderately expensive (i.e. not just fetch a value from memory) to
   compute the value because it requires a walk of the registered workers
   list.  For most callers this wouldn't be a problem because it's just
   during shmem sizing/creation; but there are places such as multixact.c
   and async.c that use it routinely, so it's likely that we need to cache
   the value somehow.  It seems relatively straightforward though.
   
   I'd like to hear opinions on just staying with the IMO ugly minimal fix,
   or pursue instead making MaxBackends a macro plus some sort of cache to
   avoid repeated computation.
   
   -- 
   Álvaro Herrera                http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services
   

2. Re: fix bgworkers in EXEC_BACKEND

   Magnus Hagander <magnus@hagander.net> — 2012-12-27T17:22:56Z

   On Thu, Dec 27, 2012 at 6:15 PM, Alvaro Herrera
   <alvherre@2ndquadrant.com> wrote:
   > I committed background workers three weeks ago, claiming it worked on
   > EXEC_BACKEND, and shortly thereafter I discovered that it didn't.  I
   > noticed that the problem is the kludge to cause postmaster and children
   > to recompute MaxBackends after shared_preload_libraries is processed; so
   > the minimal fix is to duplicate this bit, from PostmasterMain() into
   > SubPostmasterMain():
   >
   > @@ -4443,6 +4443,17 @@ SubPostmasterMain(int argc, char *argv[])
   >      */
   >     process_shared_preload_libraries();
   >
   > +   /*
   > +    * If loadable modules have added background workers, MaxBackends needs to
   > +    * be updated.  Do so now by forcing a no-op update of max_connections.
   > +    * XXX This is a pretty ugly way to do it, but it doesn't seem worth
   > +    * introducing a new entry point in guc.c to do it in a cleaner fashion.
   > +    */
   > +   if (GetNumShmemAttachedBgworkers() > 0)
   > +       SetConfigOption("max_connections",
   > +                       GetConfigOption("max_connections", false, false),
   > +                       PGC_POSTMASTER, PGC_S_OVERRIDE);
   >
   > I considered this pretty ugly when I first wrote it, and as the comment
   > says I tried to add something to guc.c to make it cleaner, but it was
   > even uglier.
   
   Isn't that version going to make the source field in pg_settings for
   max_connections show override whenever you are using background
   workers? Thus breaking things like the fields to tell you which config
   file and line it's on?
   
   
   > So I now came up with a completely different idea: how about making
   > MaxBackends a macro, i.e.
   >
   > +#define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \
   > +                    GetNumShmemAttachedBgworkers())
   >
   > so that instead of having guc.c recompute it, each caller that needs to
   > value obtains it up to date all the time?  This additionally means that
   > assign_maxconnections and assign_autovacuum_max_workers go away (only
   > the check routines remain).  Patch attached.
   
   That seems neater.
   
   
   > The one problem I see as serious with this approach is that it'd be
   > moderately expensive (i.e. not just fetch a value from memory) to
   > compute the value because it requires a walk of the registered workers
   > list.  For most callers this wouldn't be a problem because it's just
   > during shmem sizing/creation; but there are places such as multixact.c
   > and async.c that use it routinely, so it's likely that we need to cache
   > the value somehow.  It seems relatively straightforward though.
   >
   > I'd like to hear opinions on just staying with the IMO ugly minimal fix,
   > or pursue instead making MaxBackends a macro plus some sort of cache to
   > avoid repeated computation.
   
   If my understanding per above is correct, then here's a +1 for the
   non-ugly non-minimal fix.
   
   
   --
    Magnus Hagander
    Me: http://www.hagander.net/
    Work: http://www.redpill-linpro.com/
   
   
   

3. Re: fix bgworkers in EXEC_BACKEND

   Heikki Linnakangas <hlinnakangas@vmware.com> — 2012-12-27T18:06:07Z

   On 27.12.2012 19:15, Alvaro Herrera wrote:
   > I committed background workers three weeks ago, claiming it worked on
   > EXEC_BACKEND, and shortly thereafter I discovered that it didn't.  I
   > noticed that the problem is the kludge to cause postmaster and children
   > to recompute MaxBackends after shared_preload_libraries is processed; so
   > the minimal fix is to duplicate this bit, from PostmasterMain() into
   > SubPostmasterMain():
   >
   > @@ -4443,6 +4443,17 @@ SubPostmasterMain(int argc, char *argv[])
   >       */
   >      process_shared_preload_libraries();
   >
   > +   /*
   > +    * If loadable modules have added background workers, MaxBackends needs to
   > +    * be updated.  Do so now by forcing a no-op update of max_connections.
   > +    * XXX This is a pretty ugly way to do it, but it doesn't seem worth
   > +    * introducing a new entry point in guc.c to do it in a cleaner fashion.
   > +    */
   > +   if (GetNumShmemAttachedBgworkers()>  0)
   > +       SetConfigOption("max_connections",
   > +                       GetConfigOption("max_connections", false, false),
   > +                       PGC_POSTMASTER, PGC_S_OVERRIDE);
   >
   > I considered this pretty ugly when I first wrote it, and as the comment
   > says I tried to add something to guc.c to make it cleaner, but it was
   > even uglier.
   
   Might be cleaner to directly assign the correct value to MaxBackends 
   above, ie. "MaxBackends =  MaxConnections + newval + 1 + 
   GetNumShmemAttachedBgworkers()". With a comment to remind that it needs 
   to be kept in sync with the other places where that calculation is done, 
   in guc.c. Or put that calculation in a new function and call it above 
   and in guc.c.
   
   Thinking about this some more, it might be cleaner to move the 
   responsibility of setting MaxBackends out of guc.c, into postmaster.c. 
   The guc machinery would set max_connections and autovacuum_max_workers 
   as usual, but not try to set MaxBackends. After reading the config file 
   in postmaster.c, calculate MaxBackends.
   
   This would have the advantage that MaxBackends would be kept set at 
   zero, until we know the final value. That way it's obvious that you 
   cannot trust the value of MaxBackends in a contrib module 
   preload-function, for example, which would reduce the chance of 
   programmer mistakes.
   
   > So I now came up with a completely different idea: how about making
   > MaxBackends a macro, i.e.
   >
   > +#define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \
   > +                    GetNumShmemAttachedBgworkers())
   >
   > so that instead of having guc.c recompute it, each caller that needs to
   > value obtains it up to date all the time?  This additionally means that
   > assign_maxconnections and assign_autovacuum_max_workers go away (only
   > the check routines remain).  Patch attached.
   >
   > The one problem I see as serious with this approach is that it'd be
   > moderately expensive (i.e. not just fetch a value from memory) to
   > compute the value because it requires a walk of the registered workers
   > list.  For most callers this wouldn't be a problem because it's just
   > during shmem sizing/creation; but there are places such as multixact.c
   > and async.c that use it routinely, so it's likely that we need to cache
   > the value somehow.  It seems relatively straightforward though.
   
   I don't like that. The result of GetNumShmemAttachedBgWorkers() doesn't 
   change after postmaster startup, so it seems silly to call it 
   repeatedly. And from a readability point of view, it makes you think 
   that it might change, because it's recalculated every time.
   
   If I'm reading the code correctly, GetNumShmemAttachedBgWorkers() works 
   by walking through a backend-local list. What happens if a background 
   worker fails to register itself when preloaded in one backend? That 
   backend would calculate a different value of MaxBackends, with 
   interesting consequences. That would be a clear case of "don't do that", 
   but nevertheless, I think it would be better if we didn't rely on that. 
   I'd suggest adding MaxBackends to the list of variables passed from 
   postmaster to backends via BackendParameters.
   
   All in all, I propose the attached. Not tested on Windows.
   
   - Heikki
   

4. Re: fix bgworkers in EXEC_BACKEND

   Stephen Frost <sfrost@snowman.net> — 2012-12-27T18:16:21Z

   * Heikki Linnakangas (hlinnakangas@vmware.com) wrote:
   > Thinking about this some more, it might be cleaner to move the
   > responsibility of setting MaxBackends out of guc.c, into
   > postmaster.c. The guc machinery would set max_connections and
   > autovacuum_max_workers as usual, but not try to set MaxBackends.
   > After reading the config file in postmaster.c, calculate
   > MaxBackends.
   
   I tend to prefer Heikki's solution wrt supporting what we do currently.
   I do wonder if, perhaps, the reason the assign_XXX() functions were put
   in place and used from GUC was a hope that some day we'd actually
   support online changing of max_connections (and friends).  I realize
   that's pretty pie-in-the-sky, but it sure would be nice to reduce the
   number of parameters that require a full restart.
   
   All that said, putting those functions back and changing guc.c would
   certainly be pretty trivially done, should some new patch come along
   that would allow online changing of max_connections.
   
   So, +1 on Heikki's approach.
   
   	Thanks,
   
   		Stephen
   

5. Re: fix bgworkers in EXEC_BACKEND

   Simon Riggs <simon@2ndquadrant.com> — 2012-12-27T18:17:58Z

   On 27 December 2012 18:06, Heikki Linnakangas <hlinnakangas@vmware.com> wrote:
   
   > This would have the advantage that MaxBackends would be kept set at zero,
   > until we know the final value. That way it's obvious that you cannot trust
   > the value of MaxBackends in a contrib module preload-function, for example,
   > which would reduce the chance of programmer mistakes.
   
   I admire your forward thinking on that; yes, that could cause
   problems. But even then, we would be admitting that nobody now gets a
   valid value of MaxBackends, which sounds like it might be a problem in
   itself.
   
   Perhaps we should try to solve that a different way? Can we ask for
   reservations of bgworkers ahead of running their _init functions,
   using an additional API call?
   
   That way we'd know the final value and everybody would have the
   correct value at init time.
   
   -- 
    Simon Riggs                   http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

6. Re: fix bgworkers in EXEC_BACKEND

   Stephen Frost <sfrost@snowman.net> — 2012-12-27T18:36:43Z

   Simon,
   
   * Simon Riggs (simon@2ndQuadrant.com) wrote:
   > I admire your forward thinking on that; yes, that could cause
   > problems. But even then, we would be admitting that nobody now gets a
   > valid value of MaxBackends, which sounds like it might be a problem in
   > itself.
   
   I agree that the current implementation could lead to problems/confusion
   for contrib module authors, if they're doing something with MaxBackends.
   
   > Perhaps we should try to solve that a different way? Can we ask for
   > reservations of bgworkers ahead of running their _init functions,
   > using an additional API call?
   
   Before we go there, I'm honestly curious to hear what the use case is
   for a contrib module to need that information, particularly at _init
   time..?  Also, would we need every contrib module to add in that call?
   What if they don't create any additional bgworkers?  What if they do but
   don't provide the API call?  I wonder if max_connections, which is well
   defined and not subject to other modules or other things happening,
   might be a better value to encourage authors to use, if they're looking
   for some heuristic for how many possible backends there could be?
   
   > That way we'd know the final value and everybody would have the
   > correct value at init time.
   
   This sounds like an opportunity for people to add another backend worker
   and then forget to update what they tell the postmaster and everyone
   else about how many backend workers they're going to create..  Or maybe
   having a larger value be returned than they actually use.
   
   	Thanks,
   
   		Stephen
   

7. Re: fix bgworkers in EXEC_BACKEND

   Simon Riggs <simon@2ndquadrant.com> — 2012-12-27T18:45:32Z

   On 27 December 2012 18:36, Stephen Frost <sfrost@snowman.net> wrote:
   > Simon,
   >
   > * Simon Riggs (simon@2ndQuadrant.com) wrote:
   >> I admire your forward thinking on that; yes, that could cause
   >> problems. But even then, we would be admitting that nobody now gets a
   >> valid value of MaxBackends, which sounds like it might be a problem in
   >> itself.
   >
   > I agree that the current implementation could lead to problems/confusion
   > for contrib module authors, if they're doing something with MaxBackends.
   
   I can't see any problems myself and am happy with Heikki's proposal to
   accept that restriction, since other workarounds are possible.
   
   -- 
    Simon Riggs                   http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

8. Re: fix bgworkers in EXEC_BACKEND

   Alvaro Herrera <alvherre@2ndquadrant.com> — 2012-12-27T18:49:13Z

   Stephen Frost wrote:
   > * Heikki Linnakangas (hlinnakangas@vmware.com) wrote:
   > > Thinking about this some more, it might be cleaner to move the
   > > responsibility of setting MaxBackends out of guc.c, into
   > > postmaster.c. The guc machinery would set max_connections and
   > > autovacuum_max_workers as usual, but not try to set MaxBackends.
   > > After reading the config file in postmaster.c, calculate
   > > MaxBackends.
   > 
   > I tend to prefer Heikki's solution wrt supporting what we do currently.
   > I do wonder if, perhaps, the reason the assign_XXX() functions were put
   > in place and used from GUC was a hope that some day we'd actually
   > support online changing of max_connections (and friends).
   
   No, that's not the reason.  The reason is that the "check" hooks did not
   exist at all, so both the check and the assignment were done in the
   assign hook.  Now we're getting rid of the assignment hooks because
   they're useless, but the check hooks must, of course, remain.  We put
   the hooks in because it was the simplest thing we could think of to set
   MaxBackends when either max_connections or autovacuum_max_workers were
   tweaked.  My guess is if we had thought of propagating MaxBackends via
   BackendParameters back then, we'd probably gone that route as well.
   But certainly we had no intention to make max_connections online changeable.
   
   I too like Heikki's proposed patch much better than mine.  Thanks.
   
   -- 
   Álvaro Herrera                http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

9. Re: fix bgworkers in EXEC_BACKEND

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-12-27T19:08:11Z

   Stephen Frost <sfrost@snowman.net> writes:
   > Simon,
   > * Simon Riggs (simon@2ndQuadrant.com) wrote:
   >> I admire your forward thinking on that; yes, that could cause
   >> problems. But even then, we would be admitting that nobody now gets a
   >> valid value of MaxBackends, which sounds like it might be a problem in
   >> itself.
   
   > I agree that the current implementation could lead to problems/confusion
   > for contrib module authors, if they're doing something with MaxBackends.
   
   This is more or less a necessary consequence of the fact that _init
   functions are now allowed to add background workers.  If there is any
   code today that expects MaxBackends to be correct at
   preload_shared_libraries time, it's already been broken irretrievably
   by the bgworkers patch; and we'd be well advised to make that breakage
   obvious not subtle.
   
   So I'm +1 for Heikki's proposal as well.
   
   			regards, tom lane
   
   
   

10. Re: fix bgworkers in EXEC_BACKEND

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2012-12-27T20:46:45Z

    Heikki Linnakangas wrote:
    
    > Might be cleaner to directly assign the correct value to MaxBackends
    > above, ie. "MaxBackends =  MaxConnections + newval + 1 +
    > GetNumShmemAttachedBgworkers()". With a comment to remind that it
    > needs to be kept in sync with the other places where that
    > calculation is done, in guc.c. Or put that calculation in a new
    > function and call it above and in guc.c.
    > 
    > Thinking about this some more, it might be cleaner to move the
    > responsibility of setting MaxBackends out of guc.c, into
    > postmaster.c. The guc machinery would set max_connections and
    > autovacuum_max_workers as usual, but not try to set MaxBackends.
    > After reading the config file in postmaster.c, calculate
    > MaxBackends.
    
    Here's a small patch that applies on top of yours.  Do you intend to
    commit this?  If not, let me know and I'll do it.
    
    Thanks.
    
    -- 
    Álvaro Herrera                http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
    

11. Re: fix bgworkers in EXEC_BACKEND

    Andres Freund <andres@2ndquadrant.com> — 2012-12-27T23:09:18Z

    On 2012-12-27 20:06:07 +0200, Heikki Linnakangas wrote:
    > On 27.12.2012 19:15, Alvaro Herrera wrote:
    > >I committed background workers three weeks ago, claiming it worked on
    > >EXEC_BACKEND, and shortly thereafter I discovered that it didn't.  I
    > >noticed that the problem is the kludge to cause postmaster and children
    > >to recompute MaxBackends after shared_preload_libraries is processed; so
    > >the minimal fix is to duplicate this bit, from PostmasterMain() into
    > >SubPostmasterMain():
    > 
    > If I'm reading the code correctly, GetNumShmemAttachedBgWorkers() works by
    > walking through a backend-local list. What happens if a background worker
    > fails to register itself when preloaded in one backend? That backend would
    > calculate a different value of MaxBackends, with interesting consequences.
    > That would be a clear case of "don't do that", but nevertheless, I think it
    > would be better if we didn't rely on that. I'd suggest adding MaxBackends to
    > the list of variables passed from postmaster to backends via
    > BackendParameters.
    
    I am still worried about the following scenario in the EXEC_BACKEND case:
    
    1) postmaster starts
    2) reads config
    3) executes _PG_init for shared_preload_libraries
    4) library 'abc' gets config value 'abc.num_workers = 2' and registers as many workers
    5) some time goes by, workers, backends start
    6) abc.num_workers gets changed to 3, SIGHUP
    7) some worker dies and gets restarted
    8) _PG_init in the new worker does one more RegisterBackgroundWorker than before
    9) ???
    
    Greetings,
    
    Andres Freund
    
    -- 
     Andres Freund	                   http://www.2ndQuadrant.com/
     PostgreSQL Development, 24x7 Support, Training & Services
    
    
    

12. Re: fix bgworkers in EXEC_BACKEND

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-12-27T23:44:57Z

    Andres Freund <andres@2ndquadrant.com> writes:
    > I am still worried about the following scenario in the EXEC_BACKEND case:
    
    > 1) postmaster starts
    > 2) reads config
    > 3) executes _PG_init for shared_preload_libraries
    > 4) library 'abc' gets config value 'abc.num_workers = 2' and registers as many workers
    > 5) some time goes by, workers, backends start
    > 6) abc.num_workers gets changed to 3, SIGHUP
    
    This is broken whether it's EXEC_BACKEND or not: you don't get to change
    anything that determines the number of workers post-startup.
    num_workers should have been declared PGC_POSTMASTER.
    
    			regards, tom lane
    
    
    

13. Re: fix bgworkers in EXEC_BACKEND

    Andres Freund <andres@2ndquadrant.com> — 2012-12-27T23:48:50Z

    On 2012-12-27 18:44:57 -0500, Tom Lane wrote:
    > Andres Freund <andres@2ndquadrant.com> writes:
    > > I am still worried about the following scenario in the EXEC_BACKEND case:
    >
    > > 1) postmaster starts
    > > 2) reads config
    > > 3) executes _PG_init for shared_preload_libraries
    > > 4) library 'abc' gets config value 'abc.num_workers = 2' and registers as many workers
    > > 5) some time goes by, workers, backends start
    > > 6) abc.num_workers gets changed to 3, SIGHUP
    >
    > This is broken whether it's EXEC_BACKEND or not: you don't get to change
    > anything that determines the number of workers post-startup.
    > num_workers should have been declared PGC_POSTMASTER.
    
    Well, the problem is, a shared library can't do that
    afaics. abc.num_workers would be using custom_variable_classes (well,
    whatever its called now, that it doesn't need to be configured).
    
    There is no predefined 'num_workers' variable or anything like it in the
    patch, but I guess some of the module authors will come of up with
    configuration variables like that.
    
    I personally want e.g. 'bdr.databases = 'a, b, c' which obviously has
    the same problem...
    
    Greetings,
    
    Andres Freund
    
    --
     Andres Freund	                   http://www.2ndQuadrant.com/
     PostgreSQL Development, 24x7 Support, Training & Services
    
    
    

14. Re: fix bgworkers in EXEC_BACKEND

    Andres Freund <andres@anarazel.de> — 2012-12-28T00:28:49Z

    
    Tom Lane <tgl@sss.pgh.pa.us> schrieb:
    
    >Andres Freund <andres@2ndquadrant.com> writes:
    >> I am still worried about the following scenario in the EXEC_BACKEND
    >case:
    >
    >> 1) postmaster starts
    >> 2) reads config
    >> 3) executes _PG_init for shared_preload_libraries
    >> 4) library 'abc' gets config value 'abc.num_workers = 2' and
    >registers as many workers
    >> 5) some time goes by, workers, backends start
    >> 6) abc.num_workers gets changed to 3, SIGHUP
    >
    >This is broken whether it's EXEC_BACKEND or not: you don't get to
    >change
    >anything that determines the number of workers post-startup.
    >num_workers should have been declared PGC_POSTMASTER.
    
    BTW, I think it happens not to be broken in the non EXEC_BACKEND case because the registration point for bgworkers is _PG_init which will only be called once without EXEC_BACKEND, so config changes to SIGHUP'able variables won't change a thing dangerous as the bgworker stuff is all set up.
    
    Andres
    
    
    --- 
    Please excuse the brevity and formatting - I am writing this on my mobile phone.
    
    
    

15. Re: fix bgworkers in EXEC_BACKEND

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-12-28T06:09:41Z

    Andres Freund <andres@2ndquadrant.com> writes:
    > On 2012-12-27 18:44:57 -0500, Tom Lane wrote:
    >> This is broken whether it's EXEC_BACKEND or not: you don't get to change
    >> anything that determines the number of workers post-startup.
    >> num_workers should have been declared PGC_POSTMASTER.
    
    > Well, the problem is, a shared library can't do that
    > afaics. abc.num_workers would be using custom_variable_classes (well,
    > whatever its called now, that it doesn't need to be configured).
    
    We fixed that a few years ago, no?
    
    http://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=4605d1c98
    
    If that's broken, we certainly need to fix it again.  This issue exists
    for any parameter that feeds into shared memory sizing, which is exactly
    why we changed it back then.
    
    			regards, tom lane
    
    
    

16. Re: fix bgworkers in EXEC_BACKEND

    Heikki Linnakangas <hlinnakangas@vmware.com> — 2012-12-29T16:15:50Z

    On 27.12.2012 22:46, Alvaro Herrera wrote:
    > Heikki Linnakangas wrote:
    >
    >> Might be cleaner to directly assign the correct value to MaxBackends
    >> above, ie. "MaxBackends =  MaxConnections + newval + 1 +
    >> GetNumShmemAttachedBgworkers()". With a comment to remind that it
    >> needs to be kept in sync with the other places where that
    >> calculation is done, in guc.c. Or put that calculation in a new
    >> function and call it above and in guc.c.
    >>
    >> Thinking about this some more, it might be cleaner to move the
    >> responsibility of setting MaxBackends out of guc.c, into
    >> postmaster.c. The guc machinery would set max_connections and
    >> autovacuum_max_workers as usual, but not try to set MaxBackends.
    >> After reading the config file in postmaster.c, calculate
    >> MaxBackends.
    >
    > Here's a small patch that applies on top of yours.  Do you intend to
    > commit this?  If not, let me know and I'll do it.
    
    Wasn't planning to, feel free to commit.
    
    - Heikki
    
    
    

17. Re: fix bgworkers in EXEC_BACKEND

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2012-12-31T21:06:39Z

    Heikki Linnakangas wrote:
    > On 27.12.2012 22:46, Alvaro Herrera wrote:
    > >Heikki Linnakangas wrote:
    > >
    > >>Might be cleaner to directly assign the correct value to MaxBackends
    > >>above, ie. "MaxBackends =  MaxConnections + newval + 1 +
    > >>GetNumShmemAttachedBgworkers()". With a comment to remind that it
    > >>needs to be kept in sync with the other places where that
    > >>calculation is done, in guc.c. Or put that calculation in a new
    > >>function and call it above and in guc.c.
    > >>
    > >>Thinking about this some more, it might be cleaner to move the
    > >>responsibility of setting MaxBackends out of guc.c, into
    > >>postmaster.c. The guc machinery would set max_connections and
    > >>autovacuum_max_workers as usual, but not try to set MaxBackends.
    > >>After reading the config file in postmaster.c, calculate
    > >>MaxBackends.
    
    Actually this patch still needed one more change, because we weren't
    rechecking that we're not beyond the MAX_BACKENDS value after bgworker
    registration.
    
    This is hard to hit, because with the current compile constants you need
    over eight million backends in total to hit that limit (and 360 GB of
    shared memory), but it seems dangerous to leave that without any
    protection.
    
    I kinda hate this a bit because I had to move MAX_BACKENDS from a
    private value in guc.c to postmaster.h.  But the wording of the error
    message is what took the most time:
    
    +               ereport(LOG,                                                                                                                                            
    +                               (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),                                                                                         
    +                                errmsg("too many background workers"),                                                                                                 
    +                                errdetail("Up to %d background workers can be registered with the current settings.",                                                  
    +                                                  MAX_BACKENDS - (MaxConnections + autovacuum_max_workers + 1))));                                                     
    
    Completely different ideas for this error message are welcome.  My first
    try enumerated all the variables involved.
    
    -- 
    Álvaro Herrera                http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
    

18. Re: fix bgworkers in EXEC_BACKEND

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2013-01-02T16:28:19Z

    I committed this with minor tweaks to avoid having to scan the
    registered workers list on each registration.  Opinions on this error
    report are still welcome:
    
    > +               ereport(LOG,
    > +                               (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
    > +                                errmsg("too many background workers"),
    > +                                errdetail("Up to %d background workers can be registered with the current settings.",
    > +                                                  MAX_BACKENDS - (MaxConnections + autovacuum_max_workers + 1))));
    
    Thanks to everyone for their input --- and happy 2013 hacking to all.
    
    -- 
    Álvaro Herrera                http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services