Re: Hash Indexes

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Hash Indexes
Date: 2016-10-18 17:22:59
Message-ID: CA+TgmoaR-h-7KR5E1QvkUst0VatwzDwe3a_x_Soz3SMBU-YTFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 18, 2016 at 5:37 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Wed, Oct 5, 2016 at 10:22 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Tue, Oct 4, 2016 at 12:36 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-10-18 17:36:09 Re: Aggregate Push Down - Performing aggregation on foreign server
Previous Message Tom Lane 2016-10-18 17:21:38 Re: Multiple psql history files