Re: Asynchronous Append on postgres_fdw nodes.

On Sat, Dec 19, 2020 at 5:55 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Mon, Dec 14, 2020 at 4:01 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > At Sat, 12 Dec 2020 18:25:57 +0900, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote in
> > > On Fri, Nov 20, 2020 at 3:51 PM Kyotaro Horiguchi
> > > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > > > The reason for
> > > > the early fetching is letting fdw send the next request as early as
> > > > possible. (However, I didn't measure the effect of the
> > > > nodeAppend-level prefetching.)
> > >
> > > I agree that that would lead to an improved efficiency in some cases,
> > > but I still think that that would be useless in some other cases like
> > > SELECT * FROM sharded_table LIMIT 1. Also, I think the situation
> > > would get worse if we support Append on top of joins or aggregates
> > > over ForeignScans, which would be more expensive to perform than these
> > > ForeignScans.
> >
> > I'm not sure which gain we weigh on, but if doing "LIMIT 1" on Append
> > for many times is more common than fetching all or "LIMIT <many
> > multiples of fetch_size>", that discussion would be convincing... Is
> > it really the case?
>
> I don't have a clear answer for that... Performance in the case you
> mentioned would be improved by async execution without prefetching by
> Append, so it seemed reasonable to me to remove that prefetching to
> avoid unnecessary overheads in the case I mentioned. BUT: I started
> to think my proposal, which needs an additional FDW callback routine
> (ie, ForeignAsyncBegin()), might be a bad idea, because it would
> increase the burden on FDW authors.

I dropped my proposal; I modified the patch so that ExecAppend()
requests tuples from all subplans needing a request *at once*, as
originally proposed by Robert and then you. Please find attached a
new version of the patch.

Other changes:

* I renamed ExecAppendAsyncResponse() to what was originally proposed
by Robert, and modified the patch so that that function is called from
the requestee side, not the requestor side as in the previous version.

* I renamed the variable async_aware as explained by you.

* I tweaked comments a bit to address your comments.

* I made code simpler, and added a bit more assertions.

Best regards,
Etsuro Fujita