Re: Hash Indexes

On Wed, Oct 19, 2016 at 5:57 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Tue, Oct 18, 2016 at 10:52 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> 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?
>>
>
> By this problem, I mean to say deadlocks for suspended scans, that can
> happen in btree for non-Mvcc or other type of scans where we don't
> release pin during scan. In my mind, we have below options:
>
> a. problem of deadlocks for suspended scans should be tackled as a
> separate patch as it exists for other indexes (at least for some type
> of scans).
> b. Implement page-scan mode and then we won't have deadlock problem
> for MVCC scans.
> c. Let's not care for non-MVCC scans unless we have some way to hit
> those for hash indexes and proceed with Dead tuple marking idea. I
> think even if we don't care for non-MVCC scans, we might hit this
> problem (deadlocks) when the index relation is unlogged.
>

oops, my last sentence is wrong. What I wanted to say is: "I think
even if we don't care for non-MVCC scans, we might hit the problem of
TIDs reuse when the index relation is unlogged."

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com