Amit-san,
On Tue, May 11, 2021 at 9:53 PM Amit Langote <amitlangote09@gmail.com> wrote:
> 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:
> > > > 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.
Good point! I don't think it would interfere with the update/delete,
because in that case postgres_fdw would actually perform the
update/delete and the asynchronous foreign scans serially rather than
concurrently. (They wouldn't be perfomed in parallel unless they use
different connections, in other words.)
> > > 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.
+1
Here is a rebased version of the patch. I'm planning to apply this tommorow.
Thanks for the comment!
Best regards,
Etsuro Fujita