Re: Failed transaction statistics to measure the logical replication progress

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Failed transaction statistics to measure the logical replication progress
Date: 2021-11-17 13:00:04
Message-ID: CAA4eK1JCbhxuO8fEaAaw8e30=5kEDaFRXpV2+opAdMoEOH5LEg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 17, 2021 at 9:44 AM osumi(dot)takamichi(at)fujitsu(dot)com
<osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
>
> On Wednesday, November 17, 2021 12:19 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > On Tue, Nov 16, 2021 at 9:34 PM osumi(dot)takamichi(at)fujitsu(dot)com
> > <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> > >
> > > On Monday, November 15, 2021 9:14 PM I wrote:
> > > > I've conducted some update for this.
> > > > (The rebased part is only C code and checked by pgindent)
> > > I'll update my patches since a new skip xid patch has been shared in
> > > [1].
> > >
> > > This version includes some minor renames of functions that are related
> > > to transaction sizes.
> >
> > I've looked at v12-0001 patch. Here are some comments:
> Thank you for paying attention to this thread !
>
>
> > - TupleDescInitEntry(tupdesc, (AttrNumber) 3, "relid",
> > + TupleDescInitEntry(tupdesc, (AttrNumber) 3, "last_error_relid",
> > OIDOID, -1, 0);
> > - TupleDescInitEntry(tupdesc, (AttrNumber) 4, "command",
> > + TupleDescInitEntry(tupdesc, (AttrNumber) 4, "last_error_command",
> > TEXTOID, -1, 0);
> > - TupleDescInitEntry(tupdesc, (AttrNumber) 5, "xid",
> > + TupleDescInitEntry(tupdesc, (AttrNumber) 5, "last_error_xid",
> > XIDOID, -1, 0);
> > - TupleDescInitEntry(tupdesc, (AttrNumber) 6, "error_count",
> > + TupleDescInitEntry(tupdesc, (AttrNumber) 6, "last_error_count",
> > INT8OID, -1, 0);
> > - TupleDescInitEntry(tupdesc, (AttrNumber) 7, "error_message",
> > + TupleDescInitEntry(tupdesc, (AttrNumber) 7, "last_error_message",
> >
> > If renaming column names clarifies those meanings, the above changes should
> > be included into my patch that introduces pg_stat_subscription_workers
> > view?

Right.

> At first, your column names of pg_stat_subscription_workers look totally OK to me by itself
> and I thought I should take care of those renaming at the commit timing of my stats patches.
>

Can you please tell us why you think the names in your proposed patch
are better than the existing names proposed in Sawada-San's patch? Is
it because those fields always contain the information of the last or
latest error that occurred in the corresponding subscription worker?
If so, I am not very sure if that is a good reason to increase the
length of most of the column names but if you and others feel that is
helpful then it is better to do it as part of Sawada-San's patch.

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-11-17 13:16:46 Re: Proposal: allow database-specific role memberships
Previous Message Amul Sul 2021-11-17 12:50:42 Re: [Patch] ALTER SYSTEM READ ONLY