Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply

On Thu, 11 Jan 2024 at 20:00, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 13 Apr 2023 at 23:34, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >
> >
> >
> > On 2023/04/13 11:00, Kyotaro Horiguchi wrote:
> > > Agreed, it seems to be a leftover when we moved to parse_int_param()
> > > in that area.
> >
> > It looks like there was an oversight in commit e7a2217978. I've attached a patch (0002) that updates PQconnectPoll() to use parse_int_param() for parsing the keepalives parameter.
> >
> > As this change is not directly related to the bug fix, it may not be necessary to back-patch it to the stable versions, I think. Thought?
> >
> >
> > >> To clarify, are you suggesting that PQgetCancel() should
> > >> only parse the parameters for TCP connections
> > >> if cancel->raddr.addr.ss_family != AF_UNIX?
> > >> If so, I think that's a good idea.
> > >
> > > You're right. I used connip in the diff because I thought it provided
> > > the same condition, but in a simpler way.
> >
> > I made a modification to the 0001 patch. It will now allow PQgetCancel() to parse and interpret TCP connection parameters only when the connection is not made through a Unix-domain socket.
> >
> >
> > > However, I notcied that PQgetCancel() doesn't set errbuf.. So, I'm
> > > fine with your proposal.
> >
> > Ok.
> >
> >
> > >> I think it is important to inform the user when an error
> > >> occurs and a cancel request cannot be sent, as this information
> > >> can help them identify the cause of the problem (such as
> > >> setting an overly large value for the keepalives parameter).
> > >
> > > Although I view it as an internal error, I agree with emitting some
> > > error messages in that situation.
> >
> > Ok.
>
> I have changed the status of the patch to "Waiting on Author" as all
> the issues are not addressed. Feel free to address them and change the
> status accordingly.

The patch which you submitted has been awaiting your attention for
quite some time now. As such, we have moved it to "Returned with
Feedback" and removed it from the reviewing queue. Depending on
timing, this may be reversible. Kindly address the feedback you have
received, and resubmit the patch to the next CommitFest.

Regards,
Vignesh