From: | Mike Palmiotto <mike(dot)palmiotto(at)crunchydata(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Yuli Khodorkovskiy <yuli(dot)khodorkovskiy(at)crunchydata(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Auxiliary Processes and MyAuxProc |
Date: | 2020-03-20 22:17:36 |
Message-ID: | CAMN686E01ULUxrwqei7cMDte+-YSs96DRn9rcKtiJ6zm7mOLxA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 19, 2020 at 4:57 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2020-03-19 11:35:41 +0100, Peter Eisentraut wrote:
> > On 2020-03-18 17:07, Mike Palmiotto wrote:
> > > On Wed, Mar 18, 2020 at 11:26 AM Mike Palmiotto
> > > <mike(dot)palmiotto(at)crunchydata(dot)com> wrote:
> > > >
> > > > On Wed, Mar 18, 2020 at 10:17 AM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
> > > > > Also, I saw this was failing tests both before and after my rebase.
> > > > >
> > > > > http://cfbot.cputube.org/
> > > > > https://ci.appveyor.com/project/postgresql-cfbot/postgresql/builds/31535161
> > > > > https://ci.appveyor.com/project/postgresql-cfbot/postgresql/builds/31386446
> > > >
> > > > Good catch, thanks. Will address this as well in the next round. Just
> > > > need to set up a Windows dev environment to see if I can
> > > > reproduce/fix.
I'm still working on wiring up an AppVeyor instance, as seemingly
builds don't work on any of the default Azure/Visual Studio images. In
the meantime, I've fixed some spurious whitespace changes and the
compile error for non-EXEC_BACKEND. I'm posting a new version to keep
Travis happy at least while I keep working on that. Sorry for the
delay.
> On a green field, I would say that there should be one or two C
> arrays-of-structs defining subprocesses. And most behaviour should be
> channeled through that.
>
> struct PgProcessType
> {
> const char* name;
> PgProcessBeforeShmem before_shmem;
> PgProcessEntry entrypoint;
> uint8:1 only_one_exists;
> uint8:1 uses_shared_memory;
> uint8:1 client_connected;
> uint8:1 database_connected;
> ...
> };
Only some of these are currently included in the process_types struct,
but this demonstrates the extensibility of the architecture and room
for future centralization. Do you see any items in this set that
aren't currently included but are must-haves for this round?
> Then there should be a single startup routine for all postmaster
> children. Since most of the startup is actually shared between all the
> different types of processes, we can declutter it a lot.
Agreed.
> When starting a process postmaster would just specify the process type,
> and if relevant, an argument (struct Port for backends, whatever
> relevant for bgworkers etc) . Generic code should handle all the work
> until the process type entry point - and likely we should move more work
> from the individual process types into generic code.
>
> If a process is 'only_one_exists' (to be renamed), the generic code
> would also (in postmaster) register the pid as
> subprocess_pids[type] = pid;
> which would make it easy to only have per-type code in the few locations
> that need to be aware, instead of many locations in
> postmaster.c. Perhaps also some shared memory location.
All of this sounds good.
> Coming back to earth, from green field imagination land: I think the
> patchset does go in that direction (to be fair, I think I outlined
> something like the above elsewhere in this thread in a discussion with
> Mike). And that's good.
Thanks for chiming in. Is there anything else you think needs to go in
this version to push things along -- besides fixing Windows builds, of
course?
Regards,
--
Mike Palmiotto
https://crunchydata.com
Attachment | Content-Type | Size |
---|---|---|
0001-Add-subprocess-infrastructure.patch | text/x-patch | 14.0 KB |
0002-Use-centralized-StartSubprocess-for-aux-procs.patch | text/x-patch | 14.9 KB |
0003-Add-AutoVacLauncherType-to-subprocess-struct.patch | text/x-patch | 7.5 KB |
0004-Add-AutoVacuumWorkerType-to-subprocess-struct.patch | text/x-patch | 6.4 KB |
0005-Add-PgstatCollectorType-to-subprocess-struct.patch | text/x-patch | 11.6 KB |
0006-Add-PgArchiverType-to-subprocess-struct.patch | text/x-patch | 7.7 KB |
0007-Add-SysLoggerType-to-subprocess-struct.patch | text/x-patch | 16.9 KB |
0008-Add-BgWorkerType-to-subprocess-struct.patch | text/x-patch | 25.6 KB |
0009-Add-Backends-to-subprocess-struct.patch | text/x-patch | 16.0 KB |
0010-Add-WalSenderType-to-subprocess-struct.patch | text/x-patch | 1.4 KB |
0011-Move-to-new-MyBackendType.patch | text/x-patch | 19.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2020-03-20 22:29:48 | Re: backup manifests |
Previous Message | Thomas Munro | 2020-03-20 22:14:13 | Re: Should we add xid_current() or a int8->xid cast? |