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
