Re: Page Scan Mode in Hash Index

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Page Scan Mode in Hash Index
Date: 2017-08-22 10:25:12
Message-ID: CAA4eK1KMXh2zqkjb4uu+yc8UNVQTyFeTjZreziDEsV3idy0mnQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 22, 2017 at 2:28 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> On Sat, Aug 19, 2017 at 11:37 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Fri, Aug 11, 2017 at 6:51 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>>>>
>>>> 7.
>>>> _hash_kill_items(IndexScanDesc scan)
>>>> {
>>>> ..
>>>> + /*
>>>> + * If page LSN differs it means that the page was modified since the
>>>> + * last read. killedItems could be not valid so LP_DEAD hints apply-
>>>> + * ing is not safe.
>>>> + */
>>>> + page = BufferGetPage(buf);
>>>> + if (PageGetLSN(page) != so->currPos.lsn)
>>>> + {
>>>> + _hash_relbuf(rel, buf);
>>>> + return;
>>>> + }
>>>> ..
>>>> }
>>>>
>>>> How does this check cover the case of unlogged tables?
>>>
>>> Thanks for putting that point. It doesn't cover the case for unlogged
>>> tables. As suggested by you in one of your email in this mailing list, i am
>>> now not allowing vacuum to release lock on current page before acquiring
>>> lock on next page for unlogged tables. This will ensure that scan is always
>>> behind vacuum if they are running on the same bucket simultaneously.
>>> Therefore, there is danger in marking tuples as dead for unlogged pages even
>>> if they are not having any lsn.
>>>
>>
>
> Once again, Thank you for reviewing my patches.
>
>> In the last line, I guess you wanted to say "there is *no* danger
>> ..."?
>
> Yes, i meant that because, it ensures that scan will always be following VACUUM.
>
> Today, while again thinking about this part of the patch
>> (_hash_kill_items) it occurred to me that we can't rely on a pin on an
>> overflow page to guarantee that it is not modified by Vacuum.
>> Consider a case where vacuum started vacuuming the bucket before the
>> scan and then in-between scan overtakes it. Now, it is possible that
>> even if a scan has a pin on a page (and no lock), vacuum might clean
>> that page, if that happens, then we can't prevent the reuse of TID.
>> What do you think?
>>
>
> I think, you are talking about non-mvcc scan case, because in case of
> mvcc scans, even if we have released both pin and lock on a page,
> VACUUM can't remove tuples from a page if it is visible to some
> concurrently running transactions (mvcc scan in our case).
>

I am talking about tuples that are marked as dead in heap. It has
nothing to do with the visibility of tuple or type of scan (mvcc or
non-mvcc).

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Beena Emerson 2017-08-22 10:50:20 Re: Default Partition for Range
Previous Message Pavel Stehule 2017-08-22 09:29:16 Re: proposal: psql command \graw