* 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: [EMAIL PROTECTED]
                                                   WWW: http://www.sigaev.ru/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to