Re: [HACKERS] Parallel Index Scans

2017-03-06 Thread Gavin Flower
On 07/03/17 02:46, Amit Kapila wrote: On Mon, Mar 6, 2017 at 6:49 PM, Robert Haas wrote: On Mon, Mar 6, 2017 at 6:33 AM, Amit Kapila wrote: I was going to do it after index and index-only scans and parallel bitmap heap scan were committed, but

Re: [HACKERS] Parallel Index Scans

2017-03-06 Thread Amit Kapila
On Mon, Mar 6, 2017 at 6:49 PM, Robert Haas wrote: > On Mon, Mar 6, 2017 at 6:33 AM, Amit Kapila wrote: > > I was going to do it after index and index-only scans and parallel > bitmap heap scan were committed, but I've been holding off on >

Re: [HACKERS] Parallel Index Scans

2017-03-06 Thread Robert Haas
On Mon, Mar 6, 2017 at 6:33 AM, Amit Kapila wrote: > On Mon, Mar 6, 2017 at 4:57 PM, Michael Banck > wrote: >> Hi, >> >> On Thu, Feb 16, 2017 at 08:14:28AM +0530, Amit Kapila wrote: >>> On Thu, Feb 16, 2017 at 12:27 AM, Robert Haas

Re: [HACKERS] Parallel Index Scans

2017-03-06 Thread Amit Kapila
On Mon, Mar 6, 2017 at 4:57 PM, Michael Banck wrote: > Hi, > > On Thu, Feb 16, 2017 at 08:14:28AM +0530, Amit Kapila wrote: >> On Thu, Feb 16, 2017 at 12:27 AM, Robert Haas wrote: >> > On Wed, Feb 15, 2017 at 1:39 PM, Robert Haas

Re: [HACKERS] Parallel Index Scans

2017-03-06 Thread Michael Banck
Hi, On Thu, Feb 16, 2017 at 08:14:28AM +0530, Amit Kapila wrote: > On Thu, Feb 16, 2017 at 12:27 AM, Robert Haas wrote: > > On Wed, Feb 15, 2017 at 1:39 PM, Robert Haas wrote: > >> On Wed, Feb 15, 2017 at 7:11 AM, Amit Kapila

Re: [HACKERS] Parallel Index Scans

2017-02-15 Thread Amit Kapila
On Thu, Feb 16, 2017 at 12:27 AM, Robert Haas wrote: > On Wed, Feb 15, 2017 at 1:39 PM, Robert Haas wrote: >> On Wed, Feb 15, 2017 at 7:11 AM, Amit Kapila >> wrote:> >>> support related patch. In anycase, to avoid

Re: [HACKERS] Parallel Index Scans

2017-02-15 Thread Robert Haas
On Wed, Feb 15, 2017 at 1:39 PM, Robert Haas wrote: > On Wed, Feb 15, 2017 at 7:11 AM, Amit Kapila wrote:> >> support related patch. In anycase, to avoid confusion I am attaching >> all the three patches with this e-mail. > > Committed

Re: [HACKERS] Parallel Index Scans

2017-02-15 Thread Robert Haas
On Wed, Feb 15, 2017 at 7:11 AM, Amit Kapila wrote:> > support related patch. In anycase, to avoid confusion I am attaching > all the three patches with this e-mail. Committed guc_parallel_index_scan_v1.patch, with changes to the documentation and GUC descriptions. --

Re: [HACKERS] Parallel Index Scans

2017-02-15 Thread Robert Haas
On Wed, Feb 15, 2017 at 7:11 AM, Amit Kapila wrote: > Here second part of the comment (but have not yet advanced ..) seems > to be slightly misleading because this state has nothing to do with > the advancement of scan keys. > > I have not changed this because I am not

Re: [HACKERS] Parallel Index Scans

2017-02-15 Thread Amit Kapila
On Wed, Feb 15, 2017 at 2:04 AM, Robert Haas wrote: > On Tue, Feb 14, 2017 at 12:48 PM, Robert Haas wrote: >> That sounds way better. > > Here's an updated patch. Please review my changes, which include: > > * Various comment updates. 1. + *

Re: [HACKERS] Parallel Index Scans

2017-02-14 Thread Robert Haas
On Tue, Feb 14, 2017 at 12:48 PM, Robert Haas wrote: > That sounds way better. Here's an updated patch. Please review my changes, which include: * Various comment updates. * _bt_parallel_seize now unconditionally sets *pageno to P_NONE at the beginning, instead of doing

Re: [HACKERS] Parallel Index Scans

2017-02-14 Thread Robert Haas
On Mon, Feb 13, 2017 at 9:04 PM, Amit Kapila wrote: > I think the comment at that place is not as clear as it should be. So > how about changing it as below: > > Existing comment: > -- > /* > * For parallel scans, get the last page scanned as it

Re: [HACKERS] Parallel Index Scans

2017-02-13 Thread Amit Kapila
On Mon, Feb 13, 2017 at 5:47 PM, Robert Haas wrote: > On Sat, Feb 11, 2017 at 6:35 PM, Amit Kapila wrote: > Why can't we rely on _bt_walk_left? The reason is mentioned in comments, but let me try to explain with some example. When

Re: [HACKERS] Parallel Index Scans

2017-02-13 Thread Robert Haas
On Sat, Feb 11, 2017 at 6:35 PM, Amit Kapila wrote: Why can't we rely on _bt_walk_left? >>> The reason is mentioned in comments, but let me try to explain with >>> some example. When you reach that point of code, it means that either >>> the current page (assume

Re: [HACKERS] Parallel Index Scans

2017-02-11 Thread Amit Kapila
On Fri, Feb 10, 2017 at 11:27 PM, Robert Haas wrote: > On Wed, Feb 1, 2017 at 8:20 AM, Amit Kapila wrote: >>> The hunk in indexam.c looks like a leftover that can be omitted. >> >> It is not a leftover hunk. Earlier, the patch has the same check >>

Re: [HACKERS] Parallel Index Scans

2017-02-11 Thread Amit Kapila
On Sat, Feb 11, 2017 at 9:41 PM, Robert Haas wrote: > On Fri, Feb 10, 2017 at 11:22 PM, Amit Kapila wrote: >>> Why can't we rely on _bt_walk_left? >> >> The reason is mentioned in comments, but let me try to explain with >> some example. When you

Re: [HACKERS] Parallel Index Scans

2017-02-11 Thread Robert Haas
On Fri, Feb 10, 2017 at 11:22 PM, Amit Kapila wrote: >> Why can't we rely on _bt_walk_left? > > The reason is mentioned in comments, but let me try to explain with > some example. When you reach that point of code, it means that either > the current page (assume page

Re: [HACKERS] Parallel Index Scans

2017-02-10 Thread Amit Kapila
On Fri, Feb 10, 2017 at 11:27 PM, Robert Haas wrote: > On Wed, Feb 1, 2017 at 8:20 AM, Amit Kapila wrote: >>> The hunk in indexam.c looks like a leftover that can be omitted. >> >> It is not a leftover hunk. Earlier, the patch has the same check >>

Re: [HACKERS] Parallel Index Scans

2017-02-10 Thread Robert Haas
On Thu, Feb 9, 2017 at 1:33 AM, Amit Kapila wrote: > compute_index_pages_v2.patch - This function extracts the computation > of index pages to be scanned in a separate function and used it in > existing code. You will notice that I have pulled up the logic of >

Re: [HACKERS] Parallel Index Scans

2017-02-10 Thread Robert Haas
On Wed, Feb 1, 2017 at 8:20 AM, Amit Kapila wrote: >> The hunk in indexam.c looks like a leftover that can be omitted. > > It is not a leftover hunk. Earlier, the patch has the same check > btparallelrescan, but based on your comment up thread [1] (Why does >

Re: [HACKERS] Parallel Index Scans

2017-02-10 Thread Robert Haas
On Thu, Feb 9, 2017 at 5:34 AM, Amit Kapila wrote: >> What about parallel CREATE INDEX? The patch currently uses >> min_parallel_relation_size as an input into the optimizer's custom >> cost model. I had wondered if that made sense. Note that another such >> input is the

Re: [HACKERS] Parallel Index Scans

2017-02-09 Thread Amit Kapila
On Thu, Feb 9, 2017 at 12:08 PM, Peter Geoghegan wrote: > On Wed, Feb 8, 2017 at 10:33 PM, Amit Kapila wrote: >> I had some offlist discussion with Robert about the above point and we >> feel that keeping only heap pages for parallel computation might not

Re: [HACKERS] Parallel Index Scans

2017-02-08 Thread Peter Geoghegan
On Wed, Feb 8, 2017 at 10:33 PM, Amit Kapila wrote: > I had some offlist discussion with Robert about the above point and we > feel that keeping only heap pages for parallel computation might not > be future proof as for parallel index only scans there might not be > any

Re: [HACKERS] Parallel Index Scans

2017-02-08 Thread Amit Kapila
On Sat, Feb 4, 2017 at 7:14 AM, Amit Kapila wrote: > On Sat, Feb 4, 2017 at 5:54 AM, Robert Haas wrote: >> On Wed, Feb 1, 2017 at 12:58 AM, Amit Kapila wrote: > >> On balance, I'm somewhat inclined to think that we ought

Re: [HACKERS] Parallel Index Scans

2017-02-03 Thread Amit Kapila
On Sat, Feb 4, 2017 at 5:54 AM, Robert Haas wrote: > On Wed, Feb 1, 2017 at 12:58 AM, Amit Kapila wrote: >> Yeah, I understand that point and I can see there is strong argument >> to do that way, but let's wait and see what others including Robert

Re: [HACKERS] Parallel Index Scans

2017-02-03 Thread Robert Haas
On Wed, Feb 1, 2017 at 12:58 AM, Amit Kapila wrote: > Yeah, I understand that point and I can see there is strong argument > to do that way, but let's wait and see what others including Robert > have to say about this point. It seems to me that you can make an argument

Re: [HACKERS] Parallel Index Scans

2017-02-01 Thread Michael Paquier
On Wed, Feb 1, 2017 at 10:20 PM, Amit Kapila wrote: > makes sense, so changed accordingly. I have moved this patch to CF 2017-03. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Parallel Index Scans

2017-02-01 Thread tushar
On 02/01/2017 06:50 PM, Amit Kapila wrote: Used large table parallel index scans (both forward and backward scans). These tests have been done by Tushar and you can find detailed report up thread [2]. Apart from that, the patch has been tested with TPC-H queries at various scale factors and it

Re: [HACKERS] Parallel Index Scans

2017-02-01 Thread tushar
On 02/01/2017 06:50 PM, Amit Kapila wrote: Used large table parallel index scans (both forward and backward scans). These tests have been done by Tushar and you can find detailed report up thread [2]. Apart from that, the patch has been tested with TPC-H queries at various scale factors and it

Re: [HACKERS] Parallel Index Scans

2017-02-01 Thread Amit Kapila
On Wed, Feb 1, 2017 at 3:52 AM, Robert Haas wrote: > On Tue, Jan 24, 2017 at 4:51 PM, Robert Haas wrote: >> On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila wrote: >>> In spite of being careful, I missed reorganizing the

Re: [HACKERS] Parallel Index Scans

2017-01-31 Thread Rahila Syed
Hello Robert, >I am a bit mystified about how this manages to work with array keys. >_bt_parallel_done() won't set btps_pageStatus to BTPARALLEL_DONE >unless so->arrayKeyCount >= btscan->btps_arrayKeyCount, but >_bt_parallel_advance_scan() won't do anything unless btps_pageStatus >is already

Re: [HACKERS] Parallel Index Scans

2017-01-31 Thread Amit Kapila
On Tue, Jan 31, 2017 at 5:53 PM, Rahila Syed wrote: > Hello, > >>Agreed, that it makes sense to consider only the number of pages to >>scan for computation of parallel workers. I think for index scan we >>should consider both index and heap pages that need to be scanned

Re: [HACKERS] Parallel Index Scans

2017-01-31 Thread Robert Haas
On Tue, Jan 24, 2017 at 4:51 PM, Robert Haas wrote: > On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila wrote: >> In spite of being careful, I missed reorganizing the functions in >> genam.h which I have done in attached patch. > > Cool. Committed

Re: [HACKERS] Parallel Index Scans

2017-01-31 Thread Rahila Syed
Hello, >Agreed, that it makes sense to consider only the number of pages to >scan for computation of parallel workers. I think for index scan we >should consider both index and heap pages that need to be scanned >(costing of index scan consider both index and heap pages). I thin >where

Re: [HACKERS] Parallel Index Scans

2017-01-30 Thread Amit Kapila
On Sat, Jan 28, 2017 at 1:34 AM, Robert Haas wrote: > On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila wrote: >> parallel_index_opt_exec_support_v6 - Removed the usage of >> GatherSupportsBackwardScan. Expanded the comments in >> ExecReScanIndexScan.

Re: [HACKERS] Parallel Index Scans

2017-01-27 Thread Robert Haas
On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila wrote: > parallel_index_opt_exec_support_v6 - Removed the usage of > GatherSupportsBackwardScan. Expanded the comments in > ExecReScanIndexScan. I looked through this and in general it looks reasonable to me. However, I did

Re: [HACKERS] Parallel Index Scans

2017-01-26 Thread Amit Kapila
On Fri, Jan 27, 2017 at 6:53 AM, Haribabu Kommi wrote: > > > On Mon, Jan 23, 2017 at 5:07 PM, Amit Kapila > wrote: >> >> On Fri, Jan 20, 2017 at 7:29 AM, Haribabu Kommi >> wrote: >> > I didn't find any better names

Re: [HACKERS] Parallel Index Scans

2017-01-26 Thread Haribabu Kommi
On Mon, Jan 23, 2017 at 5:07 PM, Amit Kapila wrote: > On Fri, Jan 20, 2017 at 7:29 AM, Haribabu Kommi > wrote: > > > > On Thu, Jan 19, 2017 at 1:18 AM, Amit Kapila > > wrote: > >> > > >> > +extern BlockNumber

Re: [HACKERS] Parallel Index Scans

2017-01-24 Thread Robert Haas
On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila wrote: > In spite of being careful, I missed reorganizing the functions in > genam.h which I have done in attached patch. Cool. Committed parallel-generic-index-scan.2.patch. Thanks. -- Robert Haas EnterpriseDB:

Re: [HACKERS] Parallel Index Scans

2017-01-22 Thread Amit Kapila
On Fri, Jan 20, 2017 at 7:29 AM, Haribabu Kommi wrote: > > On Thu, Jan 19, 2017 at 1:18 AM, Amit Kapila > wrote: >> > >> > +extern BlockNumber _bt_parallel_seize(IndexScanDesc scan, bool >> > *status); >> > +extern void

Re: [HACKERS] Parallel Index Scans

2017-01-22 Thread Amit Kapila
On Sat, Jan 21, 2017 at 12:23 PM, Amit Kapila wrote: > On Sat, Jan 21, 2017 at 1:15 AM, Robert Haas wrote: >> >> I think it's a good idea to put all three of those functions together >> in the listing, similar to what we did in >>

Re: [HACKERS] Parallel Index Scans

2017-01-20 Thread Amit Kapila
On Sat, Jan 21, 2017 at 1:15 AM, Robert Haas wrote: > On Fri, Jan 20, 2017 at 9:29 AM, Amit Kapila wrote: >> Sure, if scan keys have changed then we can't continue, but this is >> the case where runtime keys are first time initialized. >> >> if

Re: [HACKERS] Parallel Index Scans

2017-01-20 Thread Robert Haas
On Fri, Jan 20, 2017 at 9:29 AM, Amit Kapila wrote: > Sure, if scan keys have changed then we can't continue, but this is > the case where runtime keys are first time initialized. > > if (node->iss_NumRuntimeKeys != 0 && !node->iss_RuntimeKeysReady) > > In the above

Re: [HACKERS] Parallel Index Scans

2017-01-20 Thread Amit Kapila
On Fri, Jan 20, 2017 at 3:41 AM, Robert Haas wrote: > On Wed, Jan 18, 2017 at 8:03 AM, Amit Kapila wrote: >>> Why do we need separate AM support for index_parallelrescan()? Why >>> can't index_rescan() cover that case? >> >> The reason is that

Re: [HACKERS] Parallel Index Scans

2017-01-20 Thread Amit Kapila
On Fri, Jan 20, 2017 at 12:59 AM, Robert Haas wrote: > On Thu, Jan 19, 2017 at 4:26 AM, Amit Kapila wrote: >> Fixed. > > > If all of that were no issue, the considerations in > TargetListSupportsBackwardScan could be a problem, too. But I think >

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Haribabu Kommi
On Thu, Jan 19, 2017 at 1:18 AM, Amit Kapila wrote: > On Wed, Jan 18, 2017 at 6:25 AM, Haribabu Kommi > wrote: > > > > > > On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila > > wrote: > >> > > > > + *

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Robert Haas
On Wed, Jan 18, 2017 at 8:03 AM, Amit Kapila wrote: >> Why do we need separate AM support for index_parallelrescan()? Why >> can't index_rescan() cover that case? > > The reason is that sometime index_rescan is called when we have to > just update runtime scankeys in

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Robert Haas
On Thu, Jan 19, 2017 at 4:26 AM, Amit Kapila wrote: > Fixed. I think that the whole idea of GatherSupportsBackwardScan is wrong. Gather doesn't support a backwards scan under any circumstances, whether the underlying node does or not. You can read the tuples once, in

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Amit Kapila
On Thu, Jan 19, 2017 at 12:28 AM, Robert Haas wrote: > On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: >> Fixed. > > With respect to the second patch > (parallel_index_opt_exec_support_v4.patch), I'm hoping it can use the > new function from

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Amit Kapila
On Wed, Jan 18, 2017 at 6:25 AM, Haribabu Kommi wrote: > > > On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila > wrote: >> >> >> Changed as per suggestion. >> >> >> I have also rebased the optimizer/executor support patch >>

Re: [HACKERS] Parallel Index Scans

2017-01-19 Thread Amit Kapila
On Tue, Jan 17, 2017 at 11:27 PM, Robert Haas wrote: > On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: >> Fixed. > > Thanks for the update. Some more comments: > > It shouldn't be necessary for MultiExecBitmapIndexScan to modify the >

Re: [HACKERS] Parallel Index Scans

2017-01-18 Thread Haribabu Kommi
On Wed, Jan 18, 2017 at 6:55 PM, Rahila Syed wrote: > >+ /* Check if the scan for current scan keys is finished */ > >+ if (so->arrayKeyCount < btscan->btps_arrayKeyCount) > >+ *status = false; > > >I didn't clearly understand, in which scenario the arrayKeyCount is less

Re: [HACKERS] Parallel Index Scans

2017-01-18 Thread Robert Haas
On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: > Fixed. With respect to the second patch (parallel_index_opt_exec_support_v4.patch), I'm hoping it can use the new function from Dilip's bitmap heap scan patch set. See commit 716c7d4b242f0a64ad8ac4dc48c6fed6557ba12c.

Re: [HACKERS] Parallel Index Scans

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 8:03 AM, Amit Kapila wrote: > On Tue, Jan 17, 2017 at 11:27 PM, Robert Haas wrote: >> On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: >> WAIT_EVENT_PARALLEL_INDEX_SCAN is in fact btree-specific.

Re: [HACKERS] Parallel Index Scans

2017-01-18 Thread Amit Kapila
On Wed, Jan 18, 2017 at 6:25 AM, Haribabu Kommi wrote: > > > On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila > wrote: >> > > + * index_beginscan_parallel - join parallel index scan > > The name and the description doesn't sync properly, any better

Re: [HACKERS] Parallel Index Scans

2017-01-18 Thread Amit Kapila
On Tue, Jan 17, 2017 at 11:27 PM, Robert Haas wrote: > On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: > > > WAIT_EVENT_PARALLEL_INDEX_SCAN is in fact btree-specific. There's no > guarantee that any other AMs the implement parallel index scans

Re: [HACKERS] Parallel Index Scans

2017-01-17 Thread Rahila Syed
>+ /* Check if the scan for current scan keys is finished */ >+ if (so->arrayKeyCount < btscan->btps_arrayKeyCount) >+ *status = false; >I didn't clearly understand, in which scenario the arrayKeyCount is less >than btps_arrayKeyCount? Consider following array scan keys select * from test2 where

Re: [HACKERS] Parallel Index Scans

2017-01-17 Thread Haribabu Kommi
On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila wrote: > > Changed as per suggestion. > > > I have also rebased the optimizer/executor support patch > (parallel_index_opt_exec_support_v4.patch) and added a test case in > it. Thanks for the patch. Here are comments found

Re: [HACKERS] Parallel Index Scans

2017-01-17 Thread Robert Haas
On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila wrote: > Fixed. Thanks for the update. Some more comments: It shouldn't be necessary for MultiExecBitmapIndexScan to modify the IndexScanDesc. That seems really broken. If a parallel scan isn't supported here (and I'm sure

Re: [HACKERS] Parallel Index Scans

2017-01-16 Thread Amit Kapila
On Fri, Jan 13, 2017 at 11:06 PM, Robert Haas wrote: > On Fri, Jan 13, 2017 at 9:28 AM, Anastasia Lubennikova > wrote: >> I didn't find any case of noticeable performance degradation, >> so set status to "Ready for committer". > > The very

Re: [HACKERS] Parallel Index Scans

2017-01-14 Thread Amit Kapila
On Fri, Jan 13, 2017 at 7:58 PM, Anastasia Lubennikova wrote: > 27.12.2016 17:33, Amit Kapila: > > > 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

Re: [HACKERS] Parallel Index Scans

2017-01-13 Thread Robert Haas
On Fri, Jan 13, 2017 at 9:28 AM, Anastasia Lubennikova wrote: > I didn't find any case of noticeable performance degradation, > so set status to "Ready for committer". The very first hunk of doc changes looks like it makes the whitespace totally wrong - surely it

Re: [HACKERS] Parallel Index Scans

2017-01-13 Thread Anastasia Lubennikova
27.12.2016 17:33, Amit Kapila: On Fri, Dec 23, 2016 at 6:42 PM, Anastasia Lubennikova wrote: 22.12.2016 07:19, Amit Kapila: On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova wrote: The following review has been posted through the

Re: [HACKERS] Parallel Index Scans

2016-12-27 Thread Amit Kapila
On Fri, Dec 23, 2016 at 5:48 PM, Rahila Syed wrote: >>> 5. Comment for _bt_parallel_seize() says: >>> "False indicates that we have reached the end of scan for >>> current scankeys and for that we return block number as P_NONE." >>> >>> What is the reason to check (blkno

Re: [HACKERS] Parallel Index Scans

2016-12-27 Thread Amit Kapila
On Fri, Dec 23, 2016 at 6:42 PM, Anastasia Lubennikova wrote: > 22.12.2016 07:19, Amit Kapila: >> >> On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova >> wrote: >>> >>> The following review has been posted through the commitfest

Re: [HACKERS] Parallel Index Scans

2016-12-23 Thread Anastasia Lubennikova
22.12.2016 07:19, Amit Kapila: On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova wrote: The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant:

Re: [HACKERS] Parallel Index Scans

2016-12-23 Thread Rahila Syed
>> 5. Comment for _bt_parallel_seize() says: >> "False indicates that we have reached the end of scan for >> current scankeys and for that we return block number as P_NONE." >> >> What is the reason to check (blkno == P_NONE) after checking (status == false) >> in _bt_first() (see code below)?

Re: [HACKERS] Parallel Index Scans

2016-12-23 Thread tushar
On 12/23/2016 05:38 PM, Robert Haas wrote: So why are you reporting it here rather than on a separate thread? We found it -while testing parallel index scan and later it turned out to be crash in general. Sure- make sense ,will do that. -- regards,tushar -- Sent via pgsql-hackers mailing

Re: [HACKERS] Parallel Index Scans

2016-12-23 Thread Robert Haas
On Fri, Dec 23, 2016 at 1:35 AM, tushar wrote: > In addition to that, we run the sqlsmith against PG v10+PIS (parallel index > scan) patches and found a crash but that is coming on plain PG v10 > (without applying any patches) as well So why are you reporting it

Re: [HACKERS] Parallel Index Scans

2016-12-22 Thread tushar
On 12/22/2016 01:35 PM, tushar wrote: On 12/22/2016 09:49 AM, Amit Kapila wrote: 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

Re: [HACKERS] Parallel Index Scans

2016-12-21 Thread Amit Kapila
On Thu, Dec 22, 2016 at 9:49 AM, Amit Kapila wrote: > On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova > wrote: >> The following review has been posted through the commitfest application: >> make installcheck-world: tested, passed >>

Re: [HACKERS] Parallel Index Scans

2016-12-21 Thread Amit Kapila
On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova wrote: > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: tested, passed >

Re: [HACKERS] Parallel Index Scans

2016-12-21 Thread Robert Haas
Thanks for reviewing! A few quick thoughts from me since I write a bunch of the design for this patch. On Wed, Dec 21, 2016 at 10:16 AM, Anastasia Lubennikova wrote: > 1. Can't we simply use "if (scan->parallel_scan != NULL)" instead of > xs_temp_snap flag? > > +

Re: [HACKERS] Parallel Index Scans

2016-12-21 Thread Anastasia Lubennikova
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

Re: [HACKERS] Parallel Index Scans

2016-12-04 Thread Haribabu Kommi
On Sat, Nov 26, 2016 at 10:35 PM, Amit Kapila wrote: > On Sat, Oct 22, 2016 at 9:07 AM, Amit Kapila > wrote: > > On Fri, Oct 21, 2016 at 10:55 PM, Robert Haas > wrote: > > I have rebased the patch (parallel_index_scan_v2)

Re: [HACKERS] Parallel Index Scans

2016-11-26 Thread Amit Kapila
On Sat, Oct 22, 2016 at 9:07 AM, Amit Kapila wrote: > On Fri, Oct 21, 2016 at 10:55 PM, Robert Haas wrote: I have rebased the patch (parallel_index_scan_v2) based on latest commit e8ac886c (condition variables). I have removed the usage of

Re: [HACKERS] Parallel Index Scans

2016-10-21 Thread Amit Kapila
On Fri, Oct 21, 2016 at 10:55 PM, Robert Haas wrote: > On Fri, Oct 21, 2016 at 9:27 AM, Amit Kapila wrote: >>> I think the parallel_workers reloption should override the degree of >>> parallelism for any sort of parallel scan on that table. Had I

Re: [HACKERS] Parallel Index Scans

2016-10-21 Thread Robert Haas
On Fri, Oct 21, 2016 at 9:27 AM, Amit Kapila wrote: >> I think the parallel_workers reloption should override the degree of >> parallelism for any sort of parallel scan on that table. Had I >> intended it to apply only to sequential scans, I would have named it >>

Re: [HACKERS] Parallel Index Scans

2016-10-21 Thread Amit Kapila
On Thu, Oct 20, 2016 at 10:33 PM, Robert Haas wrote: > On Wed, Oct 19, 2016 at 11:07 PM, Amit Kapila wrote: >>> Ideally, the parallel_workers storage parameter will rarely be >>> necessary because the optimizer will generally do the right thing in

Re: [HACKERS] Parallel Index Scans

2016-10-20 Thread Robert Haas
On Wed, Oct 19, 2016 at 11:07 PM, Amit Kapila wrote: >> Ideally, the parallel_workers storage parameter will rarely be >> necessary because the optimizer will generally do the right thing in >> all case. > > Yeah, we can choose not to provide any parameter for parallel

Re: [HACKERS] Parallel Index Scans

2016-10-19 Thread Peter Geoghegan
On Wed, Oct 19, 2016 at 8:07 PM, Amit Kapila wrote: > I have also checked and found that you are right. In SQL Server, they > are using max degree of parallelism (MAXDOP) parameter which is I > think is common for all the sql statements. It's not just that one that does

Re: [HACKERS] Parallel Index Scans

2016-10-19 Thread Amit Kapila
On Thu, Oct 20, 2016 at 7:39 AM, Peter Geoghegan wrote: > On Mon, Oct 17, 2016 at 8:08 PM, Amit Kapila wrote: >> Create Index With (parallel_workers = 4); >> >> If above syntax looks sensible, then we might need to think what >> should be used for

Re: [HACKERS] Parallel Index Scans

2016-10-19 Thread Peter Geoghegan
On Mon, Oct 17, 2016 at 8:08 PM, Amit Kapila wrote: > Create Index With (parallel_workers = 4); > > If above syntax looks sensible, then we might need to think what > should be used for parallel index build. It seems to me that parallel > tuple sort patch [1]

Re: [HACKERS] Parallel Index Scans

2016-10-18 Thread Amit Kapila
On Tue, Oct 18, 2016 at 4:08 PM, Rahila Syed wrote: >>Another point which needs some thoughts is whether it is good idea to >>use index relation size to calculate parallel workers for index scan. >>I think ideally for index scans it should be based on number of pages >>to

Re: [HACKERS] Parallel Index Scans

2016-10-18 Thread Rahila Syed
>Another point which needs some thoughts is whether it is good idea to >use index relation size to calculate parallel workers for index scan. >I think ideally for index scans it should be based on number of pages >to be fetched/scanned from index. IIUC, its not possible to know the exact number of

Re: [HACKERS] Parallel Index Scans

2016-10-17 Thread Amit Kapila
On Thu, Oct 13, 2016 at 8:48 AM, Amit Kapila wrote: > As of now, the driving table for parallel query is accessed by > parallel sequential scan which limits its usage to a certain degree. > Parallelising index scans would further increase the usage of parallel > query in

[HACKERS] Parallel Index Scans

2016-10-12 Thread Amit Kapila
As of now, the driving table for parallel query is accessed by parallel sequential scan which limits its usage to a certain degree. Parallelising index scans would further increase the usage of parallel query in many more cases. This patch enables the parallelism for the btree scans. Supporting