Horton

Re: Explain buffers wrong counter with parallel plans

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "Jonathan S. Katz" <jonathan.katz@excoventures.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Adrien Nayrat <adrien.nayrat@anayrat.info>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2018-08-02T17:44:58Z
Lists: pgsql-hackers 
On Thu, Aug 2, 2018 at 5:41 AM, Amit Kapila <amit.kapila16@gmail.com> 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 in ExecShutdownNode (c) Probit calling
> ExecShutdownNode if there is a possibility of backward scans (I have
> done some basic tests with this patch, if we decide to proceed with
> it, then some more verification and testing would be required).
>
> I think we should commit first two patches as that fixes the problem
> being discussed in this thread and then do some additional
> verification for the third patch (mentioned in option c).  I can
> understand if people want to commit the third patch before the second
> patch, so let me know what you guys think.

I'm happy with the first two patches.  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 the comment to which it refers, and because
future commits are likely to edit or remove that comment without
noticing the references to it from elsewhere.  Instead I would just
write, in all three places, /* If we know we won't need to back up, we
can release resources at this point. */ or something like that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Adjust comment atop ExecShutdownNode.

  2. Prohibit shutting down resources if there is a possibility of back up.

  3. Fix buffer usage stats for parallel nodes.

  4. Match the buffer usage tracking for leader and worker backends.

  5. Improve division of labor between execParallel.c and nodeGather[Merge].c.