doc fix for pg_stat_activity.backend_type
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+2-1
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
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
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
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
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+3-0
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
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+4-1
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
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
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