public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Andres Freund <[email protected]>
Cc: Tristan Partin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Thomas Munro <[email protected]>
Subject: Re: Refactoring backend fork+exec code
Date: Tue, 23 Jan 2024 21:07:08 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On 22/01/2024 23:07, Andres Freund wrote:
> On 2024-01-10 14:35:52 +0200, Heikki Linnakangas wrote:
>> @@ -5344,31 +5344,31 @@ StartChildProcess(AuxProcType type)
>> errno = save_errno;
>> switch (type)
>> {
>> - case StartupProcess:
>> + case B_STARTUP:
>> ereport(LOG,
>> (errmsg("could not fork startup process: %m")));
>> break;
>> - case ArchiverProcess:
>> + case B_ARCHIVER:
>> ereport(LOG,
>> (errmsg("could not fork archiver process: %m")));
>> break;
>> - case BgWriterProcess:
>> + case B_BG_WRITER:
>> ereport(LOG,
>> (errmsg("could not fork background writer process: %m")));
>> break;
>> - case CheckpointerProcess:
>> + case B_CHECKPOINTER:
>> ereport(LOG,
>> (errmsg("could not fork checkpointer process: %m")));
>> break;
>> - case WalWriterProcess:
>> + case B_WAL_WRITER:
>> ereport(LOG,
>> (errmsg("could not fork WAL writer process: %m")));
>> break;
>> - case WalReceiverProcess:
>> + case B_WAL_RECEIVER:
>> ereport(LOG,
>> (errmsg("could not fork WAL receiver process: %m")));
>> break;
>> - case WalSummarizerProcess:
>> + case B_WAL_SUMMARIZER:
>> ereport(LOG,
>> (errmsg("could not fork WAL summarizer process: %m")));
>> break;
>
> Seems we should replace this with something slightly more generic one of these
> days...
The later patches in this thread will turn these into
ereport(LOG,
(errmsg("could not fork %s process: %m",
PostmasterChildName(type))));
>> diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
>> index 1a1050c8da1..92f24db4e18 100644
>> --- a/src/backend/utils/activity/backend_status.c
>> +++ b/src/backend/utils/activity/backend_status.c
>> @@ -257,17 +257,16 @@ pgstat_beinit(void)
>> else
>> {
>> /* Must be an auxiliary process */
>> - Assert(MyAuxProcType != NotAnAuxProcess);
>> + Assert(IsAuxProcess(MyBackendType));
>>
>> /*
>> * Assign the MyBEEntry for an auxiliary process. Since it doesn't
>> * have a BackendId, the slot is statically allocated based on the
>> - * auxiliary process type (MyAuxProcType). Backends use slots indexed
>> - * in the range from 0 to MaxBackends (exclusive), so we use
>> - * MaxBackends + AuxProcType as the index of the slot for an auxiliary
>> - * process.
>> + * auxiliary process type. Backends use slots indexed in the range
>> + * from 0 to MaxBackends (exclusive), and aux processes use the slots
>> + * after that.
>> */
>> - MyBEEntry = &BackendStatusArray[MaxBackends + MyAuxProcType];
>> + MyBEEntry = &BackendStatusArray[MaxBackends + MyBackendType - FIRST_AUX_PROC];
>> }
>
> Hm, this seems less than pretty. It's probably ok for now, but it seems like a
> better fix might be to just start assigning backend ids to aux procs or switch
> to indexing by pgprocno?
Using pgprocno is a good idea. Come to think of it, why do we even have
a concept of backend ID that's separate from pgprocno? backend ID is
used to index the ProcState array, but AFAICS we could use pgprocno as
the index to that, too.
--
Heikki Linnakangas
Neon (https://neon.tech)
view thread (21+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Refactoring backend fork+exec code
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox