backend type in log_line_prefix?

Started by Peter Eisentrautalmost 6 years ago26 messages
#1Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

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

Attachments:

0001-Add-backend-type-placeholder-to-log_line_prefix.patchtext/plain; charset=UTF-8; name=0001-Add-backend-type-placeholder-to-log_line_prefix.patch; x-mac-creator=0; x-mac-type=0
#2Julien Rouhaud
Julien Rouhaud
rjuju123@gmail.com
In reply to: Peter Eisentraut (#1)
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.

#3Fujii Masao
Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: Julien Rouhaud (#2)
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

#4Andres Freund
Andres Freund
andres@anarazel.de
In reply to: Peter Eisentraut (#1)
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

#5Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
4 attachment(s)
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

Attachments:

v2-0001-Refactor-ps_status.c-API.patchtext/plain; charset=UTF-8; name=v2-0001-Refactor-ps_status.c-API.patch; x-mac-creator=0; x-mac-type=0
v2-0002-Unify-several-ways-to-tracking-backend-type.patchtext/plain; charset=UTF-8; name=v2-0002-Unify-several-ways-to-tracking-backend-type.patch; x-mac-creator=0; x-mac-type=0
v2-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patchtext/plain; charset=UTF-8; name=v2-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patch; x-mac-creator=0; x-mac-type=0
v2-0004-Remove-am_syslogger-global-variable.patchtext/plain; charset=UTF-8; name=v2-0004-Remove-am_syslogger-global-variable.patch; x-mac-creator=0; x-mac-type=0
#6Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Peter Eisentraut (#5)
4 attachment(s)
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

Attachments:

v3-0001-Refactor-ps_status.c-API.patchtext/plain; charset=UTF-8; name=v3-0001-Refactor-ps_status.c-API.patch; x-mac-creator=0; x-mac-type=0
v3-0002-Unify-several-ways-to-tracking-backend-type.patchtext/plain; charset=UTF-8; name=v3-0002-Unify-several-ways-to-tracking-backend-type.patch; x-mac-creator=0; x-mac-type=0
v3-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patchtext/plain; charset=UTF-8; name=v3-0003-Add-backend-type-to-csvlog-and-optionally-log_lin.patch; x-mac-creator=0; x-mac-type=0
v3-0004-Remove-am_syslogger-global-variable.patchtext/plain; charset=UTF-8; name=v3-0004-Remove-am_syslogger-global-variable.patch; x-mac-creator=0; x-mac-type=0
#7Kuntal Ghosh
Kuntal Ghosh
kuntalghosh.2007@gmail.com
In reply to: Peter Eisentraut (#6)
Re: backend type in log_line_prefix?

Hello,

On Sat, Mar 7, 2020 at 8:38 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> 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 <user> <host> <activity>
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,
+ <entry>Backend process type</entry>
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

#8Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Kuntal Ghosh (#7)
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 <user> <host> <activity>
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,
+ <entry>Backend process type</entry>
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

#9Julien Rouhaud
Julien Rouhaud
rjuju123@gmail.com
In reply to: Peter Eisentraut (#8)
Re: backend type in log_line_prefix?

On Tue, Mar 10, 2020 at 4:41 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> 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.

#10Kuntal Ghosh
Kuntal Ghosh
kuntalghosh.2007@gmail.com
In reply to: Peter Eisentraut (#8)
Re: backend type in log_line_prefix?

On Tue, Mar 10, 2020 at 9:11 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2020-03-09 16:20, Kuntal Ghosh wrote:

In v3-0001-Refactor-ps_status.c-API.patch,
- * postgres: walsender <user> <host> <activity>
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

#11Alvaro Herrera
Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#6)
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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12Justin Pryzby
Justin Pryzby
pryzby@telsasoft.com
In reply to: Fujii Masao (#3)
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

#13Justin Pryzby
Justin Pryzby
pryzby@telsasoft.com
In reply to: Justin Pryzby (#12)
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(&buf, "postmaster");
        else if (MyBackendType == B_BG_WORKER)
-               appendCSVLiteral(&buf, MyBgworkerEntry->bgw_type);
+               appendCSVLiteral(&buf, MyBgworkerEntry->bgw_name);
        else
                appendCSVLiteral(&buf, 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,00000,"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,00000,"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

#14Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Justin Pryzby (#13)
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(&buf, "postmaster");
else if (MyBackendType == B_BG_WORKER)
-               appendCSVLiteral(&buf, MyBgworkerEntry->bgw_type);
+               appendCSVLiteral(&buf, MyBgworkerEntry->bgw_name);
else
appendCSVLiteral(&buf, 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

#15Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Alvaro Herrera (#11)
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

#16Justin Pryzby
Justin Pryzby
pryzby@telsasoft.com
In reply to: Peter Eisentraut (#5)
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 <peter@eisentraut.org>
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 @@ <title>What to Log</title>

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
...
<entry>Escape</entry>
<entry>Effect</entry>
<entry>Session only</entry>

<entry>Application name</entry>
<entry>yes</entry>
</row>
+            <row>
+             <entry><literal>%b</literal></entry>
+             <entry>Backend process type</entry>
+             <entry>yes</entry>

=> 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

#17Justin Pryzby
Justin Pryzby
pryzby@telsasoft.com
In reply to: Peter Eisentraut (#14)
2 attachment(s)
Re: backend type in log_line_prefix?

On Fri, Mar 13, 2020 at 10:22:52PM +0100, Peter Eisentraut wrote:

Can I suggest:

-               appendCSVLiteral(&buf, MyBgworkerEntry->bgw_type);
+               appendCSVLiteral(&buf, 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,00000,"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,00000,"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,00000,"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

Attachments:

v1-0001-Fix-previous-commit.patchtext/x-diff; charset=us-ascii
v1-0002-Include-the-leader-PID-in-logfile.patchtext/x-diff; charset=us-ascii
#18Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Peter Eisentraut (#15)
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

#19Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Justin Pryzby (#17)
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

#20Fujii Masao
Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: Peter Eisentraut (#18)
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

#21Mike Palmiotto
Mike Palmiotto
mike.palmiotto@crunchydata.com
In reply to: Fujii Masao (#20)
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:
/messages/by-id/CAMN686HgTVRJBAw6hqFE4Lj8bgPLQqfp1c-+WBGUtEmg6wPVhg@mail.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

#22Fabrízio de Royes Mello
Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Peter Eisentraut (#18)
1 attachment(s)
Re: backend type in log_line_prefix?

On Sun, Mar 15, 2020 at 7:32 AM Peter Eisentraut <
peter.eisentraut@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

Attachments:

fix-file-fdw-doc-csvlog.patchtext/x-patch; charset=US-ASCII; name=fix-file-fdw-doc-csvlog.patch
#23Bruce Momjian
Bruce Momjian
bruce@momjian.us
In reply to: Fabrízio de Royes Mello (#22)
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.eisentraut@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' );
</programlisting>

Patch applied to master, thanks.

--
Bruce Momjian <bruce@momjian.us> 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 +
#24Kyotaro Horiguchi
Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Bruce Momjian (#23)
Re: backend type in log_line_prefix?

Hello.

At Mon, 23 Mar 2020 18:38:53 -0400, Bruce Momjian <bruce@momjian.us> 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]/messages/by-id/20200319195410.icib45bbgjwqb5zn@alap3.anarazel.de 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]: /messages/by-id/20200319195410.icib45bbgjwqb5zn@alap3.anarazel.de

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#25Bruce Momjian
Bruce Momjian
bruce@momjian.us
In reply to: Kyotaro Horiguchi (#24)
1 attachment(s)
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 <bruce@momjian.us> 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]: /messages/by-id/20200319195410.icib45bbgjwqb5zn@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 <bruce@momjian.us> 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 +

Attachments:

wal.difftext/x-diff; charset=us-ascii
#26Peter Eisentraut
Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Bruce Momjian (#25)
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