Re: [HACKERS] Parallel Bitmap Heap Scan - Prefetch pages are not updated properly

2017-04-04 Thread Robert Haas
On Tue, Apr 4, 2017 at 6:24 AM, Dilip Kumar wrote: > While analyzing the coverage for the prefetching part, I found an > issue that prefetch_pages were not updated properly while executing in > parallel mode. > > Attached patch fixes the same. Wow, OK. Committed. --

[HACKERS] Parallel Bitmap Heap Scan - Prefetch pages are not updated properly

2017-04-04 Thread Dilip Kumar
While analyzing the coverage for the prefetching part, I found an issue that prefetch_pages were not updated properly while executing in parallel mode. Attached patch fixes the same. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com parallel_bitmap_prefetch_fix.patch

Re: [HACKERS] Parallel bitmap heap scan

2017-03-27 Thread Dilip Kumar
On Mon, Mar 27, 2017 at 5:58 PM, Robert Haas wrote: > Failing to pass DSA_ALLOC_HUGE is an obvious oversight, but I think > the ptbase == NULL check shouldn't be needed, because we're not > passing DSA_ALLOC_NO_OOM. And that's good, because this is going to > be called

Re: [HACKERS] Parallel bitmap heap scan

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 5:02 AM, Dilip Kumar wrote: > On Mon, Mar 27, 2017 at 12:53 PM, Rafia Sabih > wrote: >> Recently, on testing TPC-H 300 scale factor I stumbled on to a error >> for Q6, the test environment is as follows, >> work_mem =

Re: [HACKERS] Parallel bitmap heap scan

2017-03-27 Thread Dilip Kumar
On Mon, Mar 27, 2017 at 12:53 PM, Rafia Sabih wrote: > Recently, on testing TPC-H 300 scale factor I stumbled on to a error > for Q6, the test environment is as follows, > work_mem = 1GB, > shared_buffers = 10 GB, > Effective_cache_size = 10GB > random_page_cost =

Re: [HACKERS] Parallel bitmap heap scan

2017-03-27 Thread Rafia Sabih
On Wed, Mar 8, 2017 at 11:52 PM, Robert Haas wrote: > On Wed, Mar 8, 2017 at 1:18 PM, Tom Lane wrote: >> Robert Haas writes: >>> What I'm using is: >> >>> Configured with: >>>

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
On Wed, Mar 8, 2017 at 1:18 PM, Tom Lane wrote: > Robert Haas writes: >> What I'm using is: > >> Configured with: >> --prefix=/Applications/Xcode.app/Contents/Developer/usr >> --with-gxx-include-dir=/usr/include/c++/4.2.1 >> Apple LLVM version 7.0.2

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Tom Lane
Robert Haas writes: > What I'm using is: > Configured with: > --prefix=/Applications/Xcode.app/Contents/Developer/usr > --with-gxx-include-dir=/usr/include/c++/4.2.1 > Apple LLVM version 7.0.2 (clang-700.1.81) > Target: x86_64-apple-darwin14.5.0 > Thread model: posix Hm.

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
On Wed, Mar 8, 2017 at 12:53 PM, Tom Lane wrote: >> Thanks. Sorry for the hassle; my compiler isn't as picky about this >> as I would like, and apparently Dilip's isn't either. > > Might be interesting to see whether -O level affects it. In principle, > whether you get the

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Tom Lane
Robert Haas writes: > On Wed, Mar 8, 2017 at 12:45 PM, Tom Lane wrote: >> Jeff Janes writes: >>> I'm getting some compiler warnings in gcc version 4.4.7 20120313 (Red Hat >>> 4.4.7-17) (GCC): >> Me too. Fix pushed. > Thanks.

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
On Wed, Mar 8, 2017 at 12:45 PM, Tom Lane wrote: > Jeff Janes writes: >> I'm getting some compiler warnings in gcc version 4.4.7 20120313 (Red Hat >> 4.4.7-17) (GCC): > > Me too. Fix pushed. Thanks. Sorry for the hassle; my compiler isn't as picky

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Tom Lane
Jeff Janes writes: > I'm getting some compiler warnings in gcc version 4.4.7 20120313 (Red Hat > 4.4.7-17) (GCC): Me too. Fix pushed. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Jeff Janes
On Wed, Mar 8, 2017 at 9:08 AM, Robert Haas wrote: > On Wed, Mar 8, 2017 at 11:20 AM, Dilip Kumar > wrote: > > Right, done that way > > This didn't compile because you bobbled some code in > src/backend/nodes, but it was a trivial mistake so I fixed

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
On Wed, Mar 8, 2017 at 11:20 AM, Dilip Kumar wrote: > Right, done that way This didn't compile because you bobbled some code in src/backend/nodes, but it was a trivial mistake so I fixed it. Committed with that fix and a bunch of minor cosmetic changes. -- Robert Haas

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Dilip Kumar
On Wed, Mar 8, 2017 at 8:28 PM, Robert Haas wrote: > How about adding a regression test? Added > > bitmap_subplan_mark_shared could use castNode(), which seems like it > would be better style. Maybe some other places, too. > > + ParallelBitmapPopulate > +

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
Reviewing 0003: How about adding a regression test? bitmap_subplan_mark_shared could use castNode(), which seems like it would be better style. Maybe some other places, too. + ParallelBitmapPopulate + Waiting for the leader to populate the TidBitmap. + + If you

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Dilip Kumar
On Wed, Mar 8, 2017 at 6:42 PM, Robert Haas wrote: > I don't think I understand exactly why this system might be prone to a > little bit of extra prefetching - can you explain further? Let me explain with an example, suppose there are 2 workers prefetching jointly, lets

Re: [HACKERS] Parallel bitmap heap scan

2017-03-08 Thread Robert Haas
On Wed, Mar 8, 2017 at 1:49 AM, Dilip Kumar wrote: > On Tue, Mar 7, 2017 at 10:10 PM, Dilip Kumar wrote: >>> It's not about speed. It's about not forgetting to prefetch. Suppose >>> that worker 1 becomes the prefetch worker but then doesn't return

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Dilip Kumar
On Tue, Mar 7, 2017 at 10:10 PM, Dilip Kumar wrote: >> It's not about speed. It's about not forgetting to prefetch. Suppose >> that worker 1 becomes the prefetch worker but then doesn't return to >> the Bitmap Heap Scan node for a long time because it's busy in some >>

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Dilip Kumar
On Tue, Mar 7, 2017 at 10:07 PM, Robert Haas wrote: > It's not about speed. It's about not forgetting to prefetch. Suppose > that worker 1 becomes the prefetch worker but then doesn't return to > the Bitmap Heap Scan node for a long time because it's busy in some > other

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Robert Haas
On Tue, Mar 7, 2017 at 11:27 AM, Dilip Kumar wrote: > On Tue, Mar 7, 2017 at 9:44 PM, Robert Haas wrote: >> I mean, IIUC, the call to PrefetchBuffer() is not done under any lock. >> And that's the slow part. The tiny amount of time we spend updating

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Dilip Kumar
On Tue, Mar 7, 2017 at 9:44 PM, Robert Haas wrote: > I mean, IIUC, the call to PrefetchBuffer() is not done under any lock. > And that's the slow part. The tiny amount of time we spend updating > the prefetch information under the mutex should be insignificant > compared

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Robert Haas
On Tue, Mar 7, 2017 at 11:09 AM, Dilip Kumar wrote: > On Tue, Mar 7, 2017 at 9:34 PM, Robert Haas wrote: >> >> + if (DsaPointerIsValid(node->pstate->tbmiterator)) >> + tbm_free_shared_area(dsa,

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Robert Haas
(On Tue, Feb 28, 2017 at 10:48 AM, Dilip Kumar wrote: > 0001- same as previous with some changes for freeing the shared memory stuff. +if (--ptbase->refcount == 0) +dsa_free(dsa, istate->pagetable); + +if (istate->spages) +{ +ptpages =

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Dilip Kumar
On Tue, Mar 7, 2017 at 9:34 PM, Robert Haas wrote: > > + if (DsaPointerIsValid(node->pstate->tbmiterator)) > + tbm_free_shared_area(dsa, node->pstate->tbmiterator); > + > + if

Re: [HACKERS] Parallel bitmap heap scan

2017-03-07 Thread Robert Haas
On Mon, Mar 6, 2017 at 12:35 AM, Dilip Kumar wrote: > On Thu, Mar 2, 2017 at 6:52 PM, Robert Haas wrote: >> 0002 wasn't quite careful enough about the placement of #ifdef >> USE_PREFETCH, but otherwise looks OK. Committed after changing that >> and

Re: [HACKERS] Parallel bitmap heap scan

2017-03-05 Thread Dilip Kumar
On Thu, Mar 2, 2017 at 6:52 PM, Robert Haas wrote: > 0002 wasn't quite careful enough about the placement of #ifdef > USE_PREFETCH, but otherwise looks OK. Committed after changing that > and getting rid of the local variable prefetch_iterator, which seemed > to be adding

Re: [HACKERS] Parallel bitmap heap scan

2017-03-02 Thread Robert Haas
On Tue, Feb 28, 2017 at 9:18 PM, Dilip Kumar wrote: > 0001- same as previous with some changes for freeing the shared memory stuff. > 0002- nodeBitmapHeapScan refactoring, this applies independently > 0003- actual parallel bitmap stuff applies on top of 0001 and 0002 0002

Re: [HACKERS] Parallel bitmap heap scan

2017-02-28 Thread Dilip Kumar
On Sun, Feb 26, 2017 at 9:26 PM, Robert Haas wrote: > I'm not entirely sure about the calling convention for > tbm_free_shared_area() but the rest seems OK. Changed > >> 2. Now tbm_free is not freeing any of the shared members which can be >> accessed by other worker so

Re: [HACKERS] Parallel bitmap heap scan

2017-02-26 Thread Robert Haas
On Mon, Feb 27, 2017 at 10:30 AM, Dilip Kumar wrote: > On Sun, Feb 26, 2017 at 9:26 PM, Robert Haas wrote: >> tbm_free_shared_area because the two iterators share one copy of the >> underlying iteration arrays, and the TBM code isn't smart enough to

Re: [HACKERS] Parallel bitmap heap scan

2017-02-26 Thread Dilip Kumar
On Sun, Feb 26, 2017 at 9:26 PM, Robert Haas wrote: Thanks for the review, I will work on these. There are some comments I need suggestions. > tbm_free_shared_area because the two iterators share one copy of the > underlying iteration arrays, and the TBM code isn't smart

Re: [HACKERS] Parallel bitmap heap scan

2017-02-26 Thread Robert Haas
On Tue, Feb 21, 2017 at 10:20 AM, Dilip Kumar wrote: > 0001: > 1. I have got a new interface, "tbm_free_shared_area(dsa_area *dsa, > dsa_pointer dp)" which will be responsible for freeing all the shared > members (pagetable, ptpage and ptchunk). Actually, we can not do

Re: [HACKERS] Parallel bitmap heap scan

2017-02-20 Thread Dilip Kumar
On Mon, Feb 20, 2017 at 11:18 PM, Robert Haas wrote: > On Mon, Feb 20, 2017 at 5:55 AM, Thomas Munro > wrote: >> On Sun, Feb 19, 2017 at 9:59 PM, Dilip Kumar wrote: >>> So basically, what I want to propose is that Only

Re: [HACKERS] Parallel bitmap heap scan

2017-02-20 Thread Tom Lane
Robert Haas writes: > On Mon, Feb 20, 2017 at 6:19 AM, Tom Lane wrote: >> The thing that you really have to worry about for this kind of proposal >> is "what if the query errors out and we never get to ExecEndNode"? >> It's particularly nasty if you're

Re: [HACKERS] Parallel bitmap heap scan

2017-02-20 Thread Robert Haas
On Mon, Feb 20, 2017 at 6:19 AM, Tom Lane wrote: > Thomas Munro writes: >> One practical problem that came up was the need for executor nodes to >> get a chance to do that kind of cleanup before the DSM segment is >> detached. In my patch

Re: [HACKERS] Parallel bitmap heap scan

2017-02-20 Thread Robert Haas
On Mon, Feb 20, 2017 at 5:55 AM, Thomas Munro wrote: > On Sun, Feb 19, 2017 at 9:59 PM, Dilip Kumar wrote: >> So basically, what I want to propose is that Only during >> ExecReScanBitmapHeapScan we can free all the DSA pointers because at >>

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Dilip Kumar
On Sun, Feb 19, 2017 at 10:34 PM, Robert Haas wrote: > Yeah, but it looks like ExecReScanGather gets rid of the workers, but > reuses the existing DSM. I'm not quite sure what happens to the DSA. > It looks like it probably just hangs around from the previous > iteration,

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Tom Lane
Thomas Munro writes: > One practical problem that came up was the need for executor nodes to > get a chance to do that kind of cleanup before the DSM segment is > detached. In my patch series I introduced a new node API > ExecNodeDetach to allow for that. Andres

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Thomas Munro
On Sun, Feb 19, 2017 at 10:34 PM, Robert Haas wrote: > On Sun, Feb 19, 2017 at 9:59 PM, Dilip Kumar wrote: >> I can imagine it can get executed over and over if plan is something like >> below. >> >> NestLoopJoin >> -> SeqScan >> -> Gather

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Robert Haas
On Sun, Feb 19, 2017 at 9:59 PM, Dilip Kumar wrote: > I can imagine it can get executed over and over if plan is something like > below. > > NestLoopJoin > -> SeqScan > -> Gather > -> Parallel Bitmap Heap Scan > > But in such case every time the Inner node

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Dilip Kumar
On Sun, Feb 19, 2017 at 7:44 PM, Robert Haas wrote: > It's probably OK if tbm_free() doesn't free the memory allocated from > DSA, and we just let cleanup at end of query do it. However, that > could cause some trouble if the Parallel Bitmap Heap Scan gets > executed over

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Robert Haas
On Sun, Feb 19, 2017 at 7:18 PM, Dilip Kumar wrote: > I have observed one problem with 0002 and I though of sharing that > before fixing the same because we might have encountered the same > problem in some other patches i.e parallel shared hash and there might > be already

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Amit Kapila
On Sun, Feb 19, 2017 at 7:18 PM, Dilip Kumar wrote: > On Sat, Feb 18, 2017 at 10:45 PM, Dilip Kumar wrote: >> in 0002: >> - Improved comments. >> - Code refactoring in BitmapHeapNext. >> - Removed local tbm creation in BitmapHeapNext : as per new

Re: [HACKERS] Parallel bitmap heap scan

2017-02-19 Thread Dilip Kumar
On Sat, Feb 18, 2017 at 10:45 PM, Dilip Kumar wrote: > in 0002: > - Improved comments. > - Code refactoring in BitmapHeapNext. > - Removed local tbm creation in BitmapHeapNext : as per new tidbitmap > it's of no use. I have observed one problem with 0002 and I though of

Re: [HACKERS] Parallel bitmap heap scan

2017-02-18 Thread Dilip Kumar
On Fri, Feb 17, 2017 at 2:01 AM, Robert Haas wrote: > + * in order to share relptrs of the chunk and the pages arrays and other > + * TBM related information with other workers. > > No relptrs any more. Fixed > > +dsa_pointer dsapagetable;/* dsa_pointer to the

Re: [HACKERS] Parallel bitmap heap scan

2017-02-16 Thread Robert Haas
On Thu, Feb 16, 2017 at 10:54 AM, Dilip Kumar wrote: > interface_dsa_allocate0.patch : Provides new interface dsa_allocate0, > which is used by 0001 Committed. I moved the function prototype for dsa_allocate0() next to the existing prototype for dsa_allocate(). Please

Re: [HACKERS] Parallel bitmap heap scan

2017-02-16 Thread Dilip Kumar
On Tue, Feb 14, 2017 at 10:18 PM, Robert Haas wrote: > What is the point of having a TBMSharedIterator contain a TIDBitmap > pointer? All the values in that TIDBitmap are populated from the > TBMSharedInfo, but it seems to me that the fields that are copied over >

Re: [HACKERS] Parallel bitmap heap scan

2017-02-15 Thread Robert Haas
On Wed, Feb 15, 2017 at 7:17 AM, Dilip Kumar wrote: > This is easily doable and it will look much cleaner. While doing this > I am facing one problem related to > relptr_store and relptr_access. The problem is that when I call > "relptr_store (base, rp, val)" with both

Re: [HACKERS] Parallel bitmap heap scan

2017-02-15 Thread Dilip Kumar
On Tue, Feb 14, 2017 at 10:18 PM, Robert Haas wrote: > Thanks, the external interface to this looks much cleaner now. > Scrutinizing the internals: Thanks for the comments, I am working on these. I have few doubts for some of the comments. > I suggest removing both

Re: [HACKERS] Parallel bitmap heap scan

2017-02-14 Thread Robert Haas
On Tue, Feb 14, 2017 at 12:18 AM, Dilip Kumar wrote: > Fixed. Thanks, the external interface to this looks much cleaner now. Scrutinizing the internals: What is the point of having a TBMSharedIterator contain a TIDBitmap pointer? All the values in that TIDBitmap are

Re: [HACKERS] Parallel bitmap heap scan

2017-02-13 Thread Dilip Kumar
On Tue, Feb 14, 2017 at 6:51 AM, Haribabu Kommi wrote: > +#if SIZEOF_DSA_POINTER == 4 > +typedef uint32 dsa_pointer; > +#else > +typedef uint64 dsa_pointer; > +#endif > > I feel the declaration of the above typdef can be moved into the > section above if we going with

Re: [HACKERS] Parallel bitmap heap scan

2017-02-13 Thread Robert Haas
On Mon, Feb 13, 2017 at 8:48 AM, Dilip Kumar wrote: > On Mon, Feb 13, 2017 at 6:24 PM, Robert Haas wrote: >> I don't think it's acceptable (or necessary) to move the DSA >> definitions into postgres.h. Why do you think you need to do that, >> vs.

Re: [HACKERS] Parallel bitmap heap scan

2017-02-13 Thread Haribabu Kommi
On Tue, Feb 14, 2017 at 12:48 AM, Dilip Kumar wrote: > On Mon, Feb 13, 2017 at 6:24 PM, Robert Haas > wrote: > > I don't think it's acceptable (or necessary) to move the DSA > > definitions into postgres.h. Why do you think you need to do that, > >

Re: [HACKERS] Parallel bitmap heap scan

2017-02-13 Thread Dilip Kumar
On Mon, Feb 13, 2017 at 6:24 PM, Robert Haas wrote: > I don't think it's acceptable (or necessary) to move the DSA > definitions into postgres.h. Why do you think you need to do that, > vs. just including dsa.h in a few more places? I need to access dsa_pointer in

Re: [HACKERS] Parallel bitmap heap scan

2017-02-13 Thread Robert Haas
On Sat, Feb 11, 2017 at 1:41 AM, Dilip Kumar wrote: > I tried my best, please check the latest patch (0001). I don't think it's acceptable (or necessary) to move the DSA definitions into postgres.h. Why do you think you need to do that, vs. just including dsa.h in a few

Re: [HACKERS] Parallel bitmap heap scan

2017-02-10 Thread Dilip Kumar
On Wed, Feb 8, 2017 at 8:07 AM, Robert Haas wrote: Thanks for the detailed review and your valuable feedback. > I think it would be useful to break the remaining patch into two > parts, one that enhances the tidbitmap.c API and another that uses > that to implement

Re: [HACKERS] Parallel bitmap heap scan

2017-02-09 Thread Robert Haas
On Wed, Feb 8, 2017 at 10:59 AM, Robert Haas wrote: > Looks good to me. If nobody has further ideas here, I'll push this > and your previous patch tomorrow. Done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Robert Haas
On Wed, Feb 8, 2017 at 8:58 AM, Dilip Kumar wrote: > On Wed, Feb 8, 2017 at 7:01 PM, Robert Haas wrote: >> You can store whatever you want in SH_TYPE's private_data member. >> SH_ALLOCATE and SH_FREE both get a pointer to the SH_TYPE, so they >> have

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Dilip Kumar
On Wed, Feb 8, 2017 at 7:01 PM, Robert Haas wrote: > You can store whatever you want in SH_TYPE's private_data member. > SH_ALLOCATE and SH_FREE both get a pointer to the SH_TYPE, so they > have access to that. Hmm, but there's no way to get that set in > SH_CREATE before

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Robert Haas
On Wed, Feb 8, 2017 at 1:59 AM, Dilip Kumar wrote: > IIUC, tbm_prepare_shared_iterate will be called only by the leader, > for tbmiterator as well as for the prefetch_iterator. And, > tbm_attach_shared_iterate will be called by each backend and for both > the iterators.

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Robert Haas
On Wed, Feb 8, 2017 at 4:20 AM, Dilip Kumar wrote: > The new SH_CREATE(MemoryContext ctx, uint32 nelements) don't have any > option to supply arguments to it. Our callback functions need access > to TBM. > > Is it expected that if the user of SH_CREATE who doesn't want to

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Robert Haas
On Wed, Feb 8, 2017 at 5:21 AM, Dilip Kumar wrote: > On Wed, Feb 8, 2017 at 3:44 AM, Robert Haas wrote: +#ifndef SH_USE_NONDEFAULT_ALLOCATOR + >>> >>> That should probably be documented in the file header. >> >> Right. OK, did that and a

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Dilip Kumar
On Wed, Feb 8, 2017 at 3:44 AM, Robert Haas wrote: >>> +#ifndef SH_USE_NONDEFAULT_ALLOCATOR >>> + >> >> That should probably be documented in the file header. > > Right. OK, did that and a few other cleanups, and committed. I think we need to have prototype for the

Re: [HACKERS] Parallel bitmap heap scan

2017-02-08 Thread Dilip Kumar
On Wed, Feb 8, 2017 at 3:44 AM, Robert Haas wrote: >>> +#ifndef SH_USE_NONDEFAULT_ALLOCATOR >>> + >> >> That should probably be documented in the file header. > > Right. OK, did that and a few other cleanups, and committed. The new SH_CREATE(MemoryContext ctx, uint32

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Dilip Kumar
On Wed, Feb 8, 2017 at 8:07 AM, Robert Haas wrote: .Thanks for your input, I have few queries about these comments. > 2. Add a new function dsa_pointer tbm_prepare_shared_iterate(TIDBitmap > *tbm) which allocates shared iteration arrays using the DSA passed to >

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Robert Haas
On Sun, Feb 5, 2017 at 9:48 AM, Dilip Kumar wrote: > Here are the latest version of the patch, which address all the > comments given by Robert. I think it would be useful to break the remaining patch into two parts, one that enhances the tidbitmap.c API and another that

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Robert Haas
On Tue, Feb 7, 2017 at 4:45 PM, Andres Freund wrote: > On 2017-02-07 16:36:55 -0500, Robert Haas wrote: >> On Tue, Feb 7, 2017 at 4:15 PM, Andres Freund wrote: >> > FWIW, I think it'd have been better to not add the new callbacks as >> > parameters to

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Andres Freund
On 2017-02-07 16:36:55 -0500, Robert Haas wrote: > On Tue, Feb 7, 2017 at 4:15 PM, Andres Freund wrote: > > FWIW, I think it'd have been better to not add the new callbacks as > > parameters to *_create(), but rather have them be "templatized" like the > > rest of simplehash.

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Robert Haas
On Tue, Feb 7, 2017 at 4:15 PM, Andres Freund wrote: > FWIW, I think it'd have been better to not add the new callbacks as > parameters to *_create(), but rather have them be "templatized" like the > rest of simplehash. That'd require that callback to check the context, > to

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Robert Haas
On Tue, Feb 7, 2017 at 4:13 PM, Jeff Janes wrote: > I'm getting compiler errors: > > In file included from execGrouping.c:47: > ../../../src/include/lib/simplehash.h:91: error: redefinition of typedef > 'simplehash_allocate' > ../../../src/include/lib/simplehash.h:91: note:

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Andres Freund
On 2017-02-07 13:13:43 -0800, Jeff Janes wrote: > On Tue, Feb 7, 2017 at 1:03 PM, Robert Haas wrote: > > > On Sun, Feb 5, 2017 at 9:48 AM, Dilip Kumar wrote: > > >> I think maybe we should rename the functions to element_allocate, > > >>

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Andres Freund
On 2017-02-07 16:03:43 -0500, Robert Haas wrote: > On Sun, Feb 5, 2017 at 9:48 AM, Dilip Kumar wrote: > >> I think maybe we should rename the functions to element_allocate, > >> element_free, and element_allocator_ctx, or something like that. The > >> current names aren't

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Jeff Janes
On Tue, Feb 7, 2017 at 1:03 PM, Robert Haas wrote: > On Sun, Feb 5, 2017 at 9:48 AM, Dilip Kumar wrote: > >> I think maybe we should rename the functions to element_allocate, > >> element_free, and element_allocator_ctx, or something like that. The

Re: [HACKERS] Parallel bitmap heap scan

2017-02-07 Thread Robert Haas
On Sun, Feb 5, 2017 at 9:48 AM, Dilip Kumar wrote: >> I think maybe we should rename the functions to element_allocate, >> element_free, and element_allocator_ctx, or something like that. The >> current names aren't capitalized consistently with other things in >> this

Re: [HACKERS] Parallel bitmap heap scan

2017-02-05 Thread Dilip Kumar
On Wed, Feb 1, 2017 at 11:09 PM, Robert Haas wrote: Here are the latest version of the patch, which address all the comments given by Robert. > https://www.postgresql.org/message-id/20161017201558.cr34stc6zvxy3...@alap3.anarazel.de > that you should try to share only the

Re: [HACKERS] Parallel bitmap heap scan

2017-02-01 Thread Dilip Kumar
On Wed, Feb 1, 2017 at 11:09 PM, Robert Haas wrote: Hi Robert, Thanks for the review. > When Andres wrote in > https://www.postgresql.org/message-id/20161017201558.cr34stc6zvxy3...@alap3.anarazel.de > that you should try to share only the iteration arrays, I believe that

Re: [HACKERS] Parallel bitmap heap scan

2017-02-01 Thread Robert Haas
On Tue, Jan 31, 2017 at 6:05 AM, Dilip Kumar wrote: >> 0002-hash-support-alloc-free-v14.patch: >> >> >> + if (tb->alloc) >> + { >> >> The memory for tb->alloc is allocated always, is the if check still >> required? > > In parallel case, only first worker will call

Re: [HACKERS] Parallel bitmap heap scan

2017-01-31 Thread Dilip Kumar
On Tue, Jan 31, 2017 at 7:47 AM, Haribabu Kommi wrote: > Thanks for the update. I have some comments > Thanks for the review. > > 0002-hash-support-alloc-free-v14.patch: > > > + if (tb->alloc) > + { > > The memory for tb->alloc is allocated always, is the if check still

Re: [HACKERS] Parallel bitmap heap scan

2017-01-30 Thread Michael Paquier
On Tue, Jan 31, 2017 at 11:17 AM, Haribabu Kommi wrote: > On Fri, Jan 27, 2017 at 5:32 PM, Dilip Kumar wrote: >> On Tue, Jan 24, 2017 at 10:18 AM, Dilip Kumar >> wrote: >> > I have changed as per the comments. 0002 and 0003

Re: [HACKERS] Parallel bitmap heap scan

2017-01-30 Thread Haribabu Kommi
On Fri, Jan 27, 2017 at 5:32 PM, Dilip Kumar wrote: > On Tue, Jan 24, 2017 at 10:18 AM, Dilip Kumar > wrote: > > I have changed as per the comments. 0002 and 0003 are changed, 0001 is > > still the same. > > 2 days back my colleague Rafia, reported

Re: [HACKERS] Parallel bitmap heap scan

2017-01-27 Thread Robert Haas
On Fri, Jan 27, 2017 at 1:32 AM, Dilip Kumar wrote: > There is just one line change in 0003 compared to older version, all > other patches are the same. I spent some time looking at 0001 (and how those changes are used in 0003) and I thought it looked good, so I committed

Re: [HACKERS] Parallel bitmap heap scan

2017-01-26 Thread Dilip Kumar
On Tue, Jan 24, 2017 at 10:18 AM, Dilip Kumar wrote: > I have changed as per the comments. 0002 and 0003 are changed, 0001 is > still the same. 2 days back my colleague Rafia, reported one issue (offlist) that parallel bitmap node is not scaling as good as other nodes e.g

Re: [HACKERS] Parallel bitmap heap scan

2017-01-23 Thread Dilip Kumar
On Mon, Jan 23, 2017 at 1:52 PM, Haribabu Kommi wrote: > I reviewed 0002-hash-support-alloc-free-v12.patch, some minor comments. > > - SH_TYPE*tb; > - uint64 size; > + SH_TYPE *tb; > + uint64 size; > > The above change may not be required. > > + if (tb->alloc) >

Re: [HACKERS] Parallel bitmap heap scan

2017-01-23 Thread Haribabu Kommi
On Mon, Jan 23, 2017 at 3:42 PM, Dilip Kumar wrote: > On Thu, Jan 19, 2017 at 12:26 AM, Robert Haas > wrote: > >> Patch 0001 and 0003 required to rebase on the latest head. 0002 is > >> still the same. > > > > I've committed the first half of 0001.

Re: [HACKERS] Parallel bitmap heap scan

2017-01-22 Thread Dilip Kumar
On Thu, Jan 19, 2017 at 12:26 AM, Robert Haas wrote: >> Patch 0001 and 0003 required to rebase on the latest head. 0002 is >> still the same. > > I've committed the first half of 0001. Thanks. 0001 and 0003 required rebasing after this commit. -- Regards, Dilip Kumar

Re: [HACKERS] Parallel bitmap heap scan

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 12:14 AM, Dilip Kumar wrote: > On Fri, Jan 13, 2017 at 6:36 PM, Dilip Kumar wrote: >> >> Please verify with the new patch. > > Patch 0001 and 0003 required to rebase on the latest head. 0002 is > still the same. I've

Re: [HACKERS] Parallel bitmap heap scan

2017-01-17 Thread Dilip Kumar
On Fri, Jan 13, 2017 at 6:36 PM, Dilip Kumar wrote: > > Please verify with the new patch. Patch 0001 and 0003 required to rebase on the latest head. 0002 is still the same. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Parallel bitmap heap scan

2017-01-13 Thread Dilip Kumar
On Thu, Jan 12, 2017 at 5:47 PM, Dilip Kumar wrote: >> Hello Dilip, >> I was trying to test the performance of all the parallel query related >> patches on TPC-H queries, I found that when parallel hash [1 ]and your >> patch are applied then Q10 and Q14 were hanged,

Re: [HACKERS] Parallel bitmap heap scan

2017-01-12 Thread Dilip Kumar
On Thu, Jan 12, 2017 at 5:22 PM, Rafia Sabih wrote: > Hello Dilip, > I was trying to test the performance of all the parallel query related > patches on TPC-H queries, I found that when parallel hash [1 ]and your > patch are applied then Q10 and Q14 were hanged,

Re: [HACKERS] Parallel bitmap heap scan

2017-01-12 Thread Rafia Sabih
On Wed, Jan 11, 2017 at 5:33 PM, tushar wrote: > > On 01/10/2017 05:16 PM, Dilip Kumar wrote: >> >> Please try attached patch and confirm from your >> side. > > Thanks,issue seems to be fixed now. > > > -- > regards,tushar > EnterpriseDB

Re: [HACKERS] Parallel bitmap heap scan

2017-01-11 Thread tushar
On 01/10/2017 05:16 PM, Dilip Kumar wrote: Please try attached patch and confirm from your side. Thanks,issue seems to be fixed now. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Parallel bitmap heap scan

2017-01-10 Thread Dilip Kumar
On Tue, Jan 10, 2017 at 2:19 PM, tushar wrote: > We found a regression , earlier the testcase was working fine (against the > older patches of Parallel bitmap heap scan) but now getting a server crash > against v8 patches. > > Testcase - (one of the table of TPC-H

Re: [HACKERS] Parallel bitmap heap scan

2017-01-10 Thread tushar
On 01/10/2017 11:29 AM, tushar wrote: On 01/09/2017 07:22 PM, Dilip Kumar wrote: Thanks, Tushar. I have fixed it. The defect was in 0002. I have also observed another issue related to code refactoring, Actually, there was some code present in 0001 which supposed to be in 0003. Thanks, I have

Re: [HACKERS] Parallel bitmap heap scan

2017-01-09 Thread tushar
On 01/09/2017 07:22 PM, Dilip Kumar wrote: Thanks, Tushar. I have fixed it. The defect was in 0002. I have also observed another issue related to code refactoring, Actually, there was some code present in 0001 which supposed to be in 0003. Thanks, I have checked at my end and it is fixed now.

Re: [HACKERS] Parallel bitmap heap scan

2017-01-09 Thread Dilip Kumar
On Mon, Jan 9, 2017 at 5:01 PM, tushar wrote: > Thanks Dilip. issue is reproducible if we uses '--enable-cassert' switch > in the configure. We are able to reproduce it only with --enable-cassert' . Thanks, Tushar. I have fixed it. The defect was in 0002. I have

Re: [HACKERS] Parallel bitmap heap scan

2017-01-09 Thread tushar
On 01/09/2017 04:36 PM, Dilip Kumar wrote: I have taken the latest code, applied all 3 patches and compiled. Initdb is working fine for me. Can you please verify, do you have any extra patch along with my patch? Did you properly clean the code? Thanks Dilip. issue is reproducible if we uses

Re: [HACKERS] Parallel bitmap heap scan

2017-01-09 Thread Dilip Kumar
On Mon, Jan 9, 2017 at 3:07 PM, tushar wrote: > creating directory data ... ok > creating subdirectories ... ok > selecting default max_connections ... 100 > selecting default shared_buffers ... 128MB > selecting dynamic shared memory implementation ... posix >

Re: [HACKERS] Parallel bitmap heap scan

2017-01-09 Thread tushar
On 01/09/2017 01:05 PM, Dilip Kumar wrote: This patch can be used by 0003-parallel-bitmap-heap-scan-v7.patch attached in the mail and also parallel-index-scan[2] can be rebased on this, if this get committed, After applying your patches against the fresh sources of PG v10 , not able to perform

Re: [HACKERS] Parallel bitmap heap scan

2017-01-08 Thread Dilip Kumar
On Fri, Jan 6, 2017 at 10:47 AM, Amit Khandekar wrote: > This looks good now. Thanks.. > > > Further points below Thanks for the review. > = nodeBItmapHeapscan.c == > > > In BitmapHeapNext(), the following code is relevant only for tbm > returned from

Re: [HACKERS] Parallel bitmap heap scan

2017-01-05 Thread Amit Khandekar
Sorry for the delay in my next response. I still haven't exhaustively gone through all changes, but meanwhile, below are some more points. On 26 November 2016 at 18:18, Dilip Kumar wrote: > On Tue, Nov 22, 2016 at 9:05 AM, Dilip Kumar wrote: >> On

  1   2   >