Re: PHJ file leak.

On Tue, Nov 12, 2019 at 4:23 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Tue, Nov 12, 2019 at 4:20 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > At Mon, 11 Nov 2019 17:24:45 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> > > Although the patch seems unobjectionable as far as it goes, I'd like
> > > to understand why we didn't see the need for it long since. Is there
> > > another call to ExecParallelHashCloseBatchAccessors somewhere, and
> > > if so, is that one wrongly placed?
> >
> > The previous patch would be wrong. The root cause is a open batch so
> > the right thing to be done at scan end is
> > ExecHashTableDeatchBatch. And the real issue here seems to be in
> > ExecutePlan, not in PHJ.
>
> You are right. Here is the email I just wrote that says the same
> thing, but with less efficiency:

And yeah, your Make_parallel_shutdown_on_broken_channel.patch seems
like the real fix here. It's not optional to run that at
end-of-query, though you might get that impression from various
comments, and it's also not OK to call it before the end of the query,
though you might get that impression from what the code actually does.
CC'ing to Robert who designed ExecShutdownNode(), though admittedly
the matter might have been theoretical until PHJ came along (since the
only other executor node that implements ExecShutdownNode() was
Gather, and you can't have a Gather under a Gather, and if this
happens to you in the leader process then the user is gone and no one
will hear the screams).