Amit-san, On Tue, May 11, 2021 at 9:53 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Tue, May 11, 2021 at 5:56 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: > > On Mon, May 10, 2021 at 9:21 PM Amit Langote <amitlangot...@gmail.com> > > wrote: > > > On Sat, May 8, 2021 at 1:21 AM Etsuro Fujita <etsuro.fuj...@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
fix-postgresPlanDirectModify-v2.patch
Description: Binary data