Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 18 Aug 2022 12:31:00 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Cary Huang <cary.huang@highgo.ca>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add last failed connection error message to pg_stat_wal_receiver
Message-ID: <Yv2ydB/7bLUEY/tr@paquier.xyz>
References: 
 <CALj2ACWt6wkFnu7sac3pUb4WLZ3jA1m3cu7FJE3EovpGKzhwKw@mail.gmail.com>
 <165852348876.1168.5615622255167011778.pgcf@coridan.postgresql.org>
 <CALj2ACWUb_XPGz1Rgh_YWHs1dqX2Q7uSPY5in4w=JXbx4FssDQ@mail.gmail.com>
 <Yt5eCd2uEftTZIR1@paquier.xyz>
 <CALj2ACV=0yfSzNuDu2e=6C6EkN7Sa7D628pf4NBitdBJ51_zCw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="fGndvSITvH4104LQ"
Content-Disposition: inline
In-Reply-To: 
 <CALj2ACV=0yfSzNuDu2e=6C6EkN7Sa7D628pf4NBitdBJ51_zCw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/Yv2ydB/7bLUEY/tr%40paquier.xyz>
Precedence: bulk


--fGndvSITvH4104LQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Aug 04, 2022 at 03:27:11PM +0530, Bharath Rupireddy wrote:
> Good point. The walreceiver can exit for any reason. We can either 1)
> store for all the error messages or 2) think of using sigsetjmp but
> that only catches the ERROR kinds, leaving FATAL and PANIC messages.
> The option (1) is simple but there are problems - we may miss storing
> future error messages, good commenting and reviewing may help here and
> all the error messages now need to be stored in string, which is
> complex. The option (2) seems reasonable but we will miss FATAL and
> PANIC messages (we have many ERRORs, 2 FATALs, 3 PANICs). Maybe a
> combination of option (1) for FATALs and PANICs, and option (2) for
> ERRORs helps.
>=20
> Thoughts?

PANIC is not something you'd care about as the system would go down as
and shared memory would be reset (right?) even if restart_on_crash is
enabled.  Perhaps it would help here to use something like a macro to
catch and save the error, in a style similar to what's in hba.c for
example, which is the closest example I can think of, even if on ERROR
we don't really care about the error string anyway as there is nothing
to report back to the SQL views used for the HBA/ident files.

FATAL may prove to be tricky though, because I'd expect the error to
be saved in shared memory in this case.  This is particularly critical
as this takes the WAL receiver process down, actually.

Anyway, outside the potential scope of the proposal, there are more
things that I find strange with the code:
- Why isn't the string reset when the WAL receiver is starting up?
That surely is not OK to keep a past state not referring to what
actually happens with a receiver currently running.
- pg_stat_wal_receiver (system view) reports no rows if pid is NULL,
which would be the state stored in shared memory after a connection.
This means that one would never be able to see last_conn_error except
when calling directly the SQL function pg_stat_get_wal_receiver().

One could say that we should report a row for this view all the time,
but this creates a compatibility breakage: existing application
assuming something like (one row <=3D> WAL receiver running) could
break.
--
Michael

--fGndvSITvH4104LQ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmL9snQACgkQnvQgOdby
QH09cw/9HGxc4NcwIRaVJ6Brgg/uqvEcCDxUwFOk9OyaM90ZebznZLYKbFzLjsiT
rr5dj8xKCaQo8P92Qk6M+0p4gQM1/5XQBHUXeVs49gM1PNyi8my7qkWptmV5/CNo
Izp3lbG1c4dPGEKdKrNTXAkqFf+1WqlStesVxDnkYdErZzdzMGByoClTGF4J2h2K
k4pALEx7W3eIql0sI6k2RUEKWclpOcJ8mEZU1My2H9dvReS1MZVCz/WBW5enW2j9
T1pdLJpefdm1EdiPuez60O9IAw+woyXY/MZIvULF53XQlYm64kvd4iWPdj8Icv9x
amk2A87yPfHcquTpbjiST5hQEqUO9V3hOe1c9jPmrzaQagF73CaHHEOkrUepGtkT
pScolto93uHEKR0rBTTzRaU0ODAKZIqdZznaYjrFYPOxMAVdL3Fpx1eIrVJHjm6F
15dkYhDzTxhugCYjznKG54iXdIaPAlSsLAobCyaJ7AOp/WVKaRLgsRFStH6DvUWE
CSBO68azTE6GbJLr2joLVFcv2KNTTTZjBINb2xpyoiPocDvodoH+MgXWOjj/RPFK
LwOJ3WcGglCERo6uw8KeSYar+IMK9aQ2xZ4rS7QDa5pZeXmfWJc7PvXiFSihwyo0
LS+h7XfOe3wy6OoHKgLhsXkGWuZjHkVu5GPsG+hmOnKOn4d25h8=
=0hGO
-----END PGP SIGNATURE-----

--fGndvSITvH4104LQ--