Re: ResourceOwner refactoring

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Zhihong Yu <zyu@yugabyte.com>
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:26:13Z
Lists: pgsql-hackers
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



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.