Feedback-ID: id4a34324:Fastmail
Date: Tue, 23 Jan 2024 11:50:09 -0800
From: Andres Freund <andres@anarazel.de>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tristan Partin <tristan@neon.tech>,
	pgsql-hackers <pgsql-hackers@postgresql.org>,
	Thomas Munro <thomas.munro@gmail.com>
Subject: Re: Refactoring backend fork+exec code
Message-ID: <20240123195009.44wtutga24p5sh3s@awork3.anarazel.de>
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>
 <8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi>
Archived-At: 
 <https://www.postgresql.org/message-id/20240123195009.44wtutga24p5sh3s%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2024-01-23 21:07:08 +0200, Heikki Linnakangas wrote:
> On 22/01/2024 23:07, Andres Freund wrote:
> > > 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.

I think we should do that. There are a few processes not participating in
sinval, but it doesn't make enough of a difference to make sinval slower. And
I think there'd be far bigger efficiency improvements to sinvaladt than not
having a handful more entries.

Greetings,

Andres Freund