Re: Stale hash entries when reusing an auxiliary resource owner

Hi,

> > The fast-path is the !hash case, I don't think any realistic workload would
> > notice the overhead of zeroing out hash table entries during the compaction in
> > ResourceOwnerSort()? I don't know if we'd need to do anything else?
>
> For now, the latter seems more reasonable to me, since Reset is
> specifically responsible for preparing a released owner for reuse.
> Clearing the retained hash table fits that responsibility. Also, Reset
> would currently only be used for the auxiliary owner, whose cleanup
> seems less performance-critical than the general ResourceOwner release
> path.
>
> I'm not sure which is better overall, so I have prepared patches
> for both approaches.

My two cents:

It will be interesting to benchmark the overhead of clearing these entries
during ResourceOwnerSort and ResourceOwnerRelease.

Assuming no significant performance impact, I think it makes more
sense to reset the entries where they are being compacted out or
released instead of introducing a new API
function for the same. It also guarantees that entries are cleared
even if an error occurs before they are reset.
I am planning to review the patches in detail.

FWIW, unrelated but improving the same API, [1] makes it easier for
non-transaction resource
owners like the AuxProcessResourceOwner to release resources by
allowing them to release all the resources at
once instead of calling ResourceOwnerRelease three times for each phase.
Views are welcome.

[1]. https://www.postgresql.org/message-id/CAH2L28vVo-Q3Jf+P4huRKxjnbF+2t7OdDh3ZGMxL88hiWezd=A@mail.gmail.com

Thank you,
Rahila Syed