From: Teodor Sigaev <teodor@sigaev.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCHES] GIN improvements
Date: Thu, 24 Jul 2008 19:53:56 +0400
Message-ID: <4888A594.6010102@sigaev.ru> (raw)
In-Reply-To: <8993.1216843701@sss.pgh.pa.us>
References: <483FD205.4090904@sigaev.ru>
<486BC501.6060605@sigaev.ru>
<27880.1215810535@sss.pgh.pa.us>
<487A6E0C.3090600@sigaev.ru>
<8993.1216843701@sss.pgh.pa.us>
> * shiftList() holds an exclusive lock on metapage throughout its run,
> which means that it's impossible for two of them to run concurrently.
> So why bother with "concurrent deletion" detection?
Because metapage is locked immediately before shiftList call, while metapage is
unlocked another process could produce locking metapage and execution of
shiftList. So, when shiftList starts it should check of already deleted page. If
shiftList sees already deleted page then it doesn't do anything and reports to
the caller.
> * shiftList does LockBufferForCleanup, which means that it can be blocked
> for an indefinitely long time by a concurrent scan, and since it's holding
> exclusive lock on metapage no new scans or insertions can start meanwhile.
> This is not only horrid from a performance standpoint but it very probably
> can result in deadlocks --- which will be deadlocks on LWLocks and thus
> not even detected by the system.
Ops, I see possible scenario: UPDATE tbl SET gin_indexed_field = ... where
gin_indexed_field .... with concurrent shiftList. Will fix. Thank you.
Nevertheless, shiftList should be fast in typical scenario: it doesn't do
complicated work but just marks as deleted pages which already was readed before.
> * GIN index scans release lock and pin on one pending-list page before
> acquiring pin and lock on the next, which means there's a race condition:
> shiftList could visit and delete the next page before we get to it,
> because there's a window where we're holding no buffer lock at all.
Agree, will fix.
> * It seems also possible that once a list page has been marked
> GIN_DELETED, it could be re-used for some other purpose before a
> scan-in-flight reaches it -- reused either as a regular index page or as a
Impossible - because deletion is running from the head of list and scan too. But
deletion locks metapage and locks pages for cleanup. So, scan may start only
from not yet deleted page and will go through the list before deletion process.
> * There is a bigger race condition, which is that after a scan has
> returned a tuple from a pending page, vacuum could move the index entry
> into the main index structure, and then that same scan could return that
> same index entry a second time. This is a no-no, and I don't see any easy
> fix.
Hmm, isn't it allowed for indexes? At least GiST has this behaviour from its
birth date.
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: teodor@sigaev.ru, tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org
Subject: Re: [PATCHES] GIN improvements
In-Reply-To: <4888A594.6010102@sigaev.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox