Re: use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: 盏一 <w(at)hidva(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`
Date: 2021-05-06 19:27:29
Message-ID: 4175578.1620329249@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2021-05-07 00:30:13 +0800, 盏一 wrote:
>> Since we have introduced `pgxactoff` in [941697c3c1ae5d6ee153065adb96e1e63ee11224](https://github.com/postgres/postgres/commit/941697c3c1ae5d6ee153065adb96e1e63ee11224), and `pgxactoff` is always the index of `proc->pgprocno` in `procArray->pgprocnos`. So it seems that we could directly use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`? My thought is to replace

> Sounds like a plan! Do you want to write a patch?

> If you do, I think it might be worthwhile to add an only-with-assertions
> loop checking that there's no other entry with the same pgprocno in the
> dense arrays.

Hmm, I can definitely see keeping a check that the selected entry
has the right PID and/or pgprocno, but making it search for duplicates
seems a bit over the top. The existing code isn't guarding against
that, and I don't really see a reason why there's a meaningful risk
of it.

> Given that the code is new in 14, I wonder if we should cram this
> simplification in before beta?

+1, seems like a pretty clear missed opportunity in 941697c3c.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-05-06 19:31:02 Re: Printing backtrace of postgres processes
Previous Message Hannu Krosing 2021-05-06 19:23:36 Re: .ready and .done files considered harmful