On 2017-06-07 11:51:12 +0200, Petr Jelinek wrote:
> On 07/06/17 03:00, Andres Freund wrote:
> > On 2017-06-06 19:36:13 +0200, Petr Jelinek wrote:
> > 
> >> As a side note, we are starting to have several IsSomeTypeOfProcess
> >> functions for these kind of things. I wonder if bgworker infrastructure
> >> should somehow provide this type of stuff (the proposed bgw_type might
> >> help there) as well as maybe being able to register interrupt handler
> >> for the worker (it's really hard to do it via custom SIGTERM handler as
> >> visible on worker, launcher and walsender issues we are fixing).
> >> Obviously this is PG11 related thought.
> > 
> > Maybe it's also a sign that the bgworker infrastructure isn't really the
> > best thing to use for internal processes like parallel workers and lrep
> > processes - it's really built so core code *doesn't* know anything
> > specific about them, which isn't really what we want in some of these
> > cases.  That's not to say that bgworkers and parallelism/lrep shouldn't
> > share infrastructure, don't get me wrong.
> >
> 
> Well the nice thing about bgworkers is that it provides the basic
> infrastructure.

Right.


> Main reason lrep stuff is using it is that I didn't want to add
> another special backend kind to 20 different places, but turns out it
> still needs to be in some. So either we need to add more support for
> these kind of things to bgworkers or write something for internal
> workers that shares the infrastructure with bgworkers as you say.

Yea, I think we should radically unify a lot of the related
infrastructure between all processes.  We've grown a lot of duplication.


> >> @@ -2848,6 +2849,14 @@ ProcessInterrupts(void)
> >>                    ereport(FATAL,
> >>                                    (errcode(ERRCODE_ADMIN_SHUTDOWN),
> >>                                     errmsg("terminating logical 
> >> replication worker due to administrator command")));
> >> +          else if (IsLogicalLauncher())
> >> +          {
> >> +                  ereport(DEBUG1,
> >> +                                  (errmsg("logical replication launcher 
> >> shutting down")));
> >> +
> >> +                  /* The logical replication launcher can be stopped at 
> >> any time. */
> >> +                  proc_exit(0);
> >> +          }
> > 
> > We could use PgBackendStatus->st_backendType for these, merging these
> > different paths.
> > 
> 
> Hmm, that's not that easy, st_backendType will be generic type for
> bgworker as the bgw_type patch didn't land yet (which is discussed in
> yet another thread [1]). It seems like an argument for going forward
> with it (the bgw_type patch) in PG10.

Yea.  I left it as is for now.


> I don't mind, it has some overlap with your proposed fixes for latching
> so if you are willing go ahead.

Done.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to