public inbox for [email protected]  
help / color / mirror / Atom feed
From: Masahiko Sawada <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Design of pg_stat_subscription_workers vs pgstats
Date: Thu, 24 Feb 2022 21:17:55 +0900
Message-ID: <CAD21AoCQVR0mFq5p+U-VrbpnVPPf_EVQ3ELYQ7p6pD7Qa8KDkA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1JxO81F7EbUfQ0G=KObga-bm4O=2VRbA2mOCJKCHCJhNw@mail.gmail.com>
References: <[email protected]>
	<CAA4eK1Jwn5hhgHJWZ9n+wYyQEk+brfR1zkuqLjh=waji8RYWAw@mail.gmail.com>
	<CAD21AoAqEt=TbUDY96Uhf9pNuh-NDR_3-CByd+xR=tFEq==1NA@mail.gmail.com>
	<CAD21AoA-MVZcMA1iYwbtvTCOHx1SwG8amQUTT8j2ssh2zuc6OQ@mail.gmail.com>
	<[email protected]>
	<CAD21AoDTO3bJvrc39Tv3VCeyUzZpimXnJuRhHQhKyjOWorHeOA@mail.gmail.com>
	<[email protected]>
	<CAD21AoA_r+Wc-zJZdy4Lii3oOvvdGcLwrY=2Y4y63HueYSd6Eg@mail.gmail.com>
	<[email protected]>
	<CAA4eK1+j00Pc+p=AfJLXh-a+OJ-a0UD4adMr2tRkQvKG_LggHA@mail.gmail.com>
	<[email protected]>
	<CAA4eK1J3dhfMRNQ6Q585nFr1vmXaTXVW9qv4hmUxLwDwJp-9RQ@mail.gmail.com>
	<CAD21AoAtrgBEUU08VvfzRnGS+=9rS=MEq1cTLaZ1OT51xX34dQ@mail.gmail.com>
	<CAHut+PtH-uN5rbGRh-=kCd8xvQYDf_JCcjLcVjW3OXGz6T+xCw@mail.gmail.com>
	<CAD21AoCCaQfGzDCAJgYH=yo72GXozpoMF4fau=5gViXK+uquWw@mail.gmail.com>
	<OS0PR01MB6113769B17E90ADC9ACA14B2FB3D9@OS0PR01MB6113.jpnprd01.prod.outlook.com>
	<CAHut+Pv=VmXtHmPKp4fg8VDF+TQP6xWgL91Jn-hrqg5QObfCZA@mail.gmail.com>
	<CAA4eK1JxO81F7EbUfQ0G=KObga-bm4O=2VRbA2mOCJKCHCJhNw@mail.gmail.com>

On Thu, Feb 24, 2022 at 9:05 PM Amit Kapila <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 2:24 PM Peter Smith <[email protected]> wrote:
> >
> > 10. src/backend/replication/logical/worker.c
> >
> > (from my previous [Peter-v1] #9)
> >
> > >> + /* Report the worker failed during table synchronization */
> > >> + pgstat_report_subscription_error(MyLogicalRepWorker->subid, false);
> > >>
> > >> and
> > >>
> > >> + /* Report the worker failed during the application of the change */
> > >> + pgstat_report_subscription_error(MyLogicalRepWorker->subid, true);
> > >>
> > >>
> > >> Why don't these use MySubscription->oid instead of MyLogicalRepWorker->subid?
> >
> > > It's just because we used to use MyLogicalRepWorker->subid, is there
> > > any particular reason why we should use MySubscription->oid here?
> >
> > I felt MySubscription->oid is a more natural and more direct way of
> > expressing the same thing.
> >
> > Consider:  "the oid of the current subscription" versus "the oid of
> > the subscription of the current worker". IMO the first one is simpler.
> > YMMV.
> >
>
> I think we can use either but maybe MySubscription->oid would be
> slightly better here as the same is used in nearby code as well.

Okay, will change.

> >
> > 13. src/test/subscription/t/026_worker_stats.pl - missing test?
> >
> > Shouldn't there also be some test to reset the counters to confirm
> > that they really do get reset to zero?
> >
> > ~~~
> >
>
> I think we avoid writing tests for stats for each and every case as it
> is not reliable in nature (the message can be lost). If we can find a
> reliable way then it is okay.

Yeah, the messages can even be out-of-order. Particularly, in this
test, the apply worker and table sync worker keep reporting the
messages, it's quite possible that the test becomes unstable. I
remember we removed unstable tests of resetting statistics before
(e.g., see fc6950913).

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/






view thread (40+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Design of pg_stat_subscription_workers vs pgstats
  In-Reply-To: <CAD21AoCQVR0mFq5p+U-VrbpnVPPf_EVQ3ELYQ7p6pD7Qa8KDkA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox