Hi Kevin,
> Andrey, let me know if you have concerns about lock chaining/cleanup
> with this approach.
I don't see a problem with the move. It is before the cleanup lock on
the primary bucket is acquired, while the lock chaining within
hashbucketcleanup() is unchanged. A bucket with a long overflow chain
can accumulate more vacuum cost before delaying, but there is no safe
boundary inside that chain. The per-bucket placement seems like the
best available option.
There is now a production report of exactly this hash problem [0]. A
shutdown waited about six minutes for an autovacuum in
hashbucketcleanup(). Mostafa independently posted the same per-bucket
move and measured cancellation improving from about 2.08 seconds to
0.014 seconds. That confirms the placement and makes the hash fix a
backpatch candidate.
One detail in 0002: the two assertions do not check
QueryCancelHoldoffCount. Since vacuum_delay_point() is also expected to
service query cancellation, I think this should simply be:
Assert(INTERRUPTS_CAN_BE_PROCESSED());
This also expresses Tom's proposed invariant directly. Did you run
the full CI after this change? check-world on one system might be not
enough.
Best regards, Andrey Borodin.
[0] https://postgr.es/m/19628-c2b17d358181a1ea%40postgresql.org
