Re: [HACKERS] GSoC 2017: weekly progress reports (week 4) and patch for hash index

On Sat, Mar 10, 2018 at 1:10 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Tue, Mar 6, 2018 at 11:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Mon, Mar 5, 2018 at 8:58 AM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>>> On Sun, Mar 4, 2018 at 12:53 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>> Yes, but I think it would be better if we call this once we are sure
>>>> that at least one tuple from the old bucket has been transferred
>>>> (consider if all tuples in the old bucket are dead). Apart from this,
>>>> I think this patch has missed handling the cases where we scan the
>>>> buckets when the split is in progress. In such cases, we scan both
>>>> old and new bucket, so I think we need to ensure that we take
>>>> PredicateLock on both the buckets during such scans.
>>>
>>> Hmm. Yeah.
>>>
>>> So, in _hash_first(), do you think we might just need this?
>>>
>>> if (H_BUCKET_BEING_POPULATED(opaque))
>>> {
>>> ...
>>> old_blkno = _hash_get_oldblock_from_newbucket(rel, bucket);
>>> ...
>>> old_buf = _hash_getbuf(rel, old_blkno, HASH_READ, LH_BUCKET_PAGE);
>>> + PredicateLockPage(rel, BufferGetBlockNumber(old_buf),
>>> scan->xs_snapshot);
>>> TestForOldSnapshot(scan->xs_snapshot, rel, BufferGetPage(old_buf));
>>>
>>> That is, if you begin scanning a 'new' bucket, we remember the old
>>> bucket and go and scan that too, so we'd better predicate-lock both up
>>> front (or I suppose we could do it later when we visit that page, but
>>> here it can be done in a single place).
>>>
>>
>> Yeah, that can work, but I am slightly worried that we might actually
>> never scan the old bucket (say for queries with Limit clause) in which
>> case it might give false positives for insertions in old buckets.
>>
>
> I have changed the patch to address this point by acquiring predicate
> lock in _hash_readnext where it will acquire the lock only when it
> tries to scan the old bucket. I have also addressed another problem
> related to transfer of predicate locks during split such that it will
> transfer locks only when there is any tuple transferred from old to
> the new bucket.
>

Added some additional text in README in the attached patch to explain
the new change in mechanism.

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