27.12.2016 17:33, Amit Kapila:
On Fri, Dec 23, 2016 at 6:42 PM, Anastasia Lubennikova
<a.lubennik...@postgrespro.ru> wrote:
22.12.2016 07:19, Amit Kapila:
On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova
<lubennikov...@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)

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
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

Reply via email to