Re: Explain buffers wrong counter with parallel plans

On Fri, Aug 3, 2018 at 6:56 PM, Robert Haas wrote: > On Fri, Aug 3, 2018 at 4:39 AM, Amit Kapila wrote: >> I have noticed that part of the comment atop ExecShutdownNode is now >> redundant. See attached edit_comments_shutdown_node_v1, let me know >> if you think otherwise. > > Oh, yes, that

Re: Explain buffers wrong counter with parallel plans

On Sun, Aug 5, 2018 at 5:34 PM, Amit Kapila wrote: > On Fri, Aug 3, 2018 at 2:09 PM, Amit Kapila wrote: >> On Thu, Aug 2, 2018 at 11:14 PM, Robert Haas wrote: >>> In the third one, I don't think >>> "See ExecLimit" is a good thing to put a comment like this, because >>> it's too hard to find

Re: Explain buffers wrong counter with parallel plans

On Fri, Aug 3, 2018 at 2:09 PM, Amit Kapila wrote: > On Thu, Aug 2, 2018 at 11:14 PM, Robert Haas wrote: >> On Thu, Aug 2, 2018 at 5:41 AM, Amit Kapila wrote: >>> I have created three patches (a) move InstrStartParallelQuery from its >>> original location so that we perform it just before

Re: Explain buffers wrong counter with parallel plans

On Fri, Aug 3, 2018 at 4:39 AM, Amit Kapila wrote: > I have noticed that part of the comment atop ExecShutdownNode is now > redundant. See attached edit_comments_shutdown_node_v1, let me know > if you think otherwise. Oh, yes, that should be adjusted. -- Robert Haas EnterpriseDB:

Re: Explain buffers wrong counter with parallel plans

On 08/03/2018 10:39 AM, Amit Kapila wrote: Thanks. I have pushed those two patches. Thanks!

Re: Explain buffers wrong counter with parallel plans

On Thu, Aug 2, 2018 at 11:14 PM, Robert Haas wrote: > On Thu, Aug 2, 2018 at 5:41 AM, Amit Kapila wrote: >> I have created three patches (a) move InstrStartParallelQuery from its >> original location so that we perform it just before ExecutorRun (b) >> patch to fix the gather stats by calling

Re: Explain buffers wrong counter with parallel plans

On Thu, Aug 2, 2018 at 5:41 AM, Amit Kapila wrote: > I have created three patches (a) move InstrStartParallelQuery from its > original location so that we perform it just before ExecutorRun (b) > patch to fix the gather stats by calling shutdown at appropriate place > and allow stats collection

Re: Explain buffers wrong counter with parallel plans

On Thu, Aug 2, 2018 at 10:26 AM, Amit Kapila wrote: > On Thu, Aug 2, 2018 at 8:38 AM, Andres Freund wrote: >> Hi, >> >> On 2018-08-02 08:21:58 +0530, Amit Kapila wrote: >>> I think something on the lines what Tom and you are suggesting can be >>> done with the help of EXEC_FLAG_BACKWARD, but I

Re: Explain buffers wrong counter with parallel plans

On Thu, Aug 2, 2018 at 8:38 AM, Andres Freund wrote: > Hi, > > On 2018-08-02 08:21:58 +0530, Amit Kapila wrote: >> I think something on the lines what Tom and you are suggesting can be >> done with the help of EXEC_FLAG_BACKWARD, but I don't see the need to >> do anything for this patch. The

Re: Explain buffers wrong counter with parallel plans

On Wed, Aug 1, 2018 at 7:51 PM, Robert Haas wrote: > On Sat, Jul 28, 2018 at 2:14 AM, Amit Kapila wrote: >> We have done verification that the approach works and fixes the bug in >> all known cases. Do you see any problem with this approach? > > Regarding the change to execParallel.c, I think

Re: Explain buffers wrong counter with parallel plans

Robert Haas writes: > Looking at it, this sounds suspiciously something where we could just > test EXEC_FLAG_BACKWARD, based on the comments for that flag, but I > wouldn't be willing to bet very much on me being right about that. > Can somebody try to fetch backward even if this flag wasn't

Re: Explain buffers wrong counter with parallel plans

On Wed, Aug 1, 2018 at 10:27 AM, Tom Lane wrote: > Robert Haas writes: >> ... But scanning backwards is >> a problem. I'm not exactly sure what the best way of handling that >> is, but one thing I think might work is to save ExecutePlan's >> execute_once flag in the EState and then make the

Re: Explain buffers wrong counter with parallel plans

Robert Haas writes: > ... But scanning backwards is > a problem. I'm not exactly sure what the best way of handling that > is, but one thing I think might work is to save ExecutePlan's > execute_once flag in the EState and then make the call in nodeLimit.c > and the one in ExecutePlan itself

Re: Explain buffers wrong counter with parallel plans

On Sat, Jul 28, 2018 at 2:14 AM, Amit Kapila wrote: > We have done verification that the approach works and fixes the bug in > all known cases. Do you see any problem with this approach? Regarding the change to execParallel.c, I think if you're going to move that code you should add a comment

Re: Explain buffers wrong counter with parallel plans

On Tue, Jul 31, 2018 at 12:58 AM, Andres Freund wrote: > Hi, > > I'm not an expert in the area of the code, but here's a review anyway. I > did not read through the entire thread. > > > I think we should try to get this fixed soon, to make some progress > towards release-ability. Or just declare

Re: Explain buffers wrong counter with parallel plans

Hi, I'm not an expert in the area of the code, but here's a review anyway. I did not read through the entire thread. I think we should try to get this fixed soon, to make some progress towards release-ability. Or just declare it to be entirely unrelated to the release, and remove it from the

Re: Explain buffers wrong counter with parallel plans

> On Jul 28, 2018, at 2:14 AM, Amit Kapila wrote: > > On Fri, Jul 27, 2018 at 11:12 PM, Jonathan S. Katz > wrote: >> >>> On Jul 27, 2018, at 8:31 AM, Amit Kapila wrote: >>> >>> >>> Yeah, that would be better. Today, I have tried the patch on both >>> Head and PG11 and I am getting same

Re: Explain buffers wrong counter with parallel plans

On Fri, Jul 27, 2018 at 11:12 PM, Jonathan S. Katz wrote: > >> On Jul 27, 2018, at 8:31 AM, Amit Kapila wrote: >> >> >> Yeah, that would be better. Today, I have tried the patch on both >> Head and PG11 and I am getting same and correct results. > > I have applied the the patch to PG11beta2 and

Re: Explain buffers wrong counter with parallel plans

> On Jul 27, 2018, at 8:31 AM, Amit Kapila wrote: > > On Thu, Jul 26, 2018 at 7:31 PM, Jonathan S. Katz > wrote: >> >>> On Jul 25, 2018, at 10:25 PM, Amit Kapila wrote: >>> >>> >>> You mean to say the number (Buffers: shared read=442478) in HEAD, >>> right? If so, yeah, I am also

Re: Explain buffers wrong counter with parallel plans

On Thu, Jul 26, 2018 at 7:31 PM, Jonathan S. Katz wrote: > >> On Jul 25, 2018, at 10:25 PM, Amit Kapila wrote: >> >> >> You mean to say the number (Buffers: shared read=442478) in HEAD, >> right? If so, yeah, I am also wondering why the results of the patch >> are different in HEAD and 11beta2.

Re: Explain buffers wrong counter with parallel plans

> On Jul 25, 2018, at 10:25 PM, Amit Kapila wrote: > > On Wed, Jul 25, 2018 at 7:42 PM, Jonathan S. Katz > wrote: >> >> On Jul 7, 2018, at 12:03 AM, Amit Kapila wrote: >> >> I tried running the below on both 11beta2 and HEAD with the patch >> applied: >> >> CREATE UNLOGGED TABLE t1 (c1

Re: Explain buffers wrong counter with parallel plans

On Wed, Jul 25, 2018 at 7:42 PM, Jonathan S. Katz wrote: > > On Jul 7, 2018, at 12:03 AM, Amit Kapila wrote: > > I tried running the below on both 11beta2 and HEAD with the patch > applied: > > CREATE UNLOGGED TABLE t1 (c1 int); > INSERT INTO t1 SELECT generate_series(1,1); > /** restart

Re: Explain buffers wrong counter with parallel plans

> On Jul 7, 2018, at 12:03 AM, Amit Kapila wrote: > > On Sat, Jul 7, 2018 at 7:45 AM, Amit Kapila > wrote: >> On Sat, Jul 7, 2018 at 12:44 AM, Robert Haas wrote: >>> On Fri, Jul 6, 2018 at 9:44 AM, Amit Kapila wrote: I have tried this idea, but it doesn't

Re: Explain buffers wrong counter with parallel plans

On Sat, Jul 7, 2018 at 7:45 AM, Amit Kapila wrote: > On Sat, Jul 7, 2018 at 12:44 AM, Robert Haas wrote: >> On Fri, Jul 6, 2018 at 9:44 AM, Amit Kapila wrote: >>> I have tried this idea, but it doesn't completely solve the problem. >>> The problem is that nodes below LIMIT won't get a chance to

Re: Explain buffers wrong counter with parallel plans

On Sat, Jul 7, 2018 at 12:44 AM, Robert Haas wrote: > On Fri, Jul 6, 2018 at 9:44 AM, Amit Kapila wrote: >> I have tried this idea, but it doesn't completely solve the problem. >> The problem is that nodes below LIMIT won't get a chance to accumulate >> the stats as they won't be able to call

Re: Explain buffers wrong counter with parallel plans

On Fri, Jul 6, 2018 at 9:44 AM, Amit Kapila wrote: > I have tried this idea, but it doesn't completely solve the problem. > The problem is that nodes below LIMIT won't get a chance to accumulate > the stats as they won't be able to call InstrStopNode. I'm not sure I understand. Why not? I see

Re: Explain buffers wrong counter with parallel plans

On Tue, Jul 3, 2018 at 4:18 PM, Amit Kapila wrote: > On Mon, Jul 2, 2018 at 6:02 PM, Robert Haas wrote: >> > >> To fix the problem with Limit that you mention, we could just modify >> nodeLimit.c so that when the state is changed from LIMIT_INWINDOW to >> LIMIT_WINDOWEND, we also call

Re: Explain buffers wrong counter with parallel plans

On Tue, Jul 3, 2018 at 6:48 AM, Amit Kapila wrote: > Yeah, previously, I have also pointed out the same code [1]. However, > I have not done any testing to prove it. Ah, OK. Sorry, forgot about that email. > Are you planning to work on it? If not, then I can look into it. I won't have time

Re: Explain buffers wrong counter with parallel plans

On Mon, Jul 2, 2018 at 6:02 PM, Robert Haas wrote: > > I think the core problem here is this hunk from gather_readnext: > > { > Assert(!tup); > -DestroyTupleQueueReader(reader); > --gatherstate->nreaders; > if (gatherstate->nreaders ==

Re: Explain buffers wrong counter with parallel plans

On Sun, Jun 10, 2018 at 1:18 AM, Amit Kapila wrote: > Right, I think we have following options: > (a) Come up with a solution which allows percolating the buffer usage > and or similar stats to upper nodes in all cases. > (b) Allow it to work for some of the cases as it was earlier. > > I think

Re: Explain buffers wrong counter with parallel plans

On Fri, Jun 29, 2018 at 6:12 PM, Tom Lane wrote: > Alvaro Herrera writes: >> I'm CCing Tom here, as author of the patch that caused (most of) the >> issue. > > Uh ... me? I thought this was a parallel-query issue, which I've > pretty much not been involved in. Well, it was your commit that

Re: Explain buffers wrong counter with parallel plans

On Sat, Jun 30, 2018 at 4:08 AM, Alvaro Herrera wrote: > On 2018-Jun-29, Tom Lane wrote: > >> Alvaro Herrera writes: >> > I'm CCing Tom here, as author of the patch that caused (most of) the >> > issue. >> >> Uh ... me? I thought this was a parallel-query issue, which I've >> pretty much not

Re: Explain buffers wrong counter with parallel plans

On 2018-Jun-29, Tom Lane wrote: > Alvaro Herrera writes: > > I'm CCing Tom here, as author of the patch that caused (most of) the > > issue. > > Uh ... me? I thought this was a parallel-query issue, which I've > pretty much not been involved in. Well, maybe it's a matter of opinion. Amit K

Re: Explain buffers wrong counter with parallel plans

Alvaro Herrera writes: > I'm CCing Tom here, as author of the patch that caused (most of) the > issue. Uh ... me? I thought this was a parallel-query issue, which I've pretty much not been involved in. regards, tom lane

Re: Explain buffers wrong counter with parallel plans

On 2018-Jun-08, Robert Haas wrote: > On Wed, Jun 6, 2018 at 12:08 PM, Alvaro Herrera > wrote: > > So, apparently this is not a Postgres 11 open item, but rather a bug > > that goes back to pg10. However, maybe it would be worth fixing soon > > anyway? In particular, if we want to perturb the

Re: Explain buffers wrong counter with parallel plans

On Fri, Jun 8, 2018 at 11:34 PM, Robert Haas wrote: > On Wed, Jun 6, 2018 at 12:08 PM, Alvaro Herrera > wrote: >> So, apparently this is not a Postgres 11 open item, but rather a bug >> that goes back to pg10. However, maybe it would be worth fixing soon >> anyway? In particular, if we want to

Re: Explain buffers wrong counter with parallel plans

On 2018-May-07, Robert Haas wrote: > On Sat, May 5, 2018 at 8:56 AM, Amit Kapila wrote: > > The reason why I think the current behavior is okay because it is > > coincidental that they were displayed correctly. We have not made any > > effort to percolate it to upper nodes. For ex., before

Re: Explain buffers wrong counter with parallel plans

On Sat, May 5, 2018 at 8:56 AM, Amit Kapila wrote: > The reason why I think the current behavior is okay because it is > coincidental that they were displayed correctly. We have not made any > effort to percolate it to upper nodes. For ex., before that commit > also, it

Re: Explain buffers wrong counter with parallel plans

On Fri, May 4, 2018 at 10:35 PM, Robert Haas wrote: > On Wed, May 2, 2018 at 11:37 AM, Adrien Nayrat > wrote: >> In 9.6 gather node reports sum of buffers for main process + workers. In 10, >> gather node only reports buffers from the main

Re: Explain buffers wrong counter with parallel plans

On Wed, May 2, 2018 at 11:37 AM, Adrien Nayrat wrote: > In 9.6 gather node reports sum of buffers for main process + workers. In 10, > gather node only reports buffers from the main process. Oh, really? Well, that sounds like a bug. Amit seems to think it's expected

Re: Explain buffers wrong counter with parallel plans

Hi, On 05/02/2018 05:22 PM, Robert Haas wrote: > On Tue, May 1, 2018 at 4:57 PM, Andres Freund wrote: >> Robert, you added this as an open item. I don't think it's clear that >> there's a bug here, and even less clear that it's something new for >> v11. Am I understanding

Re: Explain buffers wrong counter with parallel plans

On Tue, May 1, 2018 at 4:57 PM, Andres Freund wrote: > Robert, you added this as an open item. I don't think it's clear that > there's a bug here, and even less clear that it's something new for > v11. Am I understanding that right? Should we close this open item? Yeah, I

Re: Explain buffers wrong counter with parallel plans

On 05/01/2018 10:57 PM, Andres Freund wrote: > Robert, you added this as an open item. I don't think it's clear that > there's a bug here, and even less clear that it's something new for > v11. Am I understanding that right? Should we close this open item? Hi, FIY this change is related to

Re: Explain buffers wrong counter with parallel plans

Hi, On 2018-04-29 20:40:43 +0530, Amit Kapila wrote: > On Wed, Mar 28, 2018 at 12:07 AM, Adrien Nayrat > > QUERY PLAN > > > >

Re: Explain buffers wrong counter with parallel plans

On 04/29/2018 05:10 PM, Amit Kapila wrote: > Yeah, it would have been convenient, if Gather and Finalize Aggregate > displays that way as it would have been easier for users to > understand. However, as of now, the aggregated stats for parallel > workers and leader are displayed at

Re: Explain buffers wrong counter with parallel plans

On 04/29/2018 05:11 PM, Amit Kapila wrote: > I think you can try 'verbose' option, it will give per-worker stats. Thanks! I didn't know verbose give per-woker stats! -- Adrien signature.asc Description: OpenPGP digital signature

Re: Explain buffers wrong counter with parallel plans

On Fri, Apr 20, 2018 at 6:36 PM, Adrien Nayrat wrote: > Hello, > > I tried to understand this issue and it seems Gather node only take account of > this own buffer usage : > > > create unlogged table t1 (c1 int); > insert into t1 select generate_series(1,100); >

Re: Explain buffers wrong counter with parallel plans

On Wed, Mar 28, 2018 at 12:07 AM, Adrien Nayrat wrote: > Hello, > > I notice Explain report wrong counters with parallel plans : > > create unlogged table t1 (c1 int); > insert into t1 select generate_series(1,1); > vacuum t1; > > (stop PG, drop caches,start) >

Re: Explain buffers wrong counter with parallel plans

Hello, I tried to understand this issue and it seems Gather node only take account of this own buffer usage : create unlogged table t1 (c1 int); insert into t1 select generate_series(1,100); vacuum t1; explain (analyze,buffers,timing off,costs off) select count(*) from t1;

Explain buffers wrong counter with parallel plans

Hello, I notice Explain report wrong counters with parallel plans : create unlogged table t1 (c1 int); insert into t1 select generate_series(1,1); vacuum t1; (stop PG, drop caches,start) set track_io_timing to on; explain (analyze,buffers) select count(*) from t1;