Message-ID: <8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi>
Date: Tue, 23 Jan 2024 21:07:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Refactoring backend fork+exec code
Content-Language: en-US
To: Andres Freund <andres@anarazel.de>
Cc: Tristan Partin <tristan@neon.tech>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
 Thomas Munro <thomas.munro@gmail.com>
References: <7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi>
 <20230710225043.svl7fqxecwshwc7a@awork3.anarazel.de>
 <119551e6-60f6-49b2-ae04-a382a89f900c@iki.fi>
 <e0dcfabb-f59a-45e8-bd92-f8d706c92128@iki.fi>
 <20231130202648.7k6agmuizdilufnv@awork3.anarazel.de>
 <59527c9b-c3f7-4d0c-a3f1-00d0ed5cc186@iki.fi>
 <f3ecd4cb-85ee-4e54-8278-5fabfb3a4ed0@iki.fi>
 <20240122210740.7vq5fd4woixpwx3f@awork3.anarazel.de>
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <20240122210740.7vq5fd4woixpwx3f@awork3.anarazel.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407%40iki.fi>
Precedence: bulk

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)