On 02/10/2026 12:21, mostafa nabil wrote:
> Hi Kirill,
>
> Thanks again for the review. Did you get a chance to look at v2? It
> removes the vacuum_delay_point() call in hashbucketcleanup() as you
> suggested, and keeps the check at the top of hashbulkdelete()'s
> per-bucket loop.
I had a look at this. Some further fixes:
Do we have more places where we call vacuum_delay_point() while holding
locks? It seems like a bad idea to ever do that -- you don't want to
sleep while holding locks. I added an
"Assert(INTERRUPTS_CAN_BE_PROCESSED())" into vacuum_cost_delay() and ran
the regression tests, and that indeed revealed a few more places that
did that. The attached patch removes or moves those vacuum_delay_point()
calls too. I didn't include the assertion in the patch, because I'm
afraid there might be more places that we've missed, including in
extensions.
After fixing those, if you ever do call vacuum_cost_delay_point() while
holding a lock, e.g. from an extension or if we missed a caller, I think
you don't really want to sleep. And you definitely don't want to call
ProcessConfigFile() while in a critical section. So I added a quick exit
to vacuum_delay_point() if it's called with !INTERRUPTS_CAN_BE_PROCESSED().
What do you think?
- Heikki
