Re: pgsql: Separate RecoveryConflictReasons from procsignals

Hi,

On 2026-02-10 17:52:16 +0200, Heikki Linnakangas wrote:
> On 10/02/2026 17:19, Bertrand Drouvot wrote:
> > Hi,
> >
> > On Tue, Feb 10, 2026 at 02:32:37PM +0000, Heikki Linnakangas wrote:
> > > Separate RecoveryConflictReasons from procsignals
> > >
> > > Share the same PROCSIG_RECOVERY_CONFLICT flag for all recovery
> > > conflict reasons. To distinguish, have a bitmask in PGPROC to indicate
> > > the reason(s).
> >
> > I did not look at the thread, so sorry to be late, but that makes the size of PGPROC
> > going from 832 to 840 bytes, so not a multiple of 64 anymore. Is that something
> > to worry about? (same kind of discussion in [1]).
> >
> > [1]: https://postgr.es/m/tw53roer2j4quxh7vlyv62drc5fo6c6zdltvl6d2dttqa62uhi%40stwlpdwlftpj
>
> Right, that's a fair question. I hope the cache line alignment is not
> critical for performance, because the alignment is completely accidental
> today. I checked the size on different versions:
>
> master: 840 (after this commit)
> v18: 832
> v17: 888
> v14-v16: 880
>
> So v18 was the outlier in that it happened to be 64-byte aligned.
>
> If there's a performance reason to keep have it be aligned - and maybe there
> is - we should pad it explicitly.

We should make it a power of two or such. There are some workloads where the
indexing from GetPGProcByNumber() shows up, because it ends up having to be
implemented as a 64 bit multiplication, which has a reasonably high latency
(3-5 cycles). Whereas a shift has a latency of 1 and typically higher
throughput too.

Re false sharing: We should really separate stuff that changes (like
e.g. pendingRecoveryConflicts) and never changing stuff (backendType). You
don't need overlapping structs to have false sharing issues if you mix
different access patterns inside a struct that's accessed across processes...

Greetings,

Andres Freund