Re: Hash Indexes

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Jesper Pedersen <jesper.pedersen@redhat.com>, Jeff Janes <jeff.janes@gmail.com>, Mithun Cy <mithun.cy@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2016-10-18T17:22:59Z
Lists: pgsql-hackers
On Tue, Oct 18, 2016 at 5:37 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Oct 5, 2016 at 10:22 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Tue, Oct 4, 2016 at 12:36 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> I think one way to avoid the risk of deadlock in above scenario is to
>>> take the cleanup lock conditionally, if we get the cleanup lock then
>>> we will delete the items as we are doing in patch now, else it will
>>> just mark the tuples as dead and ensure that it won't try to remove
>>> tuples that are moved-by-split.  Now, I think the question is how will
>>> these dead tuples be removed.  We anyway need a separate mechanism to
>>> clear dead tuples for hash indexes as during scans we are marking the
>>> tuples as dead if corresponding tuple in heap is dead which are not
>>> removed later.  This is already taken care in btree code via
>>> kill_prior_tuple optimization.  So I think clearing of dead tuples can
>>> be handled by a separate patch.
>>
>> That seems like it could work.
>
> I have implemented this idea and it works for MVCC scans.  However, I
> think this might not work for non-MVCC scans.  Consider a case where
> in Process-1, hash scan has returned one row and before it could check
> it's validity in heap, vacuum marks that tuple as dead and removed the
> entry from heap and some new tuple has been placed at that offset in
> heap.

Oops, that's bad.

> Now when Process-1 checks the validity in heap, it will check
> for different tuple then what the index tuple was suppose to check.
> If we want, we can make it work similar to what btree does as being
> discussed on thread [1], but for that we need to introduce page-scan
> mode as well in hash indexes.   However, do we really want to solve
> this problem as part of this patch when this exists for other index am
> as well?

For what other index AM does this problem exist?  Kevin has been
careful not to create this problem for btree, or at least I think he
has.  That's why the pin still has to be held on the index page when
it's a non-MVCC scan.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Commits

  1. Remove _hash_wrtbuf() in favor of calling MarkBufferDirty().

  2. Fix race introduced by 6d46f4783efe457f74816a75173eb23ed8930020.

  3. Improve hash index bucket split behavior.

  4. Reduce pinning and buffer content locking for btree scans.