Hi! > 7 дек. 2018 г., в 2:50, Peter Geoghegan <p...@bowt.ie> написал(а): > > On Thu, Dec 6, 2018 at 12:51 PM Alexander Korotkov <aekorot...@gmail.com> > wrote: >> >> However, I'd like to note that 218f51584d5 introduces two changes: >> 1) Cleanup locking only if there pages to delete >> 2) Cleanup locking only subtree root >> The 2nd one is broken. But the 1st one seems still good for me and >> useful, because in vast majority of cases vacuum doesn't delete any >> index pages. So, I propose to revert 218f51584d5, but leave there >> logic, which locks root for cleanup only once there are pages to >> delete. Any thoughts? > > Can you post a patch that just removes the 2nd part, leaving the > still-correct first part?
I like the idea of keeping cleanup lock only if there are pages to delete. It will still solve the original problem that caused proposals for GIN VACUUM enhancements. But I must note that there is one more problem: ginVacuumPostingTreeLeaves() do not ensure that all splitted pages are visited. It copies downlink block numbers to a temp array and then unlocks parent. It is not correct way to scan posting tree for cleanup. PFA diff with following changes: 1. Always take root cleanup lock before deleting pages 2. Check for concurrent splits after scanning page Please note, that neither applying this diff nor reverting 218f51584d5 will solve bug of page delete redo lock on standby. Best regards, Andrey Borodin.
gin_root_lock.diff
Description: Binary data
