Re: collect_corrupt_items_vacuum.patch

On Mon, Jul 1, 2024 at 2:18 AM Noah Misch <noah@leadboat.com> wrote:
> On Tue, Mar 12, 2024 at 02:10:59PM +0200, Alexander Korotkov wrote:
> > I'm going to push this if no objections.
>
> Commit e85662d wrote:
> > --- a/src/backend/storage/ipc/procarray.c
> > +++ b/src/backend/storage/ipc/procarray.c
>
> > @@ -2740,6 +2741,8 @@ GetRunningTransactionData(void)
> >        */
> >       for (index = 0; index < arrayP->numProcs; index++)
> >       {
> > +             int                     pgprocno = arrayP->pgprocnos[index];
> > +             PGPROC     *proc = &allProcs[pgprocno];
> >               TransactionId xid;
> >
> >               /* Fetch xid just once - see GetNewTransactionId */
> > @@ -2760,6 +2763,13 @@ GetRunningTransactionData(void)
> >               if (TransactionIdPrecedes(xid, oldestRunningXid))
> >                       oldestRunningXid = xid;
> >
> > +             /*
> > +              * Also, update the oldest running xid within the current database.
> > +              */
> > +             if (proc->databaseId == MyDatabaseId &&
> > +                     TransactionIdPrecedes(xid, oldestRunningXid))
> > +                     oldestDatabaseRunningXid = xid;
>
> Shouldn't that be s/oldestRunningXid/oldestDatabaseRunningXid/?

Thank you for catching this.

> While this isn't a hot path, I likely would test TransactionIdPrecedes()
> before fetching pgprocno and PGPROC, to reduce wasted cache misses.

And thanks for suggestion.

The patchset is attached.  0001 implements
s/oldestRunningXid/oldestDatabaseRunningXid/.  0002 implements cache
misses optimization.

If no objection, I'll backpatch 0001 and apply 0002 to the head.

------
Regards,
Alexander Korotkov
Supabase