SQLSTATE for replication connection failures

On Sat, Jun 12, 2021 at 9:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

So far as I can find, just about everyplace that deals with replication
connections has slipshod error reporting. An example from worker.c is

LogRepWorkerWalRcvConn = walrcv_connect(MySubscription->conninfo, true,
MySubscription->name, &err);
if (LogRepWorkerWalRcvConn == NULL)
ereport(ERROR,
(errmsg("could not connect to the publisher: %s", err)));

Because of the lack of any errcode() call, this failure will be reported
as XX000 ERRCODE_INTERNAL_ERROR, which is surely not appropriate.
worker.c is in good company though, because EVERY caller of walrcv_connect
is equally slipshod.

Shall we just use ERRCODE_CONNECTION_FAILURE for these failures, or
would it be better to invent another SQLSTATE code? Arguably,
ERRCODE_CONNECTION_FAILURE is meant for failures of client connections;
but on the other hand, a replication connection is a sort of client.

Your reasoning sounds good to me. So, +1 for using ERRCODE_CONNECTION_FAILURE.

--
With Regards,
Amit Kapila.

On Mon, Jun 14, 2021 at 6:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Sat, Jun 12, 2021 at 9:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

So far as I can find, just about everyplace that deals with replication
connections has slipshod error reporting. An example from worker.c is

LogRepWorkerWalRcvConn = walrcv_connect(MySubscription->conninfo, true,
MySubscription->name, &err);
if (LogRepWorkerWalRcvConn == NULL)
ereport(ERROR,
(errmsg("could not connect to the publisher: %s", err)));

Because of the lack of any errcode() call, this failure will be reported
as XX000 ERRCODE_INTERNAL_ERROR, which is surely not appropriate.
worker.c is in good company though, because EVERY caller of walrcv_connect
is equally slipshod.

Shall we just use ERRCODE_CONNECTION_FAILURE for these failures, or
would it be better to invent another SQLSTATE code? Arguably,
ERRCODE_CONNECTION_FAILURE is meant for failures of client connections;
but on the other hand, a replication connection is a sort of client.

Your reasoning sounds good to me. So, +1 for using ERRCODE_CONNECTION_FAILURE.

+1

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Masahiko Sawada <sawada.mshk@gmail.com> writes:

On Mon, Jun 14, 2021 at 6:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

Shall we just use ERRCODE_CONNECTION_FAILURE for these failures, or
would it be better to invent another SQLSTATE code? Arguably,
ERRCODE_CONNECTION_FAILURE is meant for failures of client connections;
but on the other hand, a replication connection is a sort of client.

Your reasoning sounds good to me. So, +1 for using ERRCODE_CONNECTION_FAILURE.

+1

Done that way. I also fixed some nearby ereports that were missing
errcodes; some of them seemed more like PROTOCOL_VIOLATIONs than
CONNECTION_FAILUREs, though.

regards, tom lane