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
