On Thu, Jan 4, 2018 at 11:29 PM, Robert Haas wrote:
> On Wed, Dec 20, 2017 at 3:38 AM, Amit Kapila wrote:
>> On Tue, Dec 19, 2017 at 11:37 PM, Robert Haas wrote:
>>> On Wed, Dec 13, 2017 at 9:18 PM, Amit Kapila
>>> wrote:
Thanks. I think now we can proceed with
fix_accum_instr_paral
On Wed, Dec 20, 2017 at 3:38 AM, Amit Kapila wrote:
> On Tue, Dec 19, 2017 at 11:37 PM, Robert Haas wrote:
>> On Wed, Dec 13, 2017 at 9:18 PM, Amit Kapila wrote:
>>> Thanks. I think now we can proceed with
>>> fix_accum_instr_parallel_workers_v8.patch posted above which will fix
>>> the origina
On Tue, Dec 19, 2017 at 11:37 PM, Robert Haas wrote:
> On Wed, Dec 13, 2017 at 9:18 PM, Amit Kapila wrote:
>> Thanks. I think now we can proceed with
>> fix_accum_instr_parallel_workers_v8.patch posted above which will fix
>> the original issue and the problem we have found in sort and hash
>> n
On Tue, Dec 19, 2017 at 11:37 PM, Robert Haas wrote:
> On Wed, Dec 13, 2017 at 9:18 PM, Amit Kapila wrote:
>> Thanks. I think now we can proceed with
>> fix_accum_instr_parallel_workers_v8.patch posted above which will fix
>> the original issue and the problem we have found in sort and hash
>> n
On Tue, Dec 19, 2017 at 1:29 PM, Robert Haas wrote:
> Well, not really, because the context surrounding the lines you've
> added seems to refer to SQL that I can't find in join.sql or anywhere
> else in the tree. So my suspicion is that this patch is based on your
> parallel hash patch set rather
On Tue, Dec 5, 2017 at 4:23 PM, Thomas Munro
wrote:
> The hash version of this code is now committed as 5bcf389e. Here is a
> patch for discussion that adds some extra tests to join.sql to
> exercise rescans of a hash join under a Gather node. It fails on
> head, because it loses track of the in
On Wed, Dec 13, 2017 at 9:18 PM, Amit Kapila wrote:
> Thanks. I think now we can proceed with
> fix_accum_instr_parallel_workers_v8.patch posted above which will fix
> the original issue and the problem we have found in sort and hash
> nodes.
Committed and back-patched to v10. While I was doing
On Thu, Dec 14, 2017 at 2:32 AM, Robert Haas wrote:
> On Tue, Dec 12, 2017 at 9:37 PM, Amit Kapila wrote:
>>> Uh, should I just revert that commit entirely first, and then we can
>>> commit the new fix afterward?
>>
>> Yes. I have already extracted the test case of that commit to the new
>> patch
On Tue, Dec 12, 2017 at 9:37 PM, Amit Kapila wrote:
>> Uh, should I just revert that commit entirely first, and then we can
>> commit the new fix afterward?
>
> Yes. I have already extracted the test case of that commit to the new
> patch which is what we need from that commit.
OK, done.
--
Rob
On Wed, Dec 13, 2017 at 3:26 AM, Robert Haas wrote:
> On Sat, Dec 9, 2017 at 5:30 AM, Amit Kapila wrote:
>> On Sat, Dec 9, 2017 at 1:24 AM, Robert Haas wrote:
>>> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
> Okay, I have adjusted the patch accordingly. I have also added a
> reg
On Sat, Dec 9, 2017 at 5:30 AM, Amit Kapila wrote:
> On Sat, Dec 9, 2017 at 1:24 AM, Robert Haas wrote:
>> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
Okay, I have adjusted the patch accordingly. I have also added a
regression test which should produce the same result across di
On Sat, Dec 9, 2017 at 1:24 AM, Robert Haas wrote:
> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
>>> Okay, I have adjusted the patch accordingly. I have also added a
>>> regression test which should produce the same result across different
>>> runs, see if that looks okay to you, then it
On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
>> Okay, I have adjusted the patch accordingly. I have also added a
>> regression test which should produce the same result across different
>> runs, see if that looks okay to you, then it is better to add such a
>> test as well.
>
> The regressi
On Fri, Dec 8, 2017 at 2:22 PM, Thomas Munro
wrote:
> On Sat, Dec 9, 2017 at 6:42 AM, Robert Haas wrote:
>> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
>>> The regression test added by patch needs cleanup at the end which I
>>> have added in the attached patch.
>>
>> OK, so you've got a t
On Sat, Dec 9, 2017 at 6:42 AM, Robert Haas wrote:
> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
>> The regression test added by patch needs cleanup at the end which I
>> have added in the attached patch.
>
> OK, so you've got a test case now, but Thomas independently submitted
> a test ca
On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila wrote:
> The regression test added by patch needs cleanup at the end which I
> have added in the attached patch.
OK, so you've got a test case now, but Thomas independently submitted
a test case patch. Which one is more awesome? :-)
--
Robert Haas
E
On Fri, Dec 8, 2017 at 9:33 AM, Amit Kapila wrote:
> On Fri, Dec 8, 2017 at 12:06 AM, Robert Haas wrote:
>> On Wed, Dec 6, 2017 at 1:05 AM, Amit Kapila wrote:
>>> Right and seeing that I have prepared the patch (posted above [1])
>>> which fixes it such that it will resemble the non-parallel cas
On Fri, Dec 8, 2017 at 12:06 AM, Robert Haas wrote:
> On Wed, Dec 6, 2017 at 1:05 AM, Amit Kapila wrote:
>> Right and seeing that I have prepared the patch (posted above [1])
>> which fixes it such that it will resemble the non-parallel case.
>> Ideally, it would have obviated the need for my pre
On Fri, Dec 8, 2017 at 7:36 AM, Robert Haas wrote:
> On Wed, Dec 6, 2017 at 1:05 AM, Amit Kapila wrote:
>> Right and seeing that I have prepared the patch (posted above [1])
>> which fixes it such that it will resemble the non-parallel case.
>
> Long story short, I like the patch.
LGTM. There m
On Wed, Dec 6, 2017 at 1:05 AM, Amit Kapila wrote:
> Right and seeing that I have prepared the patch (posted above [1])
> which fixes it such that it will resemble the non-parallel case.
> Ideally, it would have obviated the need for my previous patch which
> got committed as 778e78ae. However, n
On Wed, Dec 6, 2017 at 1:10 AM, Robert Haas wrote:
> On Tue, Dec 5, 2017 at 12:39 AM, Amit Kapila wrote:
>> Yeah, that sounds better, so modified the patch accordingly.
>
> I committed this to master and REL_10_STABLE, but it conflicts all
> over the place on 9.6.
>
I will try to prepare the pat
On Wed, Dec 6, 2017 at 12:18 AM, Robert Haas wrote:
> On Tue, Dec 5, 2017 at 2:49 AM, Thomas Munro
> wrote:
>> As for how to aggregate the information, isn't it reasonable to show
>> data from the last loop on the basis that it's representative?
>> Summing wouldn't make too much sense, because yo
On Wed, Dec 6, 2017 at 12:02 AM, Amit Kapila wrote:
> On Tue, Dec 5, 2017 at 1:29 PM, Thomas Munro
> wrote:
>> Or would it be an option to change the time
>> ExecXXXRetrieveInstrumentation() is called so it is run only once?
>
> To me, that doesn't sound like a bad option. I think if do so, then
On Tue, Dec 5, 2017 at 12:39 AM, Amit Kapila wrote:
> Yeah, that sounds better, so modified the patch accordingly.
I committed this to master and REL_10_STABLE, but it conflicts all
over the place on 9.6.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Dec 5, 2017 at 2:49 AM, Thomas Munro
wrote:
> As for how to aggregate the information, isn't it reasonable to show
> data from the last loop on the basis that it's representative?
> Summing wouldn't make too much sense, because you didn't use that much
> memory all at once.
Sorts can be r
On Tue, Dec 5, 2017 at 1:29 PM, Thomas Munro
wrote:
> On Tue, Dec 5, 2017 at 8:49 PM, Thomas Munro
> wrote:
>> On Tue, Dec 5, 2017 at 6:39 PM, Amit Kapila wrote:
>>> I have one another observation in the somewhat related area. From the
>>> code, it looks like we might have some problem with dis
On Tue, Dec 5, 2017 at 8:49 PM, Thomas Munro
wrote:
> On Tue, Dec 5, 2017 at 6:39 PM, Amit Kapila wrote:
>> I have one another observation in the somewhat related area. From the
>> code, it looks like we might have some problem with displaying sort
>> info for workers for rescans. I think the p
On Tue, Dec 5, 2017 at 6:39 PM, Amit Kapila wrote:
> I have one another observation in the somewhat related area. From the
> code, it looks like we might have some problem with displaying sort
> info for workers for rescans. I think the problem with the sortinfo
> is that it initializes shared i
On Mon, Dec 4, 2017 at 11:17 PM, Robert Haas wrote:
> On Sat, Dec 2, 2017 at 8:04 AM, Amit Kapila wrote:
>> Attached patch contains regression test as well. Note that I have
>> carefully disabled all variable stats by using (analyze, timing off,
>> summary off, costs off) and then selected paral
On Sat, Dec 2, 2017 at 8:04 AM, Amit Kapila wrote:
> Attached patch contains regression test as well. Note that I have
> carefully disabled all variable stats by using (analyze, timing off,
> summary off, costs off) and then selected parallel sequential scan on
> the right of join so that we have
On Wed, Nov 29, 2017 at 2:04 PM, Amit Kapila wrote:
> On Tue, Nov 28, 2017 at 9:42 PM, Robert Haas wrote:
>> On Tue, Nov 28, 2017 at 2:23 AM, Amit Kapila wrote:
>>> That is wrong and I think you have hit a bug. It should be 2974 * 5 =
>>> 14870 as you have seen in other cases. The problem is t
On Tue, Nov 28, 2017 at 9:37 PM, Robert Haas wrote:
> On Mon, Nov 27, 2017 at 6:54 AM, Amit Kapila wrote:
>>> Anything "below" "Gather"?
>>>
>> I think it is "actual_time * 1" for anything below Gather.
>
> The actual time amounts below Gather show total elapsed time divided
> by loop count, just
On Tue, Nov 28, 2017 at 9:42 PM, Robert Haas wrote:
> On Tue, Nov 28, 2017 at 2:23 AM, Amit Kapila wrote:
>> That is wrong and I think you have hit a bug. It should be 2974 * 5 =
>> 14870 as you have seen in other cases. The problem is that during
>> rescan, we generally reinitialize the requir
On Tue, Nov 28, 2017 at 2:23 AM, Amit Kapila wrote:
> That is wrong and I think you have hit a bug. It should be 2974 * 5 =
> 14870 as you have seen in other cases. The problem is that during
> rescan, we generally reinitialize the required state, but we forgot to
> reinitialize the instrumentat
On Mon, Nov 27, 2017 at 6:54 AM, Amit Kapila wrote:
>> Anything "below" "Gather"?
>>
> I think it is "actual_time * 1" for anything below Gather.
The actual time amounts below Gather show total elapsed time divided
by loop count, just as they do anywhere else in the plan. The loop
count tracks t
On Tue, Nov 28, 2017 at 12:53:41PM +0530, Amit Kapila wrote:
> Is it possible for you to test the attached patch and see if you are
> still seeing any unexpected values?
well, not really. the explains i had were posted by people on
explain.depesz.com, so i don't have original queries nor their dat
On Mon, Nov 27, 2017 at 11:26 PM, hubert depesz lubaczewski
wrote:
> On Mon, Nov 27, 2017 at 05:24:49PM +0530, Amit Kapila wrote:
>> I think it is "actual_time * 1" for anything below Gather.
>
> Well, I think I found another problem.
>
> Please take a look at:
> https://explain.depesz.com/s/Bs8c
On Mon, Nov 27, 2017 at 05:24:49PM +0530, Amit Kapila wrote:
> I think it is "actual_time * 1" for anything below Gather.
Well, I think I found another problem.
Please take a look at:
https://explain.depesz.com/s/Bs8c
Node 15 is Gather with loops = 2974 (because it was ran by nested loop).
Ther
On Mon, Nov 27, 2017 at 2:45 PM, hubert depesz lubaczewski
wrote:
> On Sat, Nov 25, 2017 at 07:08:03AM +0530, Amit Kapila wrote:
>> > For example, check step 13 in https://explain.depesz.com/s/gNBd
>> >
>> > It shows time of 3ms, but loops of 1873, so the actual time is ~ 5600ms.
>> >
>> > But wit
On Sat, Nov 25, 2017 at 07:08:03AM +0530, Amit Kapila wrote:
> > For example, check step 13 in https://explain.depesz.com/s/gNBd
> >
> > It shows time of 3ms, but loops of 1873, so the actual time is ~ 5600ms.
> >
> > But with parallel execution it seems to be no longer the case.
> >
> > For exampl
On Sat, Nov 25, 2017 at 7:08 AM, Amit Kapila wrote:
>>
>> It looks that the actual time is really actual time, and loops is
>> "worker nodes + 1".
>>
>> Is that really the case?
>>
>
> I think so.
>
To add to what Amit has explained. The + 1 is for the leader (the
backend running the whole query)
On Fri, Nov 24, 2017 at 4:51 PM, hubert depesz lubaczewski
wrote:
> Hi,
>
> up to parallel executions, when we had node in explain analyze showing
> "loops=x" with x more than 1, it meant that the "actual time" had to be
> multiplied by loops to get real time spent in a node.
>
> For example, chec
42 matches
Mail list logo
