Horton

Re: Explain buffers wrong counter with parallel plans

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, "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-02T04:56:44Z
Lists: pgsql-hackers 
On Thu, Aug 2, 2018 at 8:38 AM, Andres Freund <andres@anarazel.de> 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 change in nodeLimit.c is any way for
>> forward scans, so there shouldn't be any need for any other check.
>
> I think this is almost a guarantee to introduce bugs in the future. And
> besides that, as Robert points out, it's essentially an exiting bug for
> custom scans.  Given that EXEC_FLAG_BACKWARD already exists, why not do
> the right thing here?
>

Sure, if we want we can do it in this patch, but this is not the
problem of this patch.  It is also related to existing usage of
shutdown callbacks.  I think we can use es_top_eflags in Estate to
detect it and then call shutdown only if EXEC_FLAG_BACKWARD is not
set.  I think we should do that as a separate patch either before or
after this patch rather than conflating that change into this patch.
IIUC, then Robert also said that we should fix that separately.  I
will do as per whatever consensus we reach here.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

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.