Re: Failed transaction statistics to measure the logical replication progress

On Fri, Nov 19, 2021 at 7:41 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Nov 18, 2021 at 12:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Nov 17, 2021 at 7:12 PM osumi.takamichi@fujitsu.com
> > <osumi.takamichi@fujitsu.com> wrote:
> > >
> > > On Wednesday, November 17, 2021 10:00 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > 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?
> > > This is one reason.
> > >
> > > Another big reason comes from the final alignment when we list up all columns of both patches.
> > > The patches in this thread is trying to introduce a column that indicates
> > > cumulative count of error to show all error counts that the worker got in the past.
> > >
> >
> > Okay, I see your point and it makes sense to rename columns after
> > these other stats. I am not able to come up with any better names than
> > what is being used here. Sawada-San, do you agree with this, or do let
> > us know if you have any better ideas?
> >
>
> I'm concerned that these new names will introduce confusion; if we
> have last_error_relid, last_error_command, last_error_message,
> last_error_time, and last_error_xid, I think users might think that
> first_error_time is the timestamp at which an error occurred for the
> first time in the subscription worker.
>

Isn't to some extent that confusion already exists because of
last_error_time column?

> Also, I'm not sure
> last_error_count is not clear to me (it looks like showing something
> count but the only "last" one?).
>

I feel if all the error related columns have "last_error_" as a prefix
then it should not be that confusing?

> An alternative idea would be to add
> total_error_count by this patch, resulting in having both error_count
> and total_error_count. Regarding commit_count and abort_count, I
> personally think xact_commit and xact_rollback would be better since
> they’re more consistent with pg_stat_database view, although we might
> already have discussed that.
>

Even if we decide to change the column names to
xact_commit/xact_rollback, I think with additional non-error columns
it will be clear to add 'error' in column names corresponding to error
columns, and last_error_* seems to be consistent with what we have in
pg_stat_archiver (last_failed_wal, last_failed_time). Your point
related to first_error_time has merit and I don't have a better answer
for it. I think it is just a convenience column and we are not sure
whether that will be required in practice so maybe we can drop that
column and come back to it later once we get some field feedback on
this view?

--
With Regards,
Amit Kapila.