Horton

Re: ResourceOwner refactoring

Zhihong Yu <zyu@yugabyte.com>

View thread View on postgr.es
From: Zhihong Yu <zyu@yugabyte.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, vignesh C <vignesh21@gmail.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Michael Paquier <michael@paquier.xyz>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: 2021-07-14T15:41:43Z
Lists: pgsql-hackers 
On Wed, Jul 14, 2021 at 8:26 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> Thanks for having a look!
>
> On 14/07/2021 18:18, Zhihong Yu wrote:
> > For the loop over the hash:
> >
> > +       for (int idx = 0; idx < capacity; idx++)
> >          {
> > -           if (olditemsarr[i] != resarr->invalidval)
> > -               ResourceArrayAdd(resarr, olditemsarr[i]);
> > +           while (owner->hash[idx].kind != NULL &&
> > +                  owner->hash[idx].kind->phase == phase)
> > ...
> > +   } while (capacity != owner->capacity);
> >
> > Since the phase variable doesn't seem to change for the while loop, I
> > wonder what benefit the while loop has (since the release is governed by
> > phase).
>
> Hmm, the phase variable doesn't change, but could the element at
> 'owner->hash[idx]' change? I'm not sure about that. The loop is supposed
> to handle the case that the hash table grows; could that replace the
> element at 'owner->hash[idx]' with something else, with different phase?
> The check is very cheap, so I'm inclined to keep it to be sure.
>
> - Heikki
>
Hi,
Agreed that ```owner->hash[idx].kind->phase == phase``` can be kept.

I just wonder if we should put limit on the number of iterations for the
while loop.
Maybe add a bool variable indicating that kind->ReleaseResource(value) is
called in the inner loop.
If there is no releasing when the inner loop finishes, we can come out of
the while loop.

Cheers

Commits

  1. Make RelationFlushRelation() work without ResourceOwner during abort

  2. Fix bug in bulk extending temp relation after failure

  3. Add missing PGDLLIMPORT markings

  4. Add test_dsa module.

  5. Clear CurrentResourceOwner earlier in CommitTransaction.

  6. Fix dsa.c with different resource owners.

  7. Fix bug in the new ResourceOwner implementation.

  8. Change pgcrypto to use the new ResourceOwner mechanism.

  9. Use a faster hash function in resource owners.

  10. Make ResourceOwners more easily extensible.

  11. Move a few ResourceOwnerEnlarge() calls for safety and clarity.