public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Cary Huang <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Add last failed connection error message to pg_stat_wal_receiver
Date: Thu, 18 Aug 2022 12:31:00 +0900
Message-ID: <Yv2ydB/7bLUEY/[email protected]> (raw)
In-Reply-To: <CALj2ACV=0yfSzNuDu2e=6C6EkN7Sa7D628pf4NBitdBJ51_zCw@mail.gmail.com>
References: <CALj2ACWt6wkFnu7sac3pUb4WLZ3jA1m3cu7FJE3EovpGKzhwKw@mail.gmail.com>
<165852348876.1168.5615622255167011778.pgcf@coridan.postgresql.org>
<CALj2ACWUb_XPGz1Rgh_YWHs1dqX2Q7uSPY5in4w=JXbx4FssDQ@mail.gmail.com>
<[email protected]>
<CALj2ACV=0yfSzNuDu2e=6C6EkN7Sa7D628pf4NBitdBJ51_zCw@mail.gmail.com>
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.
>
> 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 <=> WAL receiver running) could
break.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../Yv2ydB%2F7bLUEY%[email protected]/2-signature.asc)
download
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add last failed connection error message to pg_stat_wal_receiver
In-Reply-To: <Yv2ydB/7bLUEY/[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox