Re: Parallel Index Scans

27.12.2016 17:33, Amit Kapila:
> On Fri, Dec 23, 2016 at 6:42 PM, Anastasia Lubennikova
> <a.lubennikova@postgrespro.ru> wrote:
>> 22.12.2016 07:19, Amit Kapila:
>>> On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova
>>> <lubennikovaav@gmail.com> wrote:
>>>> The following review has been posted through the commitfest application:
>>>> make installcheck-world:  tested, passed
>>>> Implements feature:       tested, passed
>>>> Spec compliant:           tested, passed
>>>> Documentation:            tested, passed
>>>>
>>>> Hi, thank you for the patch.
>>>> Results are very promising. Do you see any drawbacks of this feature or
>>>> something that requires more testing?
>>>>
>>> I think you can focus on the handling of array scan keys for testing.
>>> In general, one of my colleagues has shown interest in testing this
>>> patch and I think he has tested as well but never posted his findings.
>>> I will request him to share his findings and what kind of tests he has
>>> done, if any.
>>
>>
>> Check please code related to buffer locking and pinning once again.
>> I got the warning. Here are the steps to reproduce it:
>> Except "autovacuum = off" config is default.
>>
>> pgbench -i -s 100 test
>> pgbench -c 10 -T 120 test
>>
>>      SELECT count(aid) FROM pgbench_accounts
>>      WHERE aid > 1000 AND aid < 900000 AND bid > 800 AND bid < 900;
>> WARNING:  buffer refcount leak: [8297] (rel=base/12289/16459, blockNum=2469,
>> flags=0x93800000, refcount=1 1)
>>   count
>>
> The similar problem has occurred while testing "parallel index only
> scan" patch and Rafia has included the fix in her patch [1] which
> ideally should be included in this patch, so I have copied the fix
> from her patch.  Apart from that, I observed that similar problem can
> happen for backward scans, so fixed the same as well.

I confirm that this problem is solved.

>> But I'm trying to find the worst cases for this feature. And I suppose we
>> should test parallel index scans with
>> concurrent insertions. More parallel readers we have, higher the
>> concurrency.
>> I doubt that it can significantly decrease performance, because number of
>> parallel readers is not that big,
>>
> I am not sure if such a test is meaningful for this patch because
> parallelism is generally used for large data reads and in such cases
> there are generally not many concurrent writes.

I didn't find any case of noticeable performancedegradation,
so set status to "Ready for committer".
Thank you for this patch.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company