On Sat, Nov 20, 2021 at 3:25 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Nov 19, 2021 at 7:41 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Thu, Nov 18, 2021 at 12:26 PM Amit Kapila <amit.kapil...@gmail.com> > > wrote: > > > > > > On Wed, Nov 17, 2021 at 7:12 PM osumi.takami...@fujitsu.com > > > <osumi.takami...@fujitsu.com> wrote: > > > > > > > > On Wednesday, November 17, 2021 10:00 PM Amit Kapila > > > > <amit.kapil...@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).
Okay, I agree that last_error_* columns will be consistent. > 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? +1 Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/