Re: Inherited UPDATE/DELETE vs async execution

Fujita-san,

On Tue, May 11, 2021 at 5:56 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fujita@gmail.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!

Thanks.

> > >  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.

Okay, so I take it that making these ForeignScan nodes (that only
fetch the data) asynchronous doesn't interfere with update/delete
subsequently being performed over presumably the same connection to
the remote server.

> > 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.

Ah I see.  I guess it makes sense to prevent such cases in v14 as your
patch does, and revisit this in the future.
--
Amit Langote
EDB: http://www.enterprisedb.com