MIME-Version: 1.0
References: <e587761c-eef3-1f83-bf16-6dadba0acaab@oss.nttdata.com>
 <CAPmGK15wkv1=2D2AHefdNcZJpt_d1qX-RW=738yvhyVSGUboTw@mail.gmail.com> <9ee1aca8-4678-fc71-33e6-85ee025608ee@oss.nttdata.com>
In-Reply-To: <9ee1aca8-4678-fc71-33e6-85ee025608ee@oss.nttdata.com>
From: Etsuro Fujita <etsuro.fujita@gmail.com>
Date: Fri, 14 Apr 2023 18:59:06 +0900
Message-ID: <CAPmGK15YigmZKzVBGYPdmo-QMHBdhNupK8L6kUBdUtMERkbXsQ@mail.gmail.com>
Subject: Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAPmGK15YigmZKzVBGYPdmo-QMHBdhNupK8L6kUBdUtMERkbXsQ%40mail.gmail.com>
Precedence: bulk

On Fri, Apr 14, 2023 at 3:19=E2=80=AFAM Fujii Masao <masao.fujii@oss.nttdat=
a.com> wrote:
> On 2023/04/13 15:13, Etsuro Fujita wrote:
> > I am not 100% sure that it is a good idea to use the same error
> > message "could not send cancel request" for the PQgetCancel() and
> > PQcancel() cases, because they are different functions.  How about
> > "could not create PGcancel structure=E2=80=9D or something like that, f=
or the
>
> The primary message basically should avoid reference to implementation de=
tails such as specific structure names like PGcancel, shouldn't it, as per =
the error message style guide?

I do not think that PGcancel is that specific, as it is described in
the user-facing documentation [1].  (In addition, the error message I
proposed was created by copying the existing error message "could not
create OpenSSL BIO structure" in contrib/sslinfo.c.)

> > former case, so we can distinguish the former error from the latter?
>
> Although the primary message is the same, the supplemental message provid=
es additional context that can help distinguish which function is reporting=
 the message.

If the user is familiar with the PQgetCancel/PQcancel internals, this
is true, but if not, I do not think this is always true.  Consider
this error message, for example:

2023-04-14 17:48:55.862 JST [24344] WARNING:  could not send cancel
request: invalid integer value "99999999999" for connection option
"keepalives"

It would be hard for users without the knowledge about those internals
to distinguish that from this message.  For average users, I think it
would be good to use a more distinguishable error message.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/docs/current/libpq-cancel.html