On Fri, Dec 7, 2018 at 12:50 AM Peter Geoghegan <p...@bowt.ie> wrote: > On Thu, Dec 6, 2018 at 12:51 PM Alexander Korotkov <aekorot...@gmail.com> > wrote: > > So, algorithm introduced by 218f51584d5 is broken. It tries to > > guarantee that there are no inserters in the subtree by placing > > cleanup lock to subtree root, assuming that inserter holds pins on the > > path from root to leaf. But due to concurrent splits of internal > > pages the pins held can be not relevant to actual path. I don't see > > the way to fix this easily. So, I think we should revert it from back > > branches and try to reimplement that in master. > > > > 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? > > Your proposal sounds reasonable, but I'd like to see what you have in > mind in detail before commenting.
Yep, please find attached draft patch. BTW, it seems that I've another bug in GIN. README says that "However, posting trees are only fully searched from left to right, starting from the leftmost leaf. (The tree-structure is only needed by insertions, to quickly find the correct insert location). So as long as we don't delete the leftmost page on each level, a search can never follow a downlink to page that's about to be deleted." But that's not really true once we teach GIN to skip parts of posting trees in PostgreSQL 9.4. So, there might be a risk to visit page to be deleted using downlink. But in order to get real problem, vacuum should past cleanup stage and someone else should reuse this page, before we traverse downlink. Thus, the risk of real problem is very low. But it still should be addressed. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
gin-vacuum-fix-1.patch
Description: Binary data