doc fix for pg_stat_activity.backend_type

Started by John Naylorabout 7 years ago12 messages
#1John Naylor
jcnaylor@gmail.com
1 attachment(s)

Hi all,

Commit fc70a4b0df3 added backend_type to pg_stat_activity, but the
documentation omitted "logical replication launcher". Patch attached.

-John Naylor

Attachments:

pg_stat_activity-fix.patchtext/x-patch; charset=US-ASCII; name=pg_stat_activity-fix.patchDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index add71458e2..ddf607bb17 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -807,7 +807,8 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
       <literal>background worker</literal>, <literal>background writer</literal>,
       <literal>client backend</literal>, <literal>checkpointer</literal>,
       <literal>startup</literal>, <literal>walreceiver</literal>,
-      <literal>walsender</literal> and <literal>walwriter</literal>.
+      <literal>walsender</literal>, <literal>walwriter</literal>
+      and <literal>logical replication launcher</literal>.
      </entry>
     </row>
    </tbody>
#2Amit Kapila
amit.kapila16@gmail.com
In reply to: John Naylor (#1)
Re: doc fix for pg_stat_activity.backend_type

On Mon, Nov 12, 2018 at 1:20 PM John Naylor <jcnaylor@gmail.com> wrote:

Hi all,

Commit fc70a4b0df3 added backend_type to pg_stat_activity, but the
documentation omitted "logical replication launcher". Patch attached.

Isn't this the fallout of commit 5373bc2a08 which has added background
worker types? If so, I guess your patch needs modification.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#3John Naylor
jcnaylor@gmail.com
In reply to: Amit Kapila (#2)
Re: doc fix for pg_stat_activity.backend_type

On 11/12/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Mon, Nov 12, 2018 at 1:20 PM John Naylor <jcnaylor@gmail.com> wrote:

Hi all,

Commit fc70a4b0df3 added backend_type to pg_stat_activity, but the
documentation omitted "logical replication launcher". Patch attached.

Isn't this the fallout of commit 5373bc2a08 which has added background
worker types? If so, I guess your patch needs modification.

Looks like it. A quick search revealed "parallel worker" and "logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly documented.
If that sounds right I'll update the patch to include the first two.

-John Naylor

#4Michael Paquier
michael@paquier.xyz
In reply to: John Naylor (#3)
Re: doc fix for pg_stat_activity.backend_type

On Mon, Nov 12, 2018 at 09:42:45PM +0700, John Naylor wrote:

Looks like it. A quick search revealed "parallel worker" and "logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly documented.
If that sounds right I'll update the patch to include the first two.

Just wondering: do we actually need to include in the docs this list at
all? This is a recipe to forget its update each time a new backend type
is added.
--
Michael

#5Amit Kapila
amit.kapila16@gmail.com
In reply to: Michael Paquier (#4)
Re: doc fix for pg_stat_activity.backend_type

On Tue, Nov 13, 2018 at 5:38 AM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Nov 12, 2018 at 09:42:45PM +0700, John Naylor wrote:

Looks like it. A quick search revealed "parallel worker" and "logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly documented.
If that sounds right I'll update the patch to include the first two.

Just wondering: do we actually need to include in the docs this list at
all? This is a recipe to forget its update each time a new backend type
is added.

Sure, but how will we justify documenting (autovacuum launcher and
autovacuum worker) and not (logical replication launcher and logical
replication worker)? I think we can document the type of workers that
are part of core-server functionality. We can make some generic
statement on the workers that can be launched by extensions.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#6John Naylor
jcnaylor@gmail.com
In reply to: Amit Kapila (#5)
1 attachment(s)
Re: doc fix for pg_stat_activity.backend_type

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 5:38 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Mon, Nov 12, 2018 at 09:42:45PM +0700, John Naylor wrote:

Looks like it. A quick search revealed "parallel worker" and "logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly documented.
If that sounds right I'll update the patch to include the first two.

Just wondering: do we actually need to include in the docs this list at
all? This is a recipe to forget its update each time a new backend type
is added.

Sure, but how will we justify documenting (autovacuum launcher and
autovacuum worker) and not (logical replication launcher and logical
replication worker)? I think we can document the type of workers that
are part of core-server functionality. We can make some generic
statement on the workers that can be launched by extensions.

How about something like the attached?

-John Naylor

Attachments:

pg_stat_activity-fix-v2.patchtext/x-patch; charset=US-ASCII; name=pg_stat_activity-fix-v2.patchDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index add71458e2..886477cf09 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -804,10 +804,13 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
      <entry><type>text</type></entry>
      <entry>Type of current backend. Possible types are
       <literal>autovacuum launcher</literal>, <literal>autovacuum worker</literal>,
+      <literal>logical replication launcher</literal>,
+      <literal>logical replication worker</literal>, <literal>parallel worker</literal>,
       <literal>background worker</literal>, <literal>background writer</literal>,
       <literal>client backend</literal>, <literal>checkpointer</literal>,
       <literal>startup</literal>, <literal>walreceiver</literal>,
       <literal>walsender</literal> and <literal>walwriter</literal>.
+      In addition, extensions may have additional types.
      </entry>
     </row>
    </tbody>
#7Amit Kapila
amit.kapila16@gmail.com
In reply to: John Naylor (#6)
Re: doc fix for pg_stat_activity.backend_type

On Tue, Nov 13, 2018 at 12:04 PM John Naylor <jcnaylor@gmail.com> wrote:

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 5:38 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Mon, Nov 12, 2018 at 09:42:45PM +0700, John Naylor wrote:

Looks like it. A quick search revealed "parallel worker" and "logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly documented.
If that sounds right I'll update the patch to include the first two.

Just wondering: do we actually need to include in the docs this list at
all? This is a recipe to forget its update each time a new backend type
is added.

Sure, but how will we justify documenting (autovacuum launcher and
autovacuum worker) and not (logical replication launcher and logical
replication worker)? I think we can document the type of workers that
are part of core-server functionality. We can make some generic
statement on the workers that can be launched by extensions.

How about something like the attached?

Don't you need to remove <literal>background worker</literal>?

+ In addition, extensions may have additional types.

How about: "In addition, background workers registered by extensions
may have additional types."?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#8John Naylor
jcnaylor@gmail.com
In reply to: Amit Kapila (#7)
1 attachment(s)
Re: doc fix for pg_stat_activity.backend_type

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 12:04 PM John Naylor <jcnaylor@gmail.com> wrote:

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 5:38 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Mon, Nov 12, 2018 at 09:42:45PM +0700, John Naylor wrote:

Looks like it. A quick search revealed "parallel worker" and
"logical
replication worker". src/test/modules/ also show "test_shm_mq" and
"worker_spi", but it seems those don't need to be publicly
documented.
If that sounds right I'll update the patch to include the first two.

Just wondering: do we actually need to include in the docs this list
at
all? This is a recipe to forget its update each time a new backend
type
is added.

Sure, but how will we justify documenting (autovacuum launcher and
autovacuum worker) and not (logical replication launcher and logical
replication worker)? I think we can document the type of workers that
are part of core-server functionality. We can make some generic
statement on the workers that can be launched by extensions.

How about something like the attached?

Don't you need to remove <literal>background worker</literal>?

It's handled in pgstat_get_backend_desc(), so I assumed not. If that's
just a place holder, then it's probably better left out, as in the
attached.

+ In addition, extensions may have additional types.

How about: "In addition, background workers registered by extensions
may have additional types."?

Sounds good to me -- I've included this language.

-John Naylor

Attachments:

pg_stat_activity-fix-v3.patchtext/x-patch; charset=US-ASCII; name=pg_stat_activity-fix-v3.patchDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index add71458e2..9600ef0d64 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -804,10 +804,13 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
      <entry><type>text</type></entry>
      <entry>Type of current backend. Possible types are
       <literal>autovacuum launcher</literal>, <literal>autovacuum worker</literal>,
-      <literal>background worker</literal>, <literal>background writer</literal>,
+      <literal>logical replication launcher</literal>,
+      <literal>logical replication worker</literal>,
+      <literal>parallel worker</literal>, <literal>background writer</literal>,
       <literal>client backend</literal>, <literal>checkpointer</literal>,
       <literal>startup</literal>, <literal>walreceiver</literal>,
       <literal>walsender</literal> and <literal>walwriter</literal>.
+      In addition, background workers registered by extensions may have additional types.
      </entry>
     </row>
    </tbody>
#9Amit Kapila
amit.kapila16@gmail.com
In reply to: John Naylor (#8)
Re: doc fix for pg_stat_activity.backend_type

On Tue, Nov 13, 2018 at 3:37 PM John Naylor <jcnaylor@gmail.com> wrote:

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

Don't you need to remove <literal>background worker</literal>?

It's handled in pgstat_get_backend_desc(), so I assumed not. If that's
just a place holder, then it's probably better left out, as in the
attached.

I don't think 'background worker' can be displayed as backend_type.
Do you see any way it can be displayed as backend_type?

+ In addition, extensions may have additional types.

How about: "In addition, background workers registered by extensions
may have additional types."?

Sounds good to me -- I've included this language.

LGTM. I will wait for a day or so, if nobody has any comments, I will
push your patch.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#10John Naylor
jcnaylor@gmail.com
In reply to: Amit Kapila (#9)
Re: doc fix for pg_stat_activity.backend_type

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 3:37 PM John Naylor <jcnaylor@gmail.com> wrote:

On 11/13/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

Don't you need to remove <literal>background worker</literal>?

It's handled in pgstat_get_backend_desc(), so I assumed not. If that's
just a place holder, then it's probably better left out, as in the
attached.

I don't think 'background worker' can be displayed as backend_type.

I think you're right (pgstatfuncs.c, starting at line 826).

-John Naylor

#11Amit Kapila
amit.kapila16@gmail.com
In reply to: Amit Kapila (#9)
Re: doc fix for pg_stat_activity.backend_type

On Tue, Nov 13, 2018 at 5:03 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Nov 13, 2018 at 3:37 PM John Naylor <jcnaylor@gmail.com> wrote:

+ In addition, extensions may have additional types.

How about: "In addition, background workers registered by extensions
may have additional types."?

Sounds good to me -- I've included this language.

LGTM. I will wait for a day or so, if nobody has any comments, I will
push your patch.

Pushed, thanks for working on this.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#12John Naylor
jcnaylor@gmail.com
In reply to: Amit Kapila (#11)
Re: doc fix for pg_stat_activity.backend_type

On 11/15/18, Amit Kapila <amit.kapila16@gmail.com> wrote:

Pushed, thanks for working on this.

Thanks for your input.

-John Naylor