Re: RFC: pg_stat_logmsg

On 7/16/24 19:08, Michael Paquier wrote:
> On Wed, Jul 17, 2024 at 12:14:36AM +0200, Tomas Vondra wrote:
>> I noticed this patch hasn't moved since September 2023, so I wonder
>> what's the main blocker / what is needed to move this?
>
> + /* Location of permanent stats file (valid when database is shut down) */
> + #define PGLM_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY
> "/pg_stat_logmsg.stat
>
> Perhaps this does not count as a valid reason, but does it really make
> sense to implement things this way, knowing that this could be changed
> to rely on a potential pluggable pgstats? I mean this one I've
> proposed:
> https://www.postgresql.org/message-id/Zmqm9j5EO0I4W8dx%40paquier.xyz

Yep, see my adjacent reply to Tomas.

> One potential implementation is to stick that to be fixed-numbered,
> because there is a maximum cap to the number of entries proposed by
> the patch, while keeping the whole in memory.
>
> + logmsg_store(ErrorData *edata, Size *logmsg_offset,
> + int *logmsg_len, int *gc_count)
>
> The patch shares a lot of perks with pg_stat_statements that don't
> scale well. I'm wondering if it is a good idea to duplicate these
> properties in a second, different, module, like the external file can
> could be written out on a periodic basis depending on the workload.
> I am not saying that the other thread is a magic solution for
> everything (not looked yet at how this would plug with the cap in
> entries that pg_stat_statements wants), just one option on the table.
>
>> As for the code, I wonder if the instability of line numbers could be a
>> problem - these can change (a little bit) between minor releases, so
>> after an upgrade we'll read the dump file with line numbers from the old
>> release, and then start adding entries with new line numbers. Do we need
>> to handle this in some way?
>
> Indeed. Perhaps a PostgreSQL version number assigned to each entry to
> know from which binary an entry comes from? This would cost a couple
> of extra bytes for each entry still that would be the best information
> possible to match that with a correct code tree. If it comes to that,
> even getting down to a commit SHA1 could be useful to provide the
> lowest level of granularity. Another thing would be to give up on the
> line number, stick to the uniqueness in the stats entries with the
> errcode and the file name, but that won't help for things like
> tablecmds.c.

I think including version in the key makes most sense. Also do we even
have a mechanism to grab the commit sha in running code?

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com