Re: Add 'worker_type' to pg_stat_subscription

2023-09-25 Thread Peter Smith
On Tue, Sep 26, 2023 at 7:16 AM Nathan Bossart wrote: > > On Thu, Sep 21, 2023 at 04:01:20PM +0530, Amit Kapila wrote: > > The changes looks good to me, though I haven't tested it. But feel > > free to commit if you are fine with this patch. > > Committed. > Thanks for pushing. -- Kind

Re: Add 'worker_type' to pg_stat_subscription

2023-09-25 Thread Nathan Bossart
On Thu, Sep 21, 2023 at 04:01:20PM +0530, Amit Kapila wrote: > The changes looks good to me, though I haven't tested it. But feel > free to commit if you are fine with this patch. Committed. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: Add 'worker_type' to pg_stat_subscription

2023-09-21 Thread Amit Kapila
On Thu, Sep 21, 2023 at 5:36 AM Peter Smith wrote: > > On Thu, Sep 21, 2023 at 9:34 AM Nathan Bossart > wrote: > > > > On Thu, Sep 21, 2023 at 08:14:23AM +0900, Michael Paquier wrote: > > > On Thu, Sep 21, 2023 at 09:01:01AM +1000, Peter Smith wrote: > > >> One question -- the patch comment

Re: Add 'worker_type' to pg_stat_subscription

2023-09-21 Thread Amit Kapila
On Thu, Sep 21, 2023 at 1:00 AM Nathan Bossart wrote: > > On Tue, Sep 19, 2023 at 08:36:35AM +0530, Amit Kapila wrote: > > I am of the opinion that worker_type should be 'apply' instead of > > 'leader apply' because even when it is a leader for parallel apply > > worker, it could perform

Re: Add 'worker_type' to pg_stat_subscription

2023-09-20 Thread Peter Smith
On Thu, Sep 21, 2023 at 9:34 AM Nathan Bossart wrote: > > On Thu, Sep 21, 2023 at 08:14:23AM +0900, Michael Paquier wrote: > > On Thu, Sep 21, 2023 at 09:01:01AM +1000, Peter Smith wrote: > >> One question -- the patch comment still says "Bumps catversion.", but > >> catversion.h change is

Re: Add 'worker_type' to pg_stat_subscription

2023-09-20 Thread Nathan Bossart
On Thu, Sep 21, 2023 at 08:14:23AM +0900, Michael Paquier wrote: > On Thu, Sep 21, 2023 at 09:01:01AM +1000, Peter Smith wrote: >> One question -- the patch comment still says "Bumps catversion.", but >> catversion.h change is missing from the v9 patch? > > Yeah, previous patches did that, but it

Re: Add 'worker_type' to pg_stat_subscription

2023-09-20 Thread Michael Paquier
On Thu, Sep 21, 2023 at 09:01:01AM +1000, Peter Smith wrote: > One question -- the patch comment still says "Bumps catversion.", but > catversion.h change is missing from the v9 patch? Yeah, previous patches did that, but it is no big deal. My take is that it is a good practice to never do a

Re: Add 'worker_type' to pg_stat_subscription

2023-09-20 Thread Peter Smith
On Thu, Sep 21, 2023 at 5:30 AM Nathan Bossart wrote: > > On Tue, Sep 19, 2023 at 08:36:35AM +0530, Amit Kapila wrote: > > I am of the opinion that worker_type should be 'apply' instead of > > 'leader apply' because even when it is a leader for parallel apply > > worker, it could perform

Re: Add 'worker_type' to pg_stat_subscription

2023-09-20 Thread Nathan Bossart
On Tue, Sep 19, 2023 at 08:36:35AM +0530, Amit Kapila wrote: > I am of the opinion that worker_type should be 'apply' instead of > 'leader apply' because even when it is a leader for parallel apply > worker, it could perform individual transactions apply. For reference, > I checked

Re: Add 'worker_type' to pg_stat_subscription

2023-09-18 Thread Amit Kapila
On Mon, Sep 18, 2023 at 8:50 PM Nathan Bossart wrote: > > On Mon, Sep 18, 2023 at 04:56:46PM +1000, Peter Smith wrote: > > On Mon, Sep 18, 2023 at 1:43 PM Amit Kapila wrote: > >> One simple idea to reduce confusion could be to use (leader) in the > >> above two places. Do you see any other place

Re: Add 'worker_type' to pg_stat_subscription

2023-09-18 Thread Peter Smith
On Tue, Sep 19, 2023 at 1:20 AM Nathan Bossart wrote: > > On Mon, Sep 18, 2023 at 04:56:46PM +1000, Peter Smith wrote: > > On Mon, Sep 18, 2023 at 1:43 PM Amit Kapila wrote: > >> One simple idea to reduce confusion could be to use (leader) in the > >> above two places. Do you see any other place

Re: Add 'worker_type' to pg_stat_subscription

2023-09-18 Thread Nathan Bossart
On Mon, Sep 18, 2023 at 04:56:46PM +1000, Peter Smith wrote: > On Mon, Sep 18, 2023 at 1:43 PM Amit Kapila wrote: >> One simple idea to reduce confusion could be to use (leader) in the >> above two places. Do you see any other place which could be confusing >> and what do you suggest to fix it? >

Re: Add 'worker_type' to pg_stat_subscription

2023-09-18 Thread Peter Smith
On Mon, Sep 18, 2023 at 1:43 PM Amit Kapila wrote: > > On Sun, Sep 17, 2023 at 2:10 AM Nathan Bossart > wrote: > > > > On Sat, Sep 16, 2023 at 06:09:48PM +0530, Amit Kapila wrote: > > > > This still leaves the possibility for confusion with the documentation's > > use of "leader apply worker",

Re: Add 'worker_type' to pg_stat_subscription

2023-09-17 Thread Amit Kapila
On Mon, Sep 18, 2023 at 6:10 AM Peter Smith wrote: > > IIUC some future feature syncing of sequences is likely to share a lot > of the tablesync worker code (maybe it is only differentiated by the > relid being for a RELKIND_SEQUENCE?). > > The original intent of this stats worker-type patch was

Re: Add 'worker_type' to pg_stat_subscription

2023-09-17 Thread Amit Kapila
On Sun, Sep 17, 2023 at 2:10 AM Nathan Bossart wrote: > > On Sat, Sep 16, 2023 at 06:09:48PM +0530, Amit Kapila wrote: > > This still leaves the possibility for confusion with the documentation's > use of "leader apply worker", but I haven't touched that for now. > We may want to fix that

Re: Add 'worker_type' to pg_stat_subscription

2023-09-17 Thread Peter Smith
IIUC some future feature syncing of sequences is likely to share a lot of the tablesync worker code (maybe it is only differentiated by the relid being for a RELKIND_SEQUENCE?). The original intent of this stats worker-type patch was to be able to easily know the type of the process without

Re: Add 'worker_type' to pg_stat_subscription

2023-09-16 Thread Nathan Bossart
On Sat, Sep 16, 2023 at 06:09:48PM +0530, Amit Kapila wrote: > I think there is a merit in keeping the worker type as 'sync' or > 'synchronization' because these would be used in future for syncing > other objects like sequences. One more thing that slightly looks odd > is the 'leader apply' type

Re: Add 'worker_type' to pg_stat_subscription

2023-09-16 Thread Amit Kapila
On Sat, Sep 16, 2023 at 1:06 AM Nathan Bossart wrote: > > On Thu, Sep 14, 2023 at 03:04:19PM -0700, Nathan Bossart wrote: > > The only reason I didn't apply this already is because IMHO we should > > adjust the worker types and the documentation for the view to be > > consistent. For example,

Re: Add 'worker_type' to pg_stat_subscription

2023-09-15 Thread Michael Paquier
On Fri, Sep 15, 2023 at 09:35:38AM -0700, Nathan Bossart wrote: > Concretely, like this. There are two references to "synchronization worker" in tablesync.c (exit routine and busy loop), and a bit more of "sync worker".. Anyway, these don't matter much, but there are two errmsgs where the term

Re: Add 'worker_type' to pg_stat_subscription

2023-09-15 Thread Nathan Bossart
On Thu, Sep 14, 2023 at 03:04:19PM -0700, Nathan Bossart wrote: > The only reason I didn't apply this already is because IMHO we should > adjust the worker types and the documentation for the view to be > consistent. For example, the docs say "leader apply worker" but the view > just calls them

Re: Add 'worker_type' to pg_stat_subscription

2023-09-14 Thread Nathan Bossart
On Wed, Sep 13, 2023 at 09:59:04AM -0700, Nathan Bossart wrote: > On Wed, Sep 13, 2023 at 05:06:28PM +0300, Maxim Orlov wrote: >> So, should we mark this thread as RfC? > > I've done so. Barring additional feedback, I intend to commit this in the > next few days. I did some staging work for the

Re: Add 'worker_type' to pg_stat_subscription

2023-09-13 Thread Nathan Bossart
On Wed, Sep 13, 2023 at 09:59:04AM -0700, Nathan Bossart wrote: > On Wed, Sep 13, 2023 at 05:06:28PM +0300, Maxim Orlov wrote: >> So, should we mark this thread as RfC? > > I've done so. Barring additional feedback, I intend to commit this in the > next few days. Note to self: this needs a

Re: Add 'worker_type' to pg_stat_subscription

2023-09-13 Thread Nathan Bossart
On Wed, Sep 13, 2023 at 05:06:28PM +0300, Maxim Orlov wrote: > I did a look at the patch, like the idea. The overall code is in a good > condition, implements the described feature. Thanks for reviewing. > Side note: this is not a problem of this particular patch, but in >

Re: Add 'worker_type' to pg_stat_subscription

2023-09-13 Thread Maxim Orlov
Hi! I did a look at the patch, like the idea. The overall code is in a good condition, implements the described feature. Side note: this is not a problem of this particular patch, but in pg_stat_get_subscription and many other places, there is a switch with worker types. Can we use a default

Re: Add 'worker_type' to pg_stat_subscription

2023-09-12 Thread Michael Paquier
On Tue, Sep 12, 2023 at 07:00:14PM +1000, Peter Smith wrote: > Right. I found just a single test currently using pg_stat_subscription > catalog. I added a worker_type check for that. This looks enough to me, thanks! -- Michael signature.asc Description: PGP signature

Re: Add 'worker_type' to pg_stat_subscription

2023-09-12 Thread Peter Smith
On Tue, Sep 12, 2023 at 1:44 PM Michael Paquier wrote: > > On Tue, Sep 12, 2023 at 01:07:51PM +1000, Peter Smith wrote: > > The type is only assigned during worker process launch, and during > > process cleanup [1]. It's only possible to be UNKNOWN after > > logicalrep_worker_cleanup(). > > > >

Re: Add 'worker_type' to pg_stat_subscription

2023-09-11 Thread Michael Paquier
On Tue, Sep 12, 2023 at 01:07:51PM +1000, Peter Smith wrote: > The type is only assigned during worker process launch, and during > process cleanup [1]. It's only possible to be UNKNOWN after > logicalrep_worker_cleanup(). > > AFAIK the stats can never see a worker with an UNKNOWN type, although

Re: Add 'worker_type' to pg_stat_subscription

2023-09-11 Thread Peter Smith
On Fri, Sep 8, 2023 at 8:28 AM Nathan Bossart wrote: > > On Thu, Sep 07, 2023 at 12:36:29PM +1200, Peter Smith wrote: > > Modified as suggested. PSA v3. > > Thanks. I've attached v4 with a couple of small changes. Notably, I've > moved the worker_type column to before the pid column, as it felt

Re: Add 'worker_type' to pg_stat_subscription

2023-09-07 Thread Nathan Bossart
On Thu, Sep 07, 2023 at 12:36:29PM +1200, Peter Smith wrote: > Modified as suggested. PSA v3. Thanks. I've attached v4 with a couple of small changes. Notably, I've moved the worker_type column to before the pid column, as it felt more natural to me to keep the PID columns together. I've also

Re: Add 'worker_type' to pg_stat_subscription

2023-09-06 Thread Peter Smith
On Wed, Sep 6, 2023 at 9:49 AM Nathan Bossart wrote: > > On Wed, Sep 06, 2023 at 09:02:21AM +1200, Peter Smith wrote: > > On Sat, Sep 2, 2023 at 7:41 AM Nathan Bossart > > wrote: > >> I see that the table refers to "leader apply workers". Would those show up > >> as parallel apply workers in

Re: Add 'worker_type' to pg_stat_subscription

2023-09-05 Thread Nathan Bossart
On Wed, Sep 06, 2023 at 09:02:21AM +1200, Peter Smith wrote: > On Sat, Sep 2, 2023 at 7:41 AM Nathan Bossart > wrote: >> I see that the table refers to "leader apply workers". Would those show up >> as parallel apply workers in the view? Can we add another worker type for >> those? > >

Re: Add 'worker_type' to pg_stat_subscription

2023-09-05 Thread Peter Smith
ord "leader". Now there are: "apply worker" "parallel apply worker" "table synchronization worker" PSA patch v2. -- Kind Regards, Peter Smith. Fujitsu Australia From bac581d9f6843b3df0dd5fc45e318594a7921ee6 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Mon, 4 Sep

Re: Add 'worker_type' to pg_stat_subscription

2023-09-01 Thread Nathan Bossart
On Wed, Aug 16, 2023 at 07:14:18PM +1000, Peter Smith wrote: > Earlier this year I proposed a small change for the pg_stat_subscription view: > > -- > ...it would be very useful to have an additional "kind" attribute for > this view. This will save the user from needing to do mental >

Add 'worker_type' to pg_stat_subscription

2023-08-16 Thread Peter Smith
Hi hackers, Earlier this year I proposed a small change for the pg_stat_subscription view: -- ...it would be very useful to have an additional "kind" attribute for this view. This will save the user from needing to do mental gymnastics every time just to recognise what kind of process they