Re: backend type in log_line_prefix?
On 2020-04-01 03:55, Bruce Momjian wrote: Agreed. I ended up moving "wal" as a separate word, since it looks cleaner; patch attached. Tools that look for the backend type in pg_stat_activity would need to be adjusted; it would be an incompatibility. Maybe changing it would cause too much disruption. Yeah, it's probably not worth the change for that reason. There is no confusion what the "archiver" is. Also, we have archive_mode, archive_command, etc. without a wal_ prefix. Let's leave it as is. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On Fri, Mar 27, 2020 at 04:30:07PM +0900, Kyotaro Horiguchi wrote: > Hello. > > At Mon, 23 Mar 2020 18:38:53 -0400, Bruce Momjian wrote in > > Patch applied to master, thanks. > > The patch (8e8a0becb3) named archiver process as just "archiver". On > the other hand the discussion in the thread [1] was going to name the > process as "WAL/wal archiver". As all other processes related to WAL > are named as walreceiver, walsender, walwriter, wouldn't we name the > process like "wal archiver"? > > [1]: > https://www.postgresql.org/message-id/20200319195410.icib45bbgjwqb...@alap3.anarazel.de Agreed. I ended up moving "wal" as a separate word, since it looks cleaner; patch attached. Tools that look for the backend type in pg_stat_activity would need to be adjusted; it would be an incompatibility. Maybe changing it would cause too much disruption. -- Bruce Momjian https://momjian.us EnterpriseDB https://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index a7b7b12249..2d625ee01e 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -221,16 +221,16 @@ GetBackendTypeDesc(BackendType backendType) backendDesc = "startup"; break; case B_WAL_RECEIVER: - backendDesc = "walreceiver"; + backendDesc = "wal receiver"; break; case B_WAL_SENDER: - backendDesc = "walsender"; + backendDesc = "wal sender"; break; case B_WAL_WRITER: - backendDesc = "walwriter"; + backendDesc = "wal writer"; break; case B_ARCHIVER: - backendDesc = "archiver"; + backendDesc = "wal archiver"; break; case B_STATS_COLLECTOR: backendDesc = "stats collector";
Re: backend type in log_line_prefix?
Hello. At Mon, 23 Mar 2020 18:38:53 -0400, Bruce Momjian wrote in > Patch applied to master, thanks. The patch (8e8a0becb3) named archiver process as just "archiver". On the other hand the discussion in the thread [1] was going to name the process as "WAL/wal archiver". As all other processes related to WAL are named as walreceiver, walsender, walwriter, wouldn't we name the process like "wal archiver"? [1]: https://www.postgresql.org/message-id/20200319195410.icib45bbgjwqb...@alap3.anarazel.de regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: backend type in log_line_prefix?
On Thu, Mar 19, 2020 at 01:37:17PM -0300, Fabrízio de Royes Mello wrote: > > On Sun, Mar 15, 2020 at 7:32 AM Peter Eisentraut < > peter.eisentr...@2ndquadrant.com> wrote: > > > > > > I have committed that last one also, after some corrections. > > > > IMHO we should also update file_fdw documentation. See attached! > > Regards, > > -- > Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/ > PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento > diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml > index 28b61c8f2d..ed028e4ec9 100644 > --- a/doc/src/sgml/file-fdw.sgml > +++ b/doc/src/sgml/file-fdw.sgml > @@ -261,7 +261,8 @@ CREATE FOREIGN TABLE pglog ( >query text, >query_pos integer, >location text, > - application_name text > + application_name text, > + backend_type text > ) SERVER pglog > OPTIONS ( filename '/home/josh/data/log/pglog.csv', format 'csv' ); > Patch applied to master, thanks. -- Bruce Momjian https://momjian.us EnterpriseDB https://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: backend type in log_line_prefix?
On Sun, Mar 15, 2020 at 7:32 AM Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > > > I have committed that last one also, after some corrections. > IMHO we should also update file_fdw documentation. See attached! Regards, -- Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index 28b61c8f2d..ed028e4ec9 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -261,7 +261,8 @@ CREATE FOREIGN TABLE pglog ( query text, query_pos integer, location text, - application_name text + application_name text, + backend_type text ) SERVER pglog OPTIONS ( filename '/home/josh/data/log/pglog.csv', format 'csv' );
Re: backend type in log_line_prefix?
> On 2020/03/15 19:32, Peter Eisentraut wrote: > > On 2020-03-13 22:24, Peter Eisentraut wrote: > >> On 2020-03-10 19:07, Alvaro Herrera wrote: > >>> I like these patches; the first two are nice cleanup. > >>> > >>> My only gripe is that pgstat_get_backend_desc() is not really a pgstat > >>> function; I think it should have a different name with a prototype in > >>> miscadmin.h (next to the enum's new location, which I would put > >>> someplace near the "pmod.h" comment rather than where you put it; > >>> perhaps just above the AuxProcType definition), and implementation > >>> probably in miscinit.c. > >> > >> I have committed the refactoring patches with adjustments along these > >> lines. The patch with the log_line_prefix and csvlog enhancements is > >> still under discussion. > > > > I have committed that last one also, after some corrections. Sorry for being late to this thread, but was wondering if anyone had taken a look at the Process Centralization patchset that I submitted to this CF: https://www.postgresql.org/message-id/CAMN686HgTVRJBAw6hqFE4Lj8bgPLQqfp1c-%2BWBGUtEmg6wPVhg%40mail.gmail.com There's quite a bit of that code that is in the same vein as the MyBackendType changes proposed/merged in this thread. I think we could reduce a large portion of redundant code (including the pgstat_get_backend_desc code) while also centralizing/standardizing process startup. A few useful features (outside of code reduction) include the ability to identify backends prior to their Main functions, cleaner control of SubPostmasterMain logic (including implicit handling of shmem timing considerations). If others think it's worthwhile, I will work on rebasing those changes on the changes proposed/merged in this thread (re: MyBackendType). Thanks, -- Mike Palmiotto https://crunchydata.com
Re: backend type in log_line_prefix?
On 2020/03/15 19:32, Peter Eisentraut wrote: On 2020-03-13 22:24, Peter Eisentraut wrote: On 2020-03-10 19:07, Alvaro Herrera wrote: I like these patches; the first two are nice cleanup. My only gripe is that pgstat_get_backend_desc() is not really a pgstat function; I think it should have a different name with a prototype in miscadmin.h (next to the enum's new location, which I would put someplace near the "pmod.h" comment rather than where you put it; perhaps just above the AuxProcType definition), and implementation probably in miscinit.c. I have committed the refactoring patches with adjustments along these lines. The patch with the log_line_prefix and csvlog enhancements is still under discussion. I have committed that last one also, after some corrections. Thanks for adding this nice feature! I have one comment; You seem to need to update file-fdw.sgml so that pglog table in the doc should include backend_type column. Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters
Re: backend type in log_line_prefix?
On 2020-03-15 10:57, Justin Pryzby wrote: I suggest the CSV/log should also have the leader_pid, corresponding to | b025f32e0b Add leader_pid to pg_stat_activity I haven't followed those developments. It sounds interesting, but I suggest you start a new thread or continue in the thread that added leader_pid. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On 2020-03-13 22:24, Peter Eisentraut wrote: On 2020-03-10 19:07, Alvaro Herrera wrote: I like these patches; the first two are nice cleanup. My only gripe is that pgstat_get_backend_desc() is not really a pgstat function; I think it should have a different name with a prototype in miscadmin.h (next to the enum's new location, which I would put someplace near the "pmod.h" comment rather than where you put it; perhaps just above the AuxProcType definition), and implementation probably in miscinit.c. I have committed the refactoring patches with adjustments along these lines. The patch with the log_line_prefix and csvlog enhancements is still under discussion. I have committed that last one also, after some corrections. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On Fri, Mar 13, 2020 at 10:22:52PM +0100, Peter Eisentraut wrote: > >Can I suggest: > > > >- appendCSVLiteral(, MyBgworkerEntry->bgw_type); > >+ appendCSVLiteral(, MyBgworkerEntry->bgw_name); > > The difference is intentional. bgw_type is so that you can filter and group > by type. The bgw_name could be totally different for each instance. I found 5373bc2a0867048bb78f93aede54ac1309b5e227 Your patch adds bgw_type, which is also in pg_stat_activity, so I agree it's good to allow include it log_line_prefix and CSV. I suggest the CSV/log should also have the leader_pid, corresponding to | b025f32e0b Add leader_pid to pg_stat_activity With the attached on top of your patch, CSV logs like: 2020-03-14 22:09:39.395 CDT,"pryzbyj","template1",17030,"[local]",5e6d9c69.4286,2,"idle",2020-03-14 22:09:29 CDT,3/23,0,LOG,0,"statement: explain analyze SELECT COUNT(1), a.a FROM t a JOIN t b ON a.a=b.a GROUP BY 2;","psql","client backend", 2020-03-14 22:09:43.094 CDT,,,17042,,5e6d9c73.4292,1,,2020-03-14 22:09:39 CDT,4/3,0,LOG,0,"temporary file: path ""base/pgsql_tmp/pgsql_tmp17042.0"", size 4694016",,"explain analyze SELECT COUNT(1), a.a FROM t a JOIN t b ON a.a=b.a GROUP BY 2;",,,"psql","parallel worker",17030 2020-03-14 22:09:43.094 CDT,,,17043,,5e6d9c73.4293,1,,2020-03-14 22:09:39 CDT,5/3,0,LOG,0,"temporary file: path ""base/pgsql_tmp/pgsql_tmp17043.0"", size 4694016",,"explain analyze SELECT COUNT(1), a.a FROM t a JOIN t b ON a.a=b.a GROUP BY 2;",,,"psql","parallel worker",17030 As for my question "what's using/trying/failing to use parallel workers", I was able to look into that by parsing "Workers Planned/Launched" from autoexplain. It's not a *good* way to do it, but I don't see how to do better and I don't see any way this patch can improve that. -- Justin >From 5b111a3a6cd5282dbdf1e504ef648994b1918302 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 14 Mar 2020 19:09:54 -0500 Subject: [PATCH v1 1/2] Fix previous commit.. --- doc/src/sgml/config.sgml | 2 +- src/backend/utils/error/elog.c | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 53d0c480aa..fc0e2c00c3 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6460,7 +6460,7 @@ local0.*/var/log/postgresql %b Backend process type - yes + no %u diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 3a6f7f9456..62eef7b71f 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -72,7 +72,6 @@ #include "libpq/pqformat.h" #include "mb/pg_wchar.h" #include "miscadmin.h" -#include "pgstat.h" #include "postmaster/bgworker.h" #include "postmaster/postmaster.h" #include "postmaster/syslogger.h" @@ -2503,7 +2502,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata) else if (MyBackendType == B_BG_WORKER) backend_type_str = MyBgworkerEntry->bgw_type; else - backend_type_str = pgstat_get_backend_desc(MyBackendType); + backend_type_str = GetBackendTypeDesc(MyBackendType); if (padding != 0) appendStringInfo(buf, "%*s", padding, backend_type_str); @@ -2947,7 +2946,7 @@ write_csvlog(ErrorData *edata) else if (MyBackendType == B_BG_WORKER) appendCSVLiteral(, MyBgworkerEntry->bgw_type); else - appendCSVLiteral(, pgstat_get_backend_desc(MyBackendType)); + appendCSVLiteral(, GetBackendTypeDesc(MyBackendType)); appendStringInfoChar(, '\n'); -- 2.17.0 >From 30172a4771360fd6ed4f4646651efffa3785dfb7 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 13 Mar 2020 22:03:06 -0500 Subject: [PATCH v1 2/2] Include the leader PID in logfile See also: b025f32e0b, which adds the leader PID to pg_stat_activity --- doc/src/sgml/config.sgml | 8 +++- src/backend/utils/error/elog.c| 47 +++ src/backend/utils/misc/postgresql.conf.sample | 1 + 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fc0e2c00c3..d53b62e2df 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6487,6 +6487,11 @@ local0.*/var/log/postgresql Process ID no + + %k + Leader PID for a parallel process, NULL otherwise + yes + %t Time stamp without milliseconds @@ -6777,7 +6782,7 @@ log_line_prefix = '%m [%p] %q%u@%d/%a ' character count of the error position therein, location of the error in the PostgreSQL source code (if log_error_verbosity is set to verbose), -application name, and backend type. +application name, backend type, and leader PID. Here is a sample table
Re: backend type in log_line_prefix?
On Fri, Feb 21, 2020 at 10:09:38AM +0100, Peter Eisentraut wrote: > From 75ac8ed0c47801712eb2aa300d9cb29767d2e121 Mon Sep 17 00:00:00 2001 > From: Peter Eisentraut > Date: Thu, 20 Feb 2020 18:16:39 +0100 > Subject: [PATCH v2 3/4] Add backend type to csvlog and optionally > log_line_prefix > diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml > index c1128f89ec..206778b1c3 100644 > --- a/doc/src/sgml/config.sgml > +++ b/doc/src/sgml/config.sgml > @@ -6470,6 +6470,11 @@ What to Log characters are copied straight to the log line. Some escapes are only recognized by session processes, and will be treated as empty by background processes such as the main server process. Status ... Escape Effect Session only > Application name > yes > > + > + %b > + Backend process type > + yes => should say "no", it's not blank for background processes: > + > + if (MyProcPid == PostmasterPid) > + backend_type_str = "postmaster"; > + else if (MyBackendType == B_BG_WORKER) > + backend_type_str = > MyBgworkerEntry->bgw_type; > + else > + backend_type_str = > pgstat_get_backend_desc(MyBackendType); -- Justin
Re: backend type in log_line_prefix?
On 2020-03-10 19:07, Alvaro Herrera wrote: I like these patches; the first two are nice cleanup. My only gripe is that pgstat_get_backend_desc() is not really a pgstat function; I think it should have a different name with a prototype in miscadmin.h (next to the enum's new location, which I would put someplace near the "pmod.h" comment rather than where you put it; perhaps just above the AuxProcType definition), and implementation probably in miscinit.c. I have committed the refactoring patches with adjustments along these lines. The patch with the log_line_prefix and csvlog enhancements is still under discussion. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On 2020-03-11 19:53, Justin Pryzby wrote: Can I suggest: $ git diff diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 3a6f7f9456..56e0a1437e 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2945,7 +2945,7 @@ write_csvlog(ErrorData *edata) if (MyProcPid == PostmasterPid) appendCSVLiteral(, "postmaster"); else if (MyBackendType == B_BG_WORKER) - appendCSVLiteral(, MyBgworkerEntry->bgw_type); + appendCSVLiteral(, MyBgworkerEntry->bgw_name); else appendCSVLiteral(, pgstat_get_backend_desc(MyBackendType)); The difference is intentional. bgw_type is so that you can filter and group by type. The bgw_name could be totally different for each instance. Having the bgw name available somehow would perhaps also be useful, but then we should also do this in a consistent way for processes that are not background workers, such as regular client backends or wal senders or autovacuum workers. Doing it just for background workers would create inconsistencies that the introduction of bgw_type some time ago sought to eliminate. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On Tue, Mar 10, 2020 at 02:01:42PM -0500, Justin Pryzby wrote: > On Thu, Feb 13, 2020 at 06:43:32PM +0900, Fujii Masao wrote: > > If we do this, backend type should be also included in csvlog? > > +1, I've been missing that > > Note, this patch seems to correspond to: > b025f32e0b Add leader_pid to pg_stat_activity > > I had mentioned privately to Julien missing this info in CSV log. > > Should leader_pid be exposed instead (or in addition)? Or backend_type be a I looked more closely and played with the patch. Can I suggest: $ git diff diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 3a6f7f9456..56e0a1437e 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2945,7 +2945,7 @@ write_csvlog(ErrorData *edata) if (MyProcPid == PostmasterPid) appendCSVLiteral(, "postmaster"); else if (MyBackendType == B_BG_WORKER) - appendCSVLiteral(, MyBgworkerEntry->bgw_type); + appendCSVLiteral(, MyBgworkerEntry->bgw_name); else appendCSVLiteral(, pgstat_get_backend_desc(MyBackendType)); Then it logs the leader: |2020-03-11 13:16:05.596 CDT,,,16289,,5e692ae3.3fa1,1,,2020-03-11 13:16:03 CDT,4/3,0,LOG,0,"temporary file: path ""base/pgsql_tmp/pgsql_tmp16289.0"", size 4276224",,"explain analyze SELECT * FROM t a JOIN t b USING(i) WHERE i>999 GROUP BY 1;",,,"psql","parallel worker for PID 16210" It'll be easy enough to extract the leader and join that ON leader=pid. > I think this patch alone wouldn't provide that, and there'd need to either be > a > line logged for each worker. Maybe it'd log full query+details (ugh), or just > log "parallel worker of pid...". Or maybe there'd be a new column with which > the leader would log nworkers (workers planned vs workers launched - I would > *not* want to get this out of autoexplain). I'm still not sure how to do that, though. I see I can get what's needed at DEBUG1: |2020-03-11 13:50:58.304 CDT,,,16196,,5e692aa7.3f44,22,,2020-03-11 13:15:03 CDT,,0,DEBUG,0,"registering background worker ""parallel worker for PID 16210""","","postmaster" But I don't think it's viable to run for very long with log_statement=all, log_min_messages=DEBUG. -- Justin
Re: backend type in log_line_prefix?
On Thu, Feb 13, 2020 at 06:43:32PM +0900, Fujii Masao wrote: > If we do this, backend type should be also included in csvlog? +1, I've been missing that Note, this patch seems to correspond to: b025f32e0b Add leader_pid to pg_stat_activity I had mentioned privately to Julien missing this info in CSV log. Should leader_pid be exposed instead (or in addition)? Or backend_type be a positive number giving the leader's PID if it's a parallel worker, or a some special negative number like -BackendType to indicate a nonparallel worker. NULL for a B_BACKEND which is not a parallel worker. My hope is to answer to questions like these: . is query (ever? usually?) using parallel paths? . is query usefully using parallel paths? . what queries are my max_parallel_workers(_per_process) being used for ? . Are certain longrunning or frequently running queries which are using parallel paths using all max_parallel_workers and precluding other queries from using parallel query ? Or, are semi-short queries sometimes precluding longrunning queries from using parallelism, when the long queries would better benefit ? I think this patch alone wouldn't provide that, and there'd need to either be a line logged for each worker. Maybe it'd log full query+details (ugh), or just log "parallel worker of pid...". Or maybe there'd be a new column with which the leader would log nworkers (workers planned vs workers launched - I would *not* want to get this out of autoexplain). -- Justin
Re: backend type in log_line_prefix?
I like these patches; the first two are nice cleanup. My only gripe is that pgstat_get_backend_desc() is not really a pgstat function; I think it should have a different name with a prototype in miscadmin.h (next to the enum's new location, which I would put someplace near the "pmod.h" comment rather than where you put it; perhaps just above the AuxProcType definition), and implementation probably in miscinit.c. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
On Tue, Mar 10, 2020 at 9:11 PM Peter Eisentraut wrote: > On 2020-03-09 16:20, Kuntal Ghosh wrote: > > In v3-0001-Refactor-ps_status.c-API.patch, > > - * postgres: walsender > > This part is still valid, right? > Sure but I figured this comment was in the context of the explanation of > how the old API was being abused, so it's no longer necessary. > Makes sense. > set_ps_display() checks update_process_title itself and does nothing if > it's off, so this should work okay. Right. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: backend type in log_line_prefix?
On Tue, Mar 10, 2020 at 4:41 PM Peter Eisentraut wrote: > > On 2020-03-09 16:20, Kuntal Ghosh wrote: > > In v3-0002-Unify-several-ways-to-tracking-backend-type.patch, In pgstat_get_backend_desc(), the fallback "unknown process type" description shouldn't be required anymore. Other than that, it all looks good to me.
Re: backend type in log_line_prefix?
On 2020-03-09 16:20, Kuntal Ghosh wrote:
In v3-0001-Refactor-ps_status.c-API.patch,
- *
- * For a walsender, the ps display is set in the following form:
- *
- * postgres: walsender
This part is still valid, right?
Sure but I figured this comment was in the context of the explanation of
how the old API was being abused, so it's no longer necessary.
+ init_ps_display(ps_data.data);
+ pfree(ps_data.data);
+
+ set_ps_display("initializing");
As per the existing behaviour, if update_process_title is true, we
display "authentication" as the initial string. On the other hand,
this patch, irrespective of the GUC variable, always displays
"initializing" as the initial string and In PerformAuthentication, it
sets the display as "authentication" Is this intended? Should we check
the GUC here as well?
set_ps_display() checks update_process_title itself and does nothing if
it's off, so this should work okay.
In v3-0002-Unify-several-ways-to-tracking-backend-type.patch,
+ * If fixed_part is NULL, a default will be obtained from BackendType.
s/BackendType/MyBackendType?
yup
In v3-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patch,
+ Backend process type
In other places you've written "backend type".
ok changed
In v3-0004-Remove-am_syslogger-global-variable.patch,
+ * This is exported so that elog.c can call it when BackendType is B_LOGGER.
s/BackendType/MyBackendType?
ok
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: backend type in log_line_prefix?
Hello,
On Sat, Mar 7, 2020 at 8:38 PM Peter Eisentraut
wrote:
>
> Updated patch set because of conflicts.
>
Thank you for the patch. This feature is really helpful. Here are some
minor comments:
In v3-0001-Refactor-ps_status.c-API.patch,
- *
- * For a walsender, the ps display is set in the following form:
- *
- * postgres: walsender
This part is still valid, right?
+ init_ps_display(ps_data.data);
+ pfree(ps_data.data);
+
+ set_ps_display("initializing");
As per the existing behaviour, if update_process_title is true, we
display "authentication" as the initial string. On the other hand,
this patch, irrespective of the GUC variable, always displays
"initializing" as the initial string and In PerformAuthentication, it
sets the display as "authentication" Is this intended? Should we check
the GUC here as well?
In v3-0002-Unify-several-ways-to-tracking-backend-type.patch,
+ * If fixed_part is NULL, a default will be obtained from BackendType.
s/BackendType/MyBackendType?
In v3-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patch,
+ Backend process type
In other places you've written "backend type".
In v3-0004-Remove-am_syslogger-global-variable.patch,
+ * This is exported so that elog.c can call it when BackendType is B_LOGGER.
s/BackendType/MyBackendType?
Done some basic testing. Working as expected.
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
Re: backend type in log_line_prefix?
Updated patch set because of conflicts.
On 2020-02-21 10:09, Peter Eisentraut wrote:
After positive initial feedback, here is a more ambitious patch set. In
particular, I wanted to avoid having to specify the backend type (at
least) twice, once for the ps display and once for this new facility.
I have added a new global variable MyBackendType that uses the existing
BackendType enum that was previously only used by the stats collector.
Then the ps display, the stats collector, the log_line_prefix, and other
places can just refer to this to know "who am I". (There are more
places like that, for example in the autovacuum system, so patch 0004 in
particular could be expanded in analogous ways.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From bf28870a342871400b6fbf07bbb5eaa71e23d3bd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Thu, 20 Feb 2020 18:07:37 +0100
Subject: [PATCH v3 1/4] Refactor ps_status.c API
The init_ps_display() arguments were mostly lies by now, so to match
typical usage, just use one argument and let the caller assemble it
from multiple sources if necessary. The only user of the additional
arguments is BackendInitialize(), which was already doing string
assembly on the caller side anyway.
Remove the second argument of set_ps_display() ("force") and just
handle that in init_ps_display() internally.
BackendInitialize() also used to set the initial status as
"authentication", but that was very far from where authentication
actually happened. So now it's set to "initializing" and then
"authentication" just before the actual call to
ClientAuthentication().
---
src/backend/access/transam/xlog.c | 4 ++--
src/backend/bootstrap/bootstrap.c | 2 +-
src/backend/commands/async.c | 4 ++--
src/backend/postmaster/autovacuum.c | 6 ++---
src/backend/postmaster/bgworker.c | 2 +-
src/backend/postmaster/pgarch.c | 8 +++
src/backend/postmaster/pgstat.c | 2 +-
src/backend/postmaster/postmaster.c | 32 ---
src/backend/postmaster/syslogger.c| 2 +-
src/backend/replication/basebackup.c | 2 +-
src/backend/replication/syncrep.c | 4 ++--
src/backend/replication/walreceiver.c | 7 +++---
src/backend/replication/walsender.c | 2 +-
src/backend/storage/ipc/standby.c | 4 ++--
src/backend/storage/lmgr/lock.c | 6 ++---
src/backend/tcop/postgres.c | 16 +++---
src/backend/utils/init/postinit.c | 3 ++-
src/backend/utils/misc/ps_status.c| 31 +++---
src/include/utils/ps_status.h | 5 ++---
19 files changed, 71 insertions(+), 71 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 4361568882..182e65b2df 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3648,7 +3648,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
/* Report recovery progress in PS display */
snprintf(activitymsg, sizeof(activitymsg), "waiting for
%s",
xlogfname);
- set_ps_display(activitymsg, false);
+ set_ps_display(activitymsg);
restoredFromArchive = RestoreArchivedFile(path,
xlogfname,
"RECOVERYXLOG",
@@ -3691,7 +3691,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
/* Report recovery progress in PS display */
snprintf(activitymsg, sizeof(activitymsg), "recovering %s",
xlogfname);
- set_ps_display(activitymsg, false);
+ set_ps_display(activitymsg);
/* Track source of data in assorted state variables */
readSource = source;
diff --git a/src/backend/bootstrap/bootstrap.c
b/src/backend/bootstrap/bootstrap.c
index 657b18ecc8..7923d1ec9b 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -342,7 +342,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
statmsg = "??? process";
break;
}
- init_ps_display(statmsg, "", "", "");
+ init_ps_display(statmsg);
}
/* Acquire configuration parameters, unless inherited from postmaster */
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index dae939a4ab..0c9d20ebfc 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -2225,7 +2225,7 @@ ProcessIncomingNotify(void)
if (Trace_notify)
elog(DEBUG1, "ProcessIncomingNotify");
- set_ps_display("notify interrupt", false);
+ set_ps_display("notify interrupt");
Re: backend type in log_line_prefix?
On 2020-02-13 09:56, Peter Eisentraut wrote:
Attached is a demo patch that adds a placeholder %b for log_line_prefix
(not in the default setting) that contains the backend type, the same
that you see in pg_stat_activity and in the ps status. I would have
found this occasionally useful when analyzing logs, especially if you
have a lot of background workers active. Thoughts?
After positive initial feedback, here is a more ambitious patch set. In
particular, I wanted to avoid having to specify the backend type (at
least) twice, once for the ps display and once for this new facility.
I have added a new global variable MyBackendType that uses the existing
BackendType enum that was previously only used by the stats collector.
Then the ps display, the stats collector, the log_line_prefix, and other
places can just refer to this to know "who am I". (There are more
places like that, for example in the autovacuum system, so patch 0004 in
particular could be expanded in analogous ways.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 29f9c92c5cdc7eddb60f69bb984a57e45a600938 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Thu, 20 Feb 2020 18:07:37 +0100
Subject: [PATCH v2 1/4] Refactor ps_status.c API
The init_ps_display() arguments were mostly lies by now, so to match
typical usage, just use one argument and let the caller assemble it
from multiple sources if necessary. The only user of the additional
arguments is BackendInitialize(), which was already doing string
assembly on the caller side anyway.
Remove the second argument of set_ps_display() ("force") and just
handle that in init_ps_display() internally.
BackendInitialize() also used to set the initial status as
"authentication", but that was very far from where authentication
actually happened. So now it's set to "initializing" and then
"authentication" just before the actual call to
ClientAuthentication().
---
src/backend/access/transam/xlog.c | 4 ++--
src/backend/bootstrap/bootstrap.c | 2 +-
src/backend/commands/async.c | 4 ++--
src/backend/postmaster/autovacuum.c | 6 ++---
src/backend/postmaster/bgworker.c | 2 +-
src/backend/postmaster/pgarch.c | 8 +++
src/backend/postmaster/pgstat.c | 2 +-
src/backend/postmaster/postmaster.c | 32 ---
src/backend/postmaster/syslogger.c| 2 +-
src/backend/replication/basebackup.c | 2 +-
src/backend/replication/syncrep.c | 4 ++--
src/backend/replication/walreceiver.c | 7 +++---
src/backend/replication/walsender.c | 2 +-
src/backend/storage/ipc/standby.c | 4 ++--
src/backend/storage/lmgr/lock.c | 6 ++---
src/backend/tcop/postgres.c | 16 +++---
src/backend/utils/init/postinit.c | 3 ++-
src/backend/utils/misc/ps_status.c| 31 +++---
src/include/utils/ps_status.h | 5 ++---
19 files changed, 71 insertions(+), 71 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 3813eadfb4..b4455799c4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3639,7 +3639,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
/* Report recovery progress in PS display */
snprintf(activitymsg, sizeof(activitymsg), "waiting for
%s",
xlogfname);
- set_ps_display(activitymsg, false);
+ set_ps_display(activitymsg);
restoredFromArchive = RestoreArchivedFile(path,
xlogfname,
"RECOVERYXLOG",
@@ -3682,7 +3682,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
/* Report recovery progress in PS display */
snprintf(activitymsg, sizeof(activitymsg), "recovering %s",
xlogfname);
- set_ps_display(activitymsg, false);
+ set_ps_display(activitymsg);
/* Track source of data in assorted state variables */
readSource = source;
diff --git a/src/backend/bootstrap/bootstrap.c
b/src/backend/bootstrap/bootstrap.c
index bfc629c753..4212333737 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -342,7 +342,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
statmsg = "??? process";
break;
}
- init_ps_display(statmsg, "", "", "");
+ init_ps_display(statmsg);
}
/* Acquire configuration parameters, unless inherited from postmaster */
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 9aa2b61600..50275f94ff 100644
Re: backend type in log_line_prefix?
Hi, On 2020-02-13 09:56:38 +0100, Peter Eisentraut wrote: > Attached is a demo patch that adds a placeholder %b for log_line_prefix (not > in the default setting) that contains the backend type, the same that you > see in pg_stat_activity and in the ps status. I would have found this > occasionally useful when analyzing logs, especially if you have a lot of > background workers active. Thoughts? I wished for this several times. > @@ -342,7 +342,7 @@ AuxiliaryProcessMain(int argc, char *argv[]) > statmsg = "??? process"; > break; > } > - init_ps_display(statmsg, "", "", ""); > + init_ps_display((backend_type_str = statmsg), "", "", ""); > } But I'm decidedly not a fan of this. Greetings, Andres Freund
Re: backend type in log_line_prefix?
On 2020/02/13 18:14, Julien Rouhaud wrote: On Thu, Feb 13, 2020 at 09:56:38AM +0100, Peter Eisentraut wrote: Attached is a demo patch that adds a placeholder %b for log_line_prefix (not in the default setting) that contains the backend type, the same that you see in pg_stat_activity and in the ps status. I would have found this occasionally useful when analyzing logs, especially if you have a lot of background workers active. Thoughts? If we do this, backend type should be also included in csvlog? Regarding the patch, postgresql.conf.sample needs to be updated. Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters
Re: backend type in log_line_prefix?
On Thu, Feb 13, 2020 at 09:56:38AM +0100, Peter Eisentraut wrote: > Attached is a demo patch that adds a placeholder %b for log_line_prefix (not > in the default setting) that contains the backend type, the same that you > see in pg_stat_activity and in the ps status. I would have found this > occasionally useful when analyzing logs, especially if you have a lot of > background workers active. Thoughts? +1, I'd also have been happy to have it multiple times.
backend type in log_line_prefix?
Attached is a demo patch that adds a placeholder %b for log_line_prefix
(not in the default setting) that contains the backend type, the same
that you see in pg_stat_activity and in the ps status. I would have
found this occasionally useful when analyzing logs, especially if you
have a lot of background workers active. Thoughts?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 3cb15882f671c8d37cb0f844ce62d1853870875c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Wed, 12 Feb 2020 22:04:29 +0100
Subject: [PATCH] Add backend type placeholder to log_line_prefix
---
doc/src/sgml/config.sgml| 5 +
src/backend/bootstrap/bootstrap.c | 2 +-
src/backend/postmaster/autovacuum.c | 4 ++--
src/backend/postmaster/bgworker.c | 2 +-
src/backend/postmaster/pgarch.c | 2 +-
src/backend/postmaster/pgstat.c | 2 +-
src/backend/postmaster/postmaster.c | 9 -
src/backend/postmaster/syslogger.c | 2 +-
src/backend/utils/error/elog.c | 6 ++
src/backend/utils/init/globals.c| 2 ++
src/include/miscadmin.h | 2 ++
src/test/regress/pg_regress.c | 2 +-
12 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c1128f89ec..9c068f3903 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6470,6 +6470,11 @@ What to Log
Application name
yes
+
+ %b
+ Backend process type
+ yes
+
%u
User name
diff --git a/src/backend/bootstrap/bootstrap.c
b/src/backend/bootstrap/bootstrap.c
index bfc629c753..1c559851b4 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -342,7 +342,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
statmsg = "??? process";
break;
}
- init_ps_display(statmsg, "", "", "");
+ init_ps_display((backend_type_str = statmsg), "", "", "");
}
/* Acquire configuration parameters, unless inherited from postmaster */
diff --git a/src/backend/postmaster/autovacuum.c
b/src/backend/postmaster/autovacuum.c
index 6d1f28c327..d50d4b9b02 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -434,7 +434,7 @@ AutoVacLauncherMain(int argc, char *argv[])
am_autovacuum_launcher = true;
/* Identify myself via ps */
- init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_LAUNCHER), "", "",
"");
+ init_ps_display((backend_type_str =
pgstat_get_backend_desc(B_AUTOVAC_LAUNCHER)), "", "", "");
ereport(DEBUG1,
(errmsg("autovacuum launcher started")));
@@ -1507,7 +1507,7 @@ AutoVacWorkerMain(int argc, char *argv[])
am_autovacuum_worker = true;
/* Identify myself via ps */
- init_ps_display(pgstat_get_backend_desc(B_AUTOVAC_WORKER), "", "", "");
+ init_ps_display((backend_type_str =
pgstat_get_backend_desc(B_AUTOVAC_WORKER)), "", "", "");
SetProcessingMode(InitProcessing);
diff --git a/src/backend/postmaster/bgworker.c
b/src/backend/postmaster/bgworker.c
index 75fc0d5d33..105b755c50 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -689,7 +689,7 @@ StartBackgroundWorker(void)
IsBackgroundWorker = true;
/* Identify myself via ps */
- init_ps_display(worker->bgw_name, "", "", "");
+ init_ps_display((backend_type_str = worker->bgw_name), "", "", "");
/*
* If we're not supposed to have shared memory access, then detach from
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 3ca30badb2..cd4f8b6392 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -241,7 +241,7 @@ PgArchiverMain(int argc, char *argv[])
/*
* Identify myself via ps
*/
- init_ps_display("archiver", "", "", "");
+ init_ps_display((backend_type_str = "archiver"), "", "", "");
pgarch_MainLoop();
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 7169509a79..8d48abfe1e 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -4447,7 +4447,7 @@ PgstatCollectorMain(int argc, char *argv[])
/*
* Identify myself via ps
*/
- init_ps_display("stats collecto
