Re: Potential GIN vacuum bug - Mailing list pgsql-hackers
From | Jeff Janes |
---|---|
Subject | Re: Potential GIN vacuum bug |
Date | |
Msg-id | CAMkU=1zTC_EWFeQdGjvRmL39AP2fpKM426MjcCOMxAw_FFSHWA@mail.gmail.com Whole thread Raw |
In response to | Potential GIN vacuum bug (Jeff Janes <jeff.janes@gmail.com>) |
Responses |
Re: Potential GIN vacuum bug
Re: Potential GIN vacuum bug |
List | pgsql-hackers |
On Aug 16, 2015 11:49 PM, "Heikki Linnakangas" <hlinnaka@iki.fi> wrote:
>
> 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.Could the right to clean the pending list be represented by a self-conflicting heavy weight lock on the index? Vacuum could block on it, while user back-ends could try to get it conditionally and just give up on the cleanup if it is not available.
>
> 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?Yeah. Which is possible but pretty unlikely now; but would be far more likely if we added these page to the fsm more aggressively.
Attachment
pgsql-hackers by date: