Re: Parallel seq. plan is not coming against inheritance or partition table

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>
Subject: Re: Parallel seq. plan is not coming against inheritance or partition table
Date: 2017-03-13 14:18:51
Message-ID: CAA4eK1+RmRBpy+xNBhJKO=XDW5nUpe_+m_0vP5D23-U-2Ycwww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 8, 2017 at 6:58 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Mar 8, 2017 at 7:04 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Wed, Mar 8, 2017 at 8:28 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Tue, Mar 7, 2017 at 9:24 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>> I think it can give us benefit in
>>>> such cases as well (especially when we have to discard rows based heap
>>>> rows). Now, consider it from another viewpoint, what if there are
>>>> enough index pages (> min_parallel_index_scan_size) but not sufficient
>>>> heap pages. I think in such cases parallel index (only) scans will be
>>>> beneficial especially because in the parallel index only scans
>>>> heap_pages could be very less or possibly could be zero.
>>>
>>> That's a separate problem. I think we ought to consider having an
>>> index-only scan pass -1 for the number of heap pages, so that the
>>> degree of parallelism in that case is limited only by the number of
>>> index pages.
>>
>> Sure, that sounds sensible, but even after that, I am not sure if for
>> plain index scans it is a good idea to not choose parallelism if the
>> number of heap pages is lesser than min_parallel_table_scan_size even
>> though the number of index pages is greater than
>> min_parallel_index_scan_size. I think we can try out some tests
>> (maybe TPC-H queries where parallel index scan gets picked up) to see
>> what is right here. Do you think it will be bad if just commit your
>> patch without this change and then consider changing it separately?
>
> No, I think that would be fine. I think that we need to commit
> something like what I proposed because the earlier commit broke
> something that used to work. That's got to get fixed.
>

Agreed, so I have rebased your patch and passed heap_pages as -1 for
index_only scans as discussed. Also, Rafia has tested with attached
patch that parallel index and parallel index only scans are picked for
TPC-H queries. I have also reviewed and tested your changes with
respect to problem reported and found that it works as expected. So,
I think we can go ahead with attached patch unless you see some
problem with the changes I have made.

The only remaining open item about parallel index scans is a decision
about storage parameter which is being discussed on parallel index
scan thread [1], if you and or others can share feedback, then we can
proceed on that aspect as well.

[1] - https://www.postgresql.org/message-id/44cd4d75-41f2-8e63-e204-1ecb09127fbf%40archidevsys.co.nz

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

Attachment Content-Type Size
compute-parallel-worker-fix.1.patch application/octet-stream 5.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-03-13 14:19:37 Re: [COMMITTERS] pgsql: Improve postmaster's logging of listen socket creation.
Previous Message Robert Haas 2017-03-13 14:18:10 Re: Parallel Append implementation