Re: Potential GIN vacuum bug

On 08/16/2015 12:58 AM, Jeff Janes wrote:
> When ginbulkdelete gets called for the first time in a VACUUM(i.e. stats
> == NULL), one of the first things it does is call ginInsertCleanup to get
> rid of the pending list. It does this in lieu of vacuuming the pending
> list.
>
> This is important because if there are any dead tids still in the Pending
> list, someone else could come along during the vacuum and post the dead
> tids into a part of the index that VACUUM has already passed over.
>
> The potential bug is that ginInsertCleanup exits early (ginfast.c lines
> 796, 860, 898) if it detects that someone else is cleaning up the pending
> list, without waiting for that someone else to finish the job.
>
> Isn't this a problem?

Yep, I think you're right. When that code runs as part of VACUUM, it
should not give up like that.

Hmm, I see other race conditions in that code too. Even if VACUUM wins
the race you spotted, and performs all the insertions, reaches the end
of the pending items list, and deletes the pending list pages, it's
possible that another backend started earlier, and is still processing
the same items from the pending items list. It will add them to the
tree, and after it's finished with that it will see that the pending
list page was already deleted, and bail out. But if there is a dead
tuple in the pending items list, you have trouble. The other backend
will re-insert it, and that might happen after VACUUM had already
removed it from the tree.

Also, ginInsertCleanup() seems to assume that if another backend has
just finished cleaning up the same page, it will see the page marked as
deleted. But what if the page is not only marked as deleted, but also
reused for something else already?

- Heikki