At Mon, 11 Nov 2019 17:24:45 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> Thomas Munro <thomas.munro@gmail.com> writes:
> > On Tue, Nov 12, 2019 at 1:24 AM Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote:
> >> Hello. While looking a patch, I found that PHJ sometimes complains for
> >> file leaks if accompanied by LIMIT.
>
> > Thanks for the patch! Yeah, this seems correct, but I'd like to think
> > about it some more before committing. I'm going to be a bit tied up
> > with travel so that might be next week.
>
> At this point we've missed the window for this week's releases, so
> there's no great hurry (and it'd be best not to push any noncritical
> patches into the back branches anyway, for the next 24 hours).
>
> 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?
It's a simple race conditions between leader and workers.
If a scan on workers reached to the end, no batch file is open, but a
batch file is open if it doesn't reaches to the end.
If a worker notices that the channel to the leader is already closed
before reaching the limit, it calls ExecEndNode and doesn't call
ExecShutdownNode. Otherwise, if the worker finds the limit first, the
worker *shutdowns* itself then ends. Everything's clean.
On second thought, it seems a issue of ExecutePlan, rather than PHJ
itself. ExecutePlan should shutdown nodes when output channel is
broken.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index ea4b586984..038bafe777 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1683,7 +1683,15 @@ ExecutePlan(EState *estate,
* end the loop.
*/
if (!dest->receiveSlot(slot, dest))
+ {
+ /*
+ * If we know we won't need to back up, we can release
+ * resources at this point.
+ */
+ if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
+ (void) ExecShutdownNode(planstate);
break;
+ }
}
/*