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.
--
Robert Haas
EnterpriseDB: h
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
Descriptio
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 from SH_CREATE, which isn't
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 = 1GB,
>> shared_buffers = 10 GB,
>> Effective_cache_size
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 = seq+page_cost =0.1
>
> The error
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:
>>> --prefix=/Applications/Xcode.app/Contents/Developer/usr
>>> --with-gxx-include-dir=/usr/include/c++/4.2.1
>>> Apple LLVM
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 (clang-700.1.81)
>> Target: x86_64-apple-darwin
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. I noticed that longfin
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 warning should depend
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. Sorry for the hassle; my compiler isn't as picky about this
> as I
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 about this
as I would like, and apparently Dil
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 subscription:
http://w
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 it.
>
> Committed with that fix and a bunch of
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
EnterpriseDB: http://www.
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
> + Waiting for the leader t
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
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 assume
prefetch_target is 3
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 to
>>> the Bitmap Heap Scan node for a long tim
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
>> other part of the plan tre
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 part of the plan tree.
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
>> the prefetch information under the mutex sh
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 to the cost of actually r
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, node->pstate->tbmiterator);
>> +
>> + if (DsaPointerI
(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 = dsa_get_address(dsa, istate->spag
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 (DsaPointerIsValid(node->pstate->prefetch_iterator))
> +
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 getting rid of the local variable prefetch_iter
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 rather than removing com
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 wasn't quite careful eno
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 tbm_free is safe to call f
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
>> avoid freeing them twice. You're going to h
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 enough to
> avoid freein
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 this
> in tbm_free itself
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 during
>>> ExecReScanBitmapHeapScan we can free all the DSA pointers because
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 talking about parallel queries where
>> mayb
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 series I introduced a new node API
>> ExecNodeDetach to a
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
>> that time we can be sure that all the workers have comp
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, which means that any all
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 objected that the need for
> that
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
>> -> Parallel Bitmap Heap Scan
>>
>> Bu
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 of the NLJ will be
> res
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 and over and keeps alloc
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 a way to handle that.
>
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 tidbitmap
>> it's of no use.
>
> I have observed o
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 sharing that
before fixing
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 element array */
> +dsa_
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 try to think about
things
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
> unchanged (like status and nchun
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 base and val as a same
> ad
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 "entry1" and "status" from
> T
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 populated from the
TBMShared
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 the current move into postgre
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. just including dsa.h in a few more places?
>
> I
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,
> > vs. just including dsa.h in a few more places?
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 tidbitmap.h, which is included
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 more places?
--
Robert
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 Parallel Bitmap Heap Scan.
I
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 pgsql-hackers mailing lis
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 access to that. Hmm, but there's no way to ge
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 SH_ALLOCATE is called.
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.
That's what I had in mind.
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 pass
> a "MemoryContext"
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 few other cleanups, and committed.
>
> I think w
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 default allocator outside of
#
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 nelements) don't have any
op
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
> tbm_create() and returns a dsa_po
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 uses
that to implement Pa
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 *_create(), but rather have them be "templati
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. That'd require that
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 know whether it shoul
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: previous declaration of
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,
> > >> element_free, and element_allocator_ctx, or something like
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 capitalized consistently
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
> >> current names aren't capitalized consiste
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 module, and putting the wor
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 iteration arrays, I beli
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
> he meant the results
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 SH_CREATE, other worker
> will on
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
> required?
In parallel c
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 are changed, 0001 is
>> > still the same.
>>
>> There is just one line c
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 one issue (offlist) that
> parallel bitmap node
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 0001.
--
Robert Haas
E
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
parallel sequence scan a
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)
> + {
> + tb->alloc->HashFree
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.
> Thanks. 0001 and 0003 required rebasing afte
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
EnterpriseDB: http://www.
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 committed the first half of 0001.
--
Robert Haas
Ente
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
0001-opt-parallelcost-refactoring-v11.patch
Descriptio
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, however, without any
>> one of
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, however, without any
> one of these
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 https://www.enterprisedb.com/
> The Enterprise PostgreSQL
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 (pgsql-hackers@postgre
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 )
>
> postgres=#explain analyze
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 ch
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.
-
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 also
observed another issue rel
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 '
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
> creating configuration files ... ok
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 i
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 MultiExecProcNode().
> if (!t
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 Fri, Nov 18, 2016 at 9:59 AM, Amit Khandekar
>
1 - 100 of 128 matches
Mail list logo
