Re: Inherited UPDATE/DELETE vs async execution

Amit-san,

On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> > I noticed this while working on the
> > EXPLAIN-ANALYZE-for-async-capable-nodes issue:
> >
> > EXPLAIN (VERBOSE, COSTS OFF)
> > DELETE FROM async_pt;
> > QUERY PLAN
> > ----------------------------------------------------------------
> > Delete on public.async_pt
> > Foreign Delete on public.async_p1 async_pt_1
> > Foreign Delete on public.async_p2 async_pt_2
> > Delete on public.async_p3 async_pt_3
> > -> Append
> > -> Async Foreign Delete on public.async_p1 async_pt_1
> > Remote SQL: DELETE FROM public.base_tbl1
> > -> Async Foreign Delete on public.async_p2 async_pt_2
> > Remote SQL: DELETE FROM public.base_tbl2
> > -> Seq Scan on public.async_p3 async_pt_3
> > Output: async_pt_3.tableoid, async_pt_3.ctid
> > (11 rows)
> >
> > DELETE FROM async_pt;
> > server closed the connection unexpectedly
> > This probably means the server terminated abnormally
> > before or while processing the request.
> > connection to server was lost
> >
> > The cause for this would be that direct-update plans are mistakenly
> > treated as async-capable ones, as shown in the EXPLAIN output.
>
> I guess that happens because the ForeignScan nodes responsible for
> scanning or direct-updating/deleting from child foreign tables appear
> under an Append as of 86dc90056, whereas before they would appear as
> child plans of a ModifyTable node. IIUC, it's the Append that causes
> the async_capable flag to be set in those ForeignScan nodes.

That's right.

The inherited update/delete work is great! Thanks for that!

> > To
> > fix, I think we should modify postgresPlanDirectModify() so that it
> > clears the async-capable flag if it is set. Attached is a patch for
> > that. Maybe I am missing something, though.
>
> I see that your patch is to disable asynchronous execution in
> ForeignScan nodes responsible for direct update/delete, but why not do
> the same for other ForeignScan nodes too?

I just thought it would be better to execute other ForeignScan nodes
asynchronously for performance, if they are async-capable.

> Or the other way around --
> is it because fixing the crash that occurs in the former's case would
> be a significant undertaking for little gain?

Yeah, I think it would be a good idea to support "Async Foreign
Delete" in the former's case. And actually, I tried to do so, but I
didn't, because it seemed to take time. I might be missing something,
though.

Thanks!

Best regards,
Etsuro Fujita