Thread

1. RFC: pg_stat_logmsg

   Joe Conway <mail@joeconway.com> — 2023-06-30T23:57:09Z

   Greetings,
   
   Attached please find a tarball (rather than a patch) for a proposed new 
   contrib extension, pg_stat_logmsg.
   
   The basic idea is to mirror how pg_stat_statements works, except the 
   logged messages keyed by filename, lineno, and elevel are saved with a 
   aggregate count. The format string is displayed (similar to a query 
   jumble) for context, along with function name and sqlerrcode.
   
   I threw this together rather quickly over the past couple of days 
   between meetings, so not claiming that it is committable (and lacks 
   documentation and regression tests as well), but I would love to get 
   feedback on:
   
   1/ the general concept
   2/ the pg_stat_statement-like implementation
   3/ contrib vs core vs external project
   
   Some samples and data:
   
   `make installcheck` with the extension loaded:
   8<------------------
   # All 215 tests passed.
   
   
   real    2m24.854s
   user    0m0.086s
   sys     0m0.283s
   8<------------------
   
   `make installcheck` without the extension loaded:
   8<------------------
   
   # All 215 tests passed.
   
   real    2m26.765s
   user    0m0.076s
   sys     0m0.293s
   8<------------------
   
   Sample output after running make installcheck a couple times (plus a few 
   manually generated ERRORs):
   
   8<------------------
   test=# select sum(count) from pg_stat_logmsg where elevel > 20;
      sum
   -------
     10554
   (1 row)
   
   test=# \x
   Expanded display is on.
   test=# select * from pg_stat_logmsg where elevel > 20 order by count desc;
   -[ RECORD 1 ]-------------------------------
   filename   | aclchk.c
   lineno     | 2811
   elevel     | 21
   funcname   | aclcheck_error
   sqlerrcode | 42501
   message    | permission denied for schema %s
   count      | 578
   -[ RECORD 2 ]-------------------------------
   filename   | scan.l
   lineno     | 1241
   elevel     | 21
   funcname   | scanner_yyerror
   sqlerrcode | 42601
   message    | %s at or near "%s"
   count      | 265
   ...
   
   test=# select * from pg_stat_logmsg where elevel > 20 and sqlerrcode = 
   'XX000';
   -[ RECORD 1 ]---------------------------------------
   filename   | tid.c
   lineno     | 352
   elevel     | 21
   funcname   | currtid_for_view
   sqlerrcode | XX000
   message    | ctid isn't of type TID
   count      | 2
   -[ RECORD 2 ]---------------------------------------
   filename   | pg_locale.c
   lineno     | 2493
   elevel     | 21
   funcname   | pg_ucol_open
   sqlerrcode | XX000
   message    | could not open collator for locale "%s": %s
   count      | 2
   ...
   
   8<------------------
   
   Part of the thinking is that people with fleets of postgres instances 
   can use this to scan for various errors that they care about. 
   Additionally it would be useful to look for "should not happen" errors.
   
   I will register this in the July CF and will appreciate feedback.
   
   Thanks!
   
   -- 
   Joe Conway
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com

2. Re: RFC: pg_stat_logmsg

   Pavel Stehule <pavel.stehule@gmail.com> — 2023-07-01T03:20:08Z

   Hi
   
   so 1. 7. 2023 v 1:57 odesílatel Joe Conway <mail@joeconway.com> napsal:
   
   > Greetings,
   >
   > Attached please find a tarball (rather than a patch) for a proposed new
   > contrib extension, pg_stat_logmsg.
   >
   > The basic idea is to mirror how pg_stat_statements works, except the
   > logged messages keyed by filename, lineno, and elevel are saved with a
   > aggregate count. The format string is displayed (similar to a query
   > jumble) for context, along with function name and sqlerrcode.
   >
   > I threw this together rather quickly over the past couple of days
   > between meetings, so not claiming that it is committable (and lacks
   > documentation and regression tests as well), but I would love to get
   > feedback on:
   >
   > 1/ the general concept
   > 2/ the pg_stat_statement-like implementation
   > 3/ contrib vs core vs external project
   >
   > Some samples and data:
   >
   > `make installcheck` with the extension loaded:
   > 8<------------------
   > # All 215 tests passed.
   >
   >
   > real    2m24.854s
   > user    0m0.086s
   > sys     0m0.283s
   > 8<------------------
   >
   > `make installcheck` without the extension loaded:
   > 8<------------------
   >
   > # All 215 tests passed.
   >
   > real    2m26.765s
   > user    0m0.076s
   > sys     0m0.293s
   > 8<------------------
   >
   > Sample output after running make installcheck a couple times (plus a few
   > manually generated ERRORs):
   >
   > 8<------------------
   > test=# select sum(count) from pg_stat_logmsg where elevel > 20;
   >    sum
   > -------
   >   10554
   > (1 row)
   >
   > test=# \x
   > Expanded display is on.
   > test=# select * from pg_stat_logmsg where elevel > 20 order by count desc;
   > -[ RECORD 1 ]-------------------------------
   > filename   | aclchk.c
   > lineno     | 2811
   > elevel     | 21
   > funcname   | aclcheck_error
   > sqlerrcode | 42501
   > message    | permission denied for schema %s
   > count      | 578
   > -[ RECORD 2 ]-------------------------------
   > filename   | scan.l
   > lineno     | 1241
   > elevel     | 21
   > funcname   | scanner_yyerror
   > sqlerrcode | 42601
   > message    | %s at or near "%s"
   > count      | 265
   > ...
   >
   > test=# select * from pg_stat_logmsg where elevel > 20 and sqlerrcode =
   > 'XX000';
   > -[ RECORD 1 ]---------------------------------------
   > filename   | tid.c
   > lineno     | 352
   > elevel     | 21
   > funcname   | currtid_for_view
   > sqlerrcode | XX000
   > message    | ctid isn't of type TID
   > count      | 2
   > -[ RECORD 2 ]---------------------------------------
   > filename   | pg_locale.c
   > lineno     | 2493
   > elevel     | 21
   > funcname   | pg_ucol_open
   > sqlerrcode | XX000
   > message    | could not open collator for locale "%s": %s
   > count      | 2
   > ...
   >
   > 8<------------------
   >
   > Part of the thinking is that people with fleets of postgres instances
   > can use this to scan for various errors that they care about.
   > Additionally it would be useful to look for "should not happen" errors.
   >
   > I will register this in the July CF and will appreciate feedback.
   >
   
   This can be a very interesting feature. I like it.
   
   Regards
   
   Pavel
   
   
   > Thanks!
   >
   > --
   > Joe Conway
   > PostgreSQL Contributors Team
   > RDS Open Source Databases
   > Amazon Web Services: https://aws.amazon.com
   

3. Re: RFC: pg_stat_logmsg

   Joe Conway <mail@joeconway.com> — 2023-07-01T19:52:50Z

   On 6/30/23 23:20, Pavel Stehule wrote:
   > so 1. 7. 2023 v 1:57 odesílatel Joe Conway <mail@joeconway.com 
   > <mailto:mail@joeconway.com>> napsal:
   >     Part of the thinking is that people with fleets of postgres instances
   >     can use this to scan for various errors that they care about.
   >     Additionally it would be useful to look for "should not happen" errors.
   > 
   >     I will register this in the July CF and will appreciate feedback.
   > 
   > This can be a very interesting feature. I like it.
   
   Thanks!
   
   FWIW, I just modified it to provide the localized text of the elevel 
   rather than the internal number. I also localized the message format string:
   
   8<------------------------------
   psql (16beta2)
   Type "help" for help.
   
   test=# \x
   Expanded display is on.
   test=# select * from pg_stat_logmsg where elevel = 'ERROR' and 
   sqlerrcode = 'XX000' and count > 1;
   -[ RECORD 1 ]---------------------------------------------
   filename   | tablecmds.c
   lineno     | 10908
   elevel     | ERROR
   funcname   | ATExecAlterConstraint
   sqlerrcode | XX000
   message    | cannot alter constraint "%s" on relation "%s"
   count      | 2
   -[ RECORD 2 ]---------------------------------------------
   filename   | user.c
   lineno     | 2130
   elevel     | ERROR
   funcname   | check_role_membership_authorization
   sqlerrcode | XX000
   message    | role "%s" cannot have explicit members
   count      | 2
   
   test=# set lc_messages ='sv_SE.UTF8';
   SET
   test=# select * from pg_stat_logmsg where elevel = 'FEL' and sqlerrcode 
   = 'XX000' and count > 1;
   -[ RECORD 1 ]---------------------------------------------
   filename   | tablecmds.c
   lineno     | 10908
   elevel     | FEL
   funcname   | ATExecAlterConstraint
   sqlerrcode | XX000
   message    | kan inte ändra villkoret "%s" i relation "%s"
   count      | 2
   -[ RECORD 2 ]---------------------------------------------
   filename   | user.c
   lineno     | 2130
   elevel     | FEL
   funcname   | check_role_membership_authorization
   sqlerrcode | XX000
   message    | rollen "%s" kan inte ha explicita medlemmar
   count      | 2
   8<------------------------------
   
   New tarball attached.
   
   -- 
   Joe Conway
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   

4. Re: RFC: pg_stat_logmsg

   Masahiko Sawada <sawada.mshk@gmail.com> — 2023-07-06T07:36:13Z

   Hi,
   
   On Sat, Jul 1, 2023 at 8:57 AM Joe Conway <mail@joeconway.com> wrote:
   >
   > Greetings,
   >
   > Attached please find a tarball (rather than a patch) for a proposed new
   > contrib extension, pg_stat_logmsg.
   >
   > The basic idea is to mirror how pg_stat_statements works, except the
   > logged messages keyed by filename, lineno, and elevel are saved with a
   > aggregate count. The format string is displayed (similar to a query
   > jumble) for context, along with function name and sqlerrcode.
   >
   >
   > Part of the thinking is that people with fleets of postgres instances
   > can use this to scan for various errors that they care about.
   > Additionally it would be useful to look for "should not happen" errors.
   
   Interesting idea and use cases.
   
   I'm concerned that displaying the format string could not be helpful
   in some cases. For example, when raising an ERROR while reading WAL
   records, we typically write the error message stored in
   record->errormsg_buf:
   
   in ReadRecord():
               if (errormsg)
                   ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
                           (errmsg_internal("%s", errormsg) /* already
   translated */ ));
   
   In this case, the error message stored in pg_stat_logmsg is just '%s'.
   The filename and line number columns might help identify the actual
   error but it requires users to read the source code and cannot know
   the actual error message.
   
   A similar case is where we construct the error message on the fly. For
   example, in LogRecoveryConflict() the string of the recovery conflict
   description comes from get_recovery_conflict_desc():
   
   in LogRecoveryConflict():
           ereport(LOG,
                   errmsg("recovery still waiting after %ld.%03d ms: %s",
                          msecs, usecs, get_recovery_conflict_desc(reason)),
                   nprocs > 0 ? errdetail_log_plural("Conflicting process: %s.",
                                                     "Conflicting processes: %s.",
                                                     nprocs, buf.data) : 0);
   
   The user might want to search the error message by the actual conflict
   reason, but cannot. In this case, I'd like to see the actual error
   message (I'd like to normalize the number part, though).
   
   That being said, using the format string for the error messages like
   "ERROR:  relation "nonexist_table" does not exist" makes sense to me
   since we can avoid having too many similar entries.
   
   So I feel that we might need to figure out what part of the log
   message should be normalized like pg_stat_statement does for query
   strings.
   
   Regards,
   
   --
   Masahiko Sawada
   Amazon Web Services: https://aws.amazon.com
   
   
   
   

5. Re: RFC: pg_stat_logmsg

   Gurjeet Singh <gurjeet@singh.im> — 2023-07-07T05:38:33Z

   On Thu, Jul 6, 2023 at 12:37 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
   > On Sat, Jul 1, 2023 at 8:57 AM Joe Conway <mail@joeconway.com> wrote:
   > >
   > > The basic idea is to mirror how pg_stat_statements works, except the
   > > logged messages keyed by filename, lineno, and elevel are saved with a
   > > aggregate count. The format string is displayed (similar to a query
   > > jumble) for context, along with function name and sqlerrcode.
   > >
   > >
   > > Part of the thinking is that people with fleets of postgres instances
   > > can use this to scan for various errors that they care about.
   > > Additionally it would be useful to look for "should not happen" errors.
   
   > I'm concerned that displaying the format string could not be helpful
   > in some cases. For example, when raising an ERROR while reading WAL
   > records, we typically write the error message stored in
   > record->errormsg_buf:
   >
   > in ReadRecord():
   >             if (errormsg)
   >                 ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
   >                         (errmsg_internal("%s", errormsg) /* already
   > translated */ ));
   >
   > In this case, the error message stored in pg_stat_logmsg is just '%s'.
   > The filename and line number columns might help identify the actual
   > error but it requires users to read the source code and cannot know
   > the actual error message.
   
   I believe that the number of such error messages, the ones with very
   little descriptive content, is very low in Postgres code. These kinds
   of messages are not prevalent, and must be used when code hits obscure
   conditions, like seen in your example above. These are the kinds of
   errors that Joe is referring to as "should not happen". In these
   cases, even if the error message was descriptive, the user will very
   likely have to dive deep into code to find out the real cause.
   
   I feel that the unique combination of file name and line number is
   useful information, even in cases where the format string not very
   descriptive. So I believe the extension's behaviour in this regard is
   acceptable.
   
   In cases where the extension's output is not descriptive enough, the
   user can use the filename:lineno pair to look for fully formed error
   messages in the actual log files; they may have to make appropriate
   changes to log_* parameters, though.
   
   If we still feel strongly about getting the actual message for these
   cases, perhaps we can develop a heuristic to identify such messages,
   and use either full or a prefix of the 'message' field, instead of
   'message_id' field. The heuristic could be: strlen(message_id) is too
   short, or that message_id is all/overwhelmingly format specifiers
   (e.g. '%s: %d').
   
   The core benefit of this extension is to make it easy for the user to
   discover error messages affecting their workload. The user may be
   interested in knowing the most common log messages in their server
   logs, so that they can work on reducing those errors or warnings. Or
   they may be interested in knowing when their workload hits
   unexpected/serious error messages, even if they're infrequent. The
   data exposed by pg_stat_logmsg view would serve as a starting point,
   but I'm guessing it would not be sufficient for their investigation.
   
   On Fri, Jun 30, 2023 at 4:57 PM Joe Conway <mail@joeconway.com> wrote:
   > I would love to get
   > feedback on:
   >
   > 1/ the general concept
   > 2/ the pg_stat_statement-like implementation
   > 3/ contrib vs core vs external project
   
   +1 for the idea; a monitoring system trained at this view can bubble
   up problems to users that may otherwise go unnoticed. Piggybacking on,
   or using pg_stat_statments implementation as a model seems fine. I
   believe making this available as a contrib module hits the right
   balance; not all installations may want this, hence in-core, always
   collecting data, seems undesirable; if it's an external project, it
   won't be discoverable, I see that as a very high bar for someone to
   trust it and begin to use it.
   
   Best regards,
   Gurjeet
   http://Gurje.et
   
   
   
   

6. Re: RFC: pg_stat_logmsg

   Joe Conway <mail@joeconway.com> — 2023-07-09T18:13:09Z

   On 7/7/23 01:38, Gurjeet Singh wrote:
   > On Thu, Jul 6, 2023 at 12:37 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
   >> On Sat, Jul 1, 2023 at 8:57 AM Joe Conway <mail@joeconway.com> wrote:
   >> >
   >> > The basic idea is to mirror how pg_stat_statements works, except the
   >> > logged messages keyed by filename, lineno, and elevel are saved with a
   >> > aggregate count. The format string is displayed (similar to a query
   >> > jumble) for context, along with function name and sqlerrcode.
   >> >
   >> >
   >> > Part of the thinking is that people with fleets of postgres instances
   >> > can use this to scan for various errors that they care about.
   >> > Additionally it would be useful to look for "should not happen" errors.
   > 
   >> I'm concerned that displaying the format string could not be helpful
   >> in some cases. For example, when raising an ERROR while reading WAL
   >> records, we typically write the error message stored in
   >> record->errormsg_buf:
   >>
   >> in ReadRecord():
   >>             if (errormsg)
   >>                 ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
   >>                         (errmsg_internal("%s", errormsg) /* already
   >> translated */ ));
   >>
   >> In this case, the error message stored in pg_stat_logmsg is just '%s'.
   >> The filename and line number columns might help identify the actual
   >> error but it requires users to read the source code and cannot know
   >> the actual error message.
   > 
   > I believe that the number of such error messages, the ones with very
   > little descriptive content, is very low in Postgres code. These kinds
   > of messages are not prevalent, and must be used when code hits obscure
   > conditions, like seen in your example above. These are the kinds of
   > errors that Joe is referring to as "should not happen". In these
   > cases, even if the error message was descriptive, the user will very
   > likely have to dive deep into code to find out the real cause.
   
   Agreed. As an example, after running `make installcheck`
   
   8<-----------------
   select sum(count) from pg_stat_logmsg;
     sum
   ------
     6005
   (1 row)
   
   select message,sum(count)
   from pg_stat_logmsg
   where length(message) < 30
      and elevel in ('ERROR','FATAL','PANIC')
      and message like '%\%s%' escape '\'
   group by message
   order by length(message);
                message            | sum
   -------------------------------+-----
     %s                            | 107
     "%s" is a view                |   9
     "%s" is a table               |   4
     %s is a procedure             |   1
     invalid size: "%s"            |  13
     %s at or near "%s"            | 131
     %s at end of input            |   3
   ...
   8<-----------------
   
   I only see three message formats there that are generic enough to be of 
   concern (the first and last two shown -- FWIW I did not see any more of 
   them as the fmt string gets longer)
   
   So out of 6005 log events, 241 fit this concern.
   
   Perhaps given the small number of message format strings affected, it 
   would make sense to special case those, but I am not sure it is worth 
   the effort, at least for version 1.
   
   > I feel that the unique combination of file name and line number is
   > useful information, even in cases where the format string not very
   > descriptive. So I believe the extension's behaviour in this regard is
   > acceptable.
   > 
   > In cases where the extension's output is not descriptive enough, the
   > user can use the filename:lineno pair to look for fully formed error
   > messages in the actual log files; they may have to make appropriate
   > changes to log_* parameters, though.
   
   Right
   
   > If we still feel strongly about getting the actual message for these
   > cases, perhaps we can develop a heuristic to identify such messages,
   > and use either full or a prefix of the 'message' field, instead of
   > 'message_id' field. The heuristic could be: strlen(message_id) is too
   > short, or that message_id is all/overwhelmingly format specifiers
   > (e.g. '%s: %d').
   
   Based on the above analysis (though granted, not all inclusive), it 
   seems like just special casing the specific message format strings of 
   interest would work.
   
   > The core benefit of this extension is to make it easy for the user to
   > discover error messages affecting their workload. The user may be
   > interested in knowing the most common log messages in their server
   > logs, so that they can work on reducing those errors or warnings. Or
   > they may be interested in knowing when their workload hits
   > unexpected/serious error messages, even if they're infrequent. The
   > data exposed by pg_stat_logmsg view would serve as a starting point,
   > but I'm guessing it would not be sufficient for their investigation.
   
   Yes, exactly.
   
   -- 
   Joe Conway
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   
   
   
   
   

7. Re: RFC: pg_stat_logmsg

   Joe Conway <mail@joeconway.com> — 2023-09-13T19:30:45Z

   On 7/9/23 14:13, Joe Conway wrote:
   > On 7/7/23 01:38, Gurjeet Singh wrote:
   >>> In this case, the error message stored in pg_stat_logmsg is just '%s'.
   >>> The filename and line number columns might help identify the actual
   >>> error but it requires users to read the source code and cannot know
   >>> the actual error message.
   >> 
   >> I believe that the number of such error messages, the ones with very
   >> little descriptive content, is very low in Postgres code. These kinds
   >> of messages are not prevalent, and must be used when code hits obscure
   >> conditions, like seen in your example above. These are the kinds of
   >> errors that Joe is referring to as "should not happen". In these
   >> cases, even if the error message was descriptive, the user will very
   >> likely have to dive deep into code to find out the real cause.
   > 
   > Agreed. As an example, after running `make installcheck`
   > 
   > 8<-----------------
   > select sum(count) from pg_stat_logmsg;
   >    sum
   > ------
   >    6005
   > (1 row)
   > 
   > select message,sum(count)
   > from pg_stat_logmsg
   > where length(message) < 30
   >     and elevel in ('ERROR','FATAL','PANIC')
   >     and message like '%\%s%' escape '\'
   > group by message
   > order by length(message);
   >               message            | sum
   > -------------------------------+-----
   >    %s                            | 107
   >    "%s" is a view                |   9
   >    "%s" is a table               |   4
   >    %s is a procedure             |   1
   >    invalid size: "%s"            |  13
   >    %s at or near "%s"            | 131
   >    %s at end of input            |   3
   > ...
   > 8<-----------------
   > 
   > I only see three message formats there that are generic enough to be of
   > concern (the first and last two shown -- FWIW I did not see any more of
   > them as the fmt string gets longer)
   > 
   > So out of 6005 log events, 241 fit this concern.
   > 
   > Perhaps given the small number of message format strings affected, it
   > would make sense to special case those, but I am not sure it is worth
   > the effort, at least for version 1.
   
   Attached is an update, this time as a patch against 17devel. It is not 
   quite complete, but getting close IMHO.
   
   Changes:
   --------
   1. Now is integrated into contrib as a "Additional Supplied Extension"
   
   2. Documentation added
   
   3. I had a verbal conversation with Andres and he reminded me that the 
   original idea for this was to collect data across fleets of pg hosts so 
   we could look for how often "should never happen" errors actually 
   happen. As such, we need to think in terms of aggregating the info from 
   many hosts, potentially with many localized languages for the messages. 
   So I converted the "message" column back to an untranslated string, and 
   added a "translated_message" column which is localized.
   
   An alternative approach might be to provide a separate function that 
   accepts the message string and returns the translation. Thoughts on that?
   
   4. In the same vein, I added a pgversion column since the filename and 
   line number for the same log message could vary across major or even 
   minor releases.
   
   Not done:
   ---------
   1. The main thing lacking at this point is a regression test.
   
   2. No special casing for message == "%s". I am still not convinced it is 
   worthwhile to do so.
   
   Comments gratefully welcomed.
   
   Thanks,
   
   -- 
   Joe Conway
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   

8. Re: RFC: pg_stat_logmsg

   Tomas Vondra <tomas.vondra@enterprisedb.com> — 2024-07-16T22:14:36Z

   Hi,
   
   I noticed this patch hasn't moved since September 2023, so I wonder
   what's the main blocker / what is needed to move this?
   
   As for the feature, I've never done a fleet-wide analysis, so if this is
   one of the main use cases, I'm not really sure I can judge if this is a
   good tool for that. It seems like it might be a convenient way to do
   that, but does that require we add the module to contrib?
   
   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?
   
   This might be partially solved by eviction of entries from the old
   release - we apply decay, so after a while their usage will be 0. But
   what if there's no pressure for space, we'll not actually evict them.
   And it'll be confusing to have a mix of old/new line numbers.
   
   
   regards
   
   -- 
   Tomas Vondra
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   
   
   
   

9. Re: RFC: pg_stat_logmsg

   Michael Paquier <michael@paquier.xyz> — 2024-07-16T23:08:48Z

   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
   
   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.
   
   > This might be partially solved by eviction of entries from the old
   > release - we apply decay, so after a while their usage will be 0. But
   > what if there's no pressure for space, we'll not actually evict them.
   > And it'll be confusing to have a mix of old/new line numbers.
   
   Once we know that these stats not going to be relevant anymore as of a
   minor upgrade flow, resetting them could be the move that makes the
   most sense, leaving the reset to the provider doing the upgrades,
   while taking a snapshot of the past data before the reset?  I find the
   whole problem tricky to define, TBH.
   --
   Michael
   

10. Re: RFC: pg_stat_logmsg

    Joe Conway <mail@joeconway.com> — 2024-07-17T11:43:13Z

    On 7/16/24 18:14, 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?
    
    Mainly me finding time I'm afraid.
    
    > As for the feature, I've never done a fleet-wide analysis, so if this is
    > one of the main use cases, I'm not really sure I can judge if this is a
    > good tool for that. It seems like it might be a convenient way to do
    > that, but does that require we add the module to contrib?
    
    I had an offlist chat with Andres about this IIRC and he suggested he 
    thought it ought to be just built in to the backend as part of the 
    statistics subsystem. Lately though I have been toying with the idea of 
    keeping it as an extension and basing it off Michael Paquier's work for 
    Pluggable cumulative statistics.
    
    > 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?
    
    Hmm, yeah, I had been planning to include postgres version as part of 
    the output, but maybe it would need to be part of the key.
    
    > This might be partially solved by eviction of entries from the old
    > release - we apply decay, so after a while their usage will be 0. But
    > what if there's no pressure for space, we'll not actually evict them.
    > And it'll be confusing to have a mix of old/new line numbers.
    
    Agreed.
    
    I am going to try hard to get back to this sooner rather than later, but 
    realistically that might be in time for the September commitfest rather 
    than during this one.
    
    -- 
    Joe Conway
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
    
    
    
    

11. Re: RFC: pg_stat_logmsg

    Joe Conway <mail@joeconway.com> — 2024-07-17T11:48:15Z

    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
    
    
    
    
    

12. Re: RFC: pg_stat_logmsg

    Michael Paquier <michael@paquier.xyz> — 2024-07-18T04:26:29Z

    On Wed, Jul 17, 2024 at 07:48:15AM -0400, Joe Conway wrote:
    > 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?
    
    Not directly, still that's doable.
    
    The closest thing I would consider here is to get the output of
    something like 'git rev-parse --short HEAD` and attach it to
    PG_VERSION with --with-extra-version.  I do that in my local builds
    because I always want to know from which commit I am building
    something.  Then, PG_VERSION could be stored with the entries while
    hashing the stats key with the version string, the error code, the
    source file name and/or the line number for uniqueness.  32 bytes of
    room would be most likely enough when it comes to the PG_VERSION data
    stored in the stats entries.
    --
    Michael
    

13. Re: RFC: pg_stat_logmsg

    Michael Paquier <michael@paquier.xyz> — 2024-07-18T04:32:48Z

    On Wed, Jul 17, 2024 at 07:43:13AM -0400, Joe Conway wrote:
    > On 7/16/24 18:14, Tomas Vondra wrote:
    >> As for the feature, I've never done a fleet-wide analysis, so if this is
    >> one of the main use cases, I'm not really sure I can judge if this is a
    >> good tool for that. It seems like it might be a convenient way to do
    >> that, but does that require we add the module to contrib?
    > 
    > I had an offlist chat with Andres about this IIRC and he suggested he
    > thought it ought to be just built in to the backend as part of the
    > statistics subsystem. Lately though I have been toying with the idea of
    > keeping it as an extension and basing it off Michael Paquier's work for
    > Pluggable cumulative statistics.
    
    This may live better as a contrib/ module, serving as well as an extra
    template for what can be done with the pluggable stats.  Adding that
    in core is of course OK for me if that's the consensus.  The APIs for
    pluggable stats are really the same as what you would store in core,
    minus the system functions you'd want to add in the catalog .dat
    files, of course.
    
    I'd like to get it this part done by the end of this commit fest to
    have room with pg_stat_statements for this release, but well, we'll
    see.  As far as I can see everybody who commented on the thread seems
    kind of OK with the idea to fix the stats kinds IDs in time, like
    custom RMGRs.  That's just simpler implementation-wise, but I'm also
    looking for more opinions.
    
    > Hmm, yeah, I had been planning to include postgres version as part of the
    > output, but maybe it would need to be part of the key.
    
    Seems to me that you should do both, then: add PG_VERSION to the
    entries, and hash the keys with it for uniqueness.  You could also
    have a reset function that performs a removal of the stats for
    anything else than the current PG_VERSION, for example.
    --
    Michael
    

14. Re: RFC: pg_stat_logmsg

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-03-24T08:05:23Z

    Hi,
    
    On Thu, Jul 18, 2024 at 12:33 PM Michael Paquier <michael@paquier.xyz> wrote:
    >
    > On Wed, Jul 17, 2024 at 07:43:13AM -0400, Joe Conway wrote:
    > > On 7/16/24 18:14, Tomas Vondra wrote:
    > >> As for the feature, I've never done a fleet-wide analysis, so if this is
    > >> one of the main use cases, I'm not really sure I can judge if this is a
    > >> good tool for that. It seems like it might be a convenient way to do
    > >> that, but does that require we add the module to contrib?
    > >
    > > I had an offlist chat with Andres about this IIRC and he suggested he
    > > thought it ought to be just built in to the backend as part of the
    > > statistics subsystem. Lately though I have been toying with the idea of
    > > keeping it as an extension and basing it off Michael Paquier's work for
    > > Pluggable cumulative statistics.
    >
    > This may live better as a contrib/ module, serving as well as an extra
    > template for what can be done with the pluggable stats.  Adding that
    > in core is of course OK for me if that's the consensus.  The APIs for
    > pluggable stats are really the same as what you would store in core,
    > minus the system functions you'd want to add in the catalog .dat
    > files, of course.
    >
    > I'd like to get it this part done by the end of this commit fest to
    > have room with pg_stat_statements for this release, but well, we'll
    > see.  As far as I can see everybody who commented on the thread seems
    > kind of OK with the idea to fix the stats kinds IDs in time, like
    > custom RMGRs.  That's just simpler implementation-wise, but I'm also
    > looking for more opinions.
    >
    > > Hmm, yeah, I had been planning to include postgres version as part of the
    > > output, but maybe it would need to be part of the key.
    >
    > Seems to me that you should do both, then: add PG_VERSION to the
    > entries, and hash the keys with it for uniqueness.  You could also
    > have a reset function that performs a removal of the stats for
    > anything else than the current PG_VERSION, for example.
    > --
    > Michael
    
    
    This feature seems both interesting and useful, yet it hasn't
    progressed for some time. Looking at the thread history, the patch
    stalled around 2023 in part because the pluggable cumulative
    statistics infrastructure was still under development, which offered
    an opportunity to rework it on top of framework primitives rather than
    carrying DIY implementations of shared hash tables, persistence, and
    reset logic.
    
    The refactoring opportunity is now basically available. The relevant
    infrastructure landed in stages:
    
    7949d9594582 (Aug 2024) — kind registration, dshash routing, flush_pending_cb
    c06e71d1aca (Nov 2024) — write_to_file flag, enabling opt-in persistence
    4ba012a8ed9 (Dec 2025) — to_serialized_data / from_serialized_data /
    finish callbacks
    
    I’m considering picking this up and 'd like to move it forward. Before
    doing so, I am curious about whether there was a consensus on putting
    it into core or keeping it as an extension, and whether there are any
    major blockers I might have overlooked.
    
    
    Best,
    Xuneng
    
    
    
    

15. Re: RFC: pg_stat_logmsg

    Joe Conway <mail@joeconway.com> — 2026-03-28T14:14:04Z

    On 3/24/26 04:05, Xuneng Zhou wrote:
    > Hi,
    > 
    > On Thu, Jul 18, 2024 at 12:33 PM Michael Paquier <michael@paquier.xyz> wrote:
    >>
    >> On Wed, Jul 17, 2024 at 07:43:13AM -0400, Joe Conway wrote:
    >> > On 7/16/24 18:14, Tomas Vondra wrote:
    >> >> As for the feature, I've never done a fleet-wide analysis, so if this is
    >> >> one of the main use cases, I'm not really sure I can judge if this is a
    >> >> good tool for that. It seems like it might be a convenient way to do
    >> >> that, but does that require we add the module to contrib?
    >> >
    >> > I had an offlist chat with Andres about this IIRC and he suggested he
    >> > thought it ought to be just built in to the backend as part of the
    >> > statistics subsystem. Lately though I have been toying with the idea of
    >> > keeping it as an extension and basing it off Michael Paquier's work for
    >> > Pluggable cumulative statistics.
    >>
    >> This may live better as a contrib/ module, serving as well as an extra
    >> template for what can be done with the pluggable stats.  Adding that
    >> in core is of course OK for me if that's the consensus.  The APIs for
    >> pluggable stats are really the same as what you would store in core,
    >> minus the system functions you'd want to add in the catalog .dat
    >> files, of course.
    >>
    >> I'd like to get it this part done by the end of this commit fest to
    >> have room with pg_stat_statements for this release, but well, we'll
    >> see.  As far as I can see everybody who commented on the thread seems
    >> kind of OK with the idea to fix the stats kinds IDs in time, like
    >> custom RMGRs.  That's just simpler implementation-wise, but I'm also
    >> looking for more opinions.
    >>
    >> > Hmm, yeah, I had been planning to include postgres version as part of the
    >> > output, but maybe it would need to be part of the key.
    >>
    >> Seems to me that you should do both, then: add PG_VERSION to the
    >> entries, and hash the keys with it for uniqueness.  You could also
    >> have a reset function that performs a removal of the stats for
    >> anything else than the current PG_VERSION, for example.
    >> --
    >> Michael
    > 
    > 
    > This feature seems both interesting and useful, yet it hasn't
    > progressed for some time. Looking at the thread history, the patch
    > stalled around 2023 in part because the pluggable cumulative
    > statistics infrastructure was still under development, which offered
    > an opportunity to rework it on top of framework primitives rather than
    > carrying DIY implementations of shared hash tables, persistence, and
    > reset logic.
    > 
    > The refactoring opportunity is now basically available. The relevant
    > infrastructure landed in stages:
    > 
    > 7949d9594582 (Aug 2024) — kind registration, dshash routing, flush_pending_cb
    > c06e71d1aca (Nov 2024) — write_to_file flag, enabling opt-in persistence
    > 4ba012a8ed9 (Dec 2025) — to_serialized_data / from_serialized_data /
    > finish callbacks
    > 
    > I’m considering picking this up and 'd like to move it forward. Before
    > doing so, I am curious about whether there was a consensus on putting
    > it into core or keeping it as an extension, and whether there are any
    > major blockers I might have overlooked.
    
    
    It has not progressed mainly due to lack of time on my part. It is still 
    on my TODO list, and I would like to find time to complete it.
    
    Personally I would love to see it builtin to core, but an extension 
    would have the advantage of working with prior releases. In any case, if 
    it is in core, or even contrib, now is not the right time as it has zero 
    chance to get in before feature freeze. It would make more sense to 
    discuss this after April.
    
    That said, if you wanted to create an extension on github or whatever 
    you can of course determine your own timing.
    
    -- 
    Joe Conway
    PostgreSQL Contributors Team
    Amazon Web Services: https://aws.amazon.com
    
    
    
    

16. Re: RFC: pg_stat_logmsg

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-03-28T15:14:58Z

    On Sat, Mar 28, 2026 at 10:14 PM Joe Conway <mail@joeconway.com> wrote:
    >
    > On 3/24/26 04:05, Xuneng Zhou wrote:
    > > Hi,
    > >
    > > On Thu, Jul 18, 2024 at 12:33 PM Michael Paquier <michael@paquier.xyz> wrote:
    > >>
    > >> On Wed, Jul 17, 2024 at 07:43:13AM -0400, Joe Conway wrote:
    > >> > On 7/16/24 18:14, Tomas Vondra wrote:
    > >> >> As for the feature, I've never done a fleet-wide analysis, so if this is
    > >> >> one of the main use cases, I'm not really sure I can judge if this is a
    > >> >> good tool for that. It seems like it might be a convenient way to do
    > >> >> that, but does that require we add the module to contrib?
    > >> >
    > >> > I had an offlist chat with Andres about this IIRC and he suggested he
    > >> > thought it ought to be just built in to the backend as part of the
    > >> > statistics subsystem. Lately though I have been toying with the idea of
    > >> > keeping it as an extension and basing it off Michael Paquier's work for
    > >> > Pluggable cumulative statistics.
    > >>
    > >> This may live better as a contrib/ module, serving as well as an extra
    > >> template for what can be done with the pluggable stats.  Adding that
    > >> in core is of course OK for me if that's the consensus.  The APIs for
    > >> pluggable stats are really the same as what you would store in core,
    > >> minus the system functions you'd want to add in the catalog .dat
    > >> files, of course.
    > >>
    > >> I'd like to get it this part done by the end of this commit fest to
    > >> have room with pg_stat_statements for this release, but well, we'll
    > >> see.  As far as I can see everybody who commented on the thread seems
    > >> kind of OK with the idea to fix the stats kinds IDs in time, like
    > >> custom RMGRs.  That's just simpler implementation-wise, but I'm also
    > >> looking for more opinions.
    > >>
    > >> > Hmm, yeah, I had been planning to include postgres version as part of the
    > >> > output, but maybe it would need to be part of the key.
    > >>
    > >> Seems to me that you should do both, then: add PG_VERSION to the
    > >> entries, and hash the keys with it for uniqueness.  You could also
    > >> have a reset function that performs a removal of the stats for
    > >> anything else than the current PG_VERSION, for example.
    > >> --
    > >> Michael
    > >
    > >
    > > This feature seems both interesting and useful, yet it hasn't
    > > progressed for some time. Looking at the thread history, the patch
    > > stalled around 2023 in part because the pluggable cumulative
    > > statistics infrastructure was still under development, which offered
    > > an opportunity to rework it on top of framework primitives rather than
    > > carrying DIY implementations of shared hash tables, persistence, and
    > > reset logic.
    > >
    > > The refactoring opportunity is now basically available. The relevant
    > > infrastructure landed in stages:
    > >
    > > 7949d9594582 (Aug 2024) — kind registration, dshash routing, flush_pending_cb
    > > c06e71d1aca (Nov 2024) — write_to_file flag, enabling opt-in persistence
    > > 4ba012a8ed9 (Dec 2025) — to_serialized_data / from_serialized_data /
    > > finish callbacks
    > >
    > > I’m considering picking this up and 'd like to move it forward. Before
    > > doing so, I am curious about whether there was a consensus on putting
    > > it into core or keeping it as an extension, and whether there are any
    > > major blockers I might have overlooked.
    >
    >
    > It has not progressed mainly due to lack of time on my part. It is still
    > on my TODO list, and I would like to find time to complete it.
    
    That would be great!
    
    > Personally I would love to see it builtin to core, but an extension
    > would have the advantage of working with prior releases. In any case, if
    > it is in core, or even contrib, now is not the right time as it has zero
    > chance to get in before feature freeze. It would make more sense to
    > discuss this after April.
    
    Yeah, integrating this into core would make it universally accessible,
    while providing it as an extension would allow earlier versions to
    benefit.  Certainly, it should be considered for upcoming releases.
    
    > That said, if you wanted to create an extension on github or whatever
    > you can of course determine your own timing.
    >
    
    I'll try to create an extension on github. It would be fun to play
    with and help to learn the new cumulative statistics infra.
    
    -- 
    Best,
    Xuneng
    
    
    
    

17. Re: RFC: pg_stat_logmsg

    Fabrízio de Royes Mello <fabriziomello@gmail.com> — 2026-04-22T14:21:56Z

    On Sat, Mar 28, 2026 at 12:15 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    >
    > On Sat, Mar 28, 2026 at 10:14 PM Joe Conway <mail@joeconway.com> wrote:
    >
    > > It has not progressed mainly due to lack of time on my part. It is still
    > > on my TODO list, and I would like to find time to complete it.
    >
    > That would be great!
    >
    > > Personally I would love to see it builtin to core, but an extension
    > > would have the advantage of working with prior releases. In any case, if
    > > it is in core, or even contrib, now is not the right time as it has zero
    > > chance to get in before feature freeze. It would make more sense to
    > > discuss this after April.
    >
    > Yeah, integrating this into core would make it universally accessible,
    > while providing it as an extension would allow earlier versions to
    > benefit.  Certainly, it should be considered for upcoming releases.
    >
    > > That said, if you wanted to create an extension on github or whatever
    > > you can of course determine your own timing.
    > >
    >
    > I'll try to create an extension on github. It would be fun to play
    > with and help to learn the new cumulative statistics infra.
    >
    
    Here's a WIP extension: https://github.com/fabriziomello/pg_stat_log
    
    
    Regards,
    
    -- 
    Fabrízio de Royes Mello
    

18. Re: RFC: pg_stat_logmsg

    Xuneng Zhou <xunengzhou@gmail.com> — 2026-04-23T01:04:07Z

    Hi Fabrízio,
    
    On Wed, Apr 22, 2026 at 10:22 PM Fabrízio de Royes Mello
    <fabriziomello@gmail.com> wrote:
    >
    >
    > On Sat, Mar 28, 2026 at 12:15 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > >
    > > On Sat, Mar 28, 2026 at 10:14 PM Joe Conway <mail@joeconway.com> wrote:
    > >
    > > > It has not progressed mainly due to lack of time on my part. It is still
    > > > on my TODO list, and I would like to find time to complete it.
    > >
    > > That would be great!
    > >
    > > > Personally I would love to see it builtin to core, but an extension
    > > > would have the advantage of working with prior releases. In any case, if
    > > > it is in core, or even contrib, now is not the right time as it has zero
    > > > chance to get in before feature freeze. It would make more sense to
    > > > discuss this after April.
    > >
    > > Yeah, integrating this into core would make it universally accessible,
    > > while providing it as an extension would allow earlier versions to
    > > benefit.  Certainly, it should be considered for upcoming releases.
    > >
    > > > That said, if you wanted to create an extension on github or whatever
    > > > you can of course determine your own timing.
    > > >
    > >
    > > I'll try to create an extension on github. It would be fun to play
    > > with and help to learn the new cumulative statistics infra.
    > >
    >
    > Here's a WIP extension: https://github.com/fabriziomello/pg_stat_log
    
    I'll look into it.
    
    -- 
    Best,
    Xuneng