On Thursday, October 14, 2021 2:13 PM Osumi, Takamichi wrote: > On Thursday, October 14, 2021 12:54 PM Hou, Zhijie<houzj.f...@fujitsu.com> > wrote: > > On Thursday, September 30, 2021 12:15 PM Amit Kapila > > > On Thu, Sep 30, 2021 at 8:22 AM Hou, Zhijie/侯 志杰wrote: > > > > > > > > On Tues, Sep 28, 2021 6:05 PM Amit Kapila > > > > <amit.kapil...@gmail.com> > > wrote: > > > > > > > > > > Can't we keep the current and new stats both in-memory and > > > > > persist on disk? So, the persistent stats data will be used to > > > > > fill the in-memory counters after restarting of workers, > > > > > otherwise, we will always refer to in-memory values. > > > > > > > > I think this approach works, but I have another concern about it. > > > > > > > > The current pg_stat_subscription view is listed as "Dynamic > > > > Statistics Views" in > > > > the document, the data in it seems about the worker process, and > > > > the view data > > > > shows what the current worker did. But if we keep the new xact > > > > stat persist, then it's not what the current worker did, it looks > > > > more related to the subscription historic data. > > > > > > > > > > I see your point. > > > > > > > Adding a new view seems resonalble, but it will bring another > > > > subscription related view which might be too much. OTOH, I can see > > > > there are already some > > > > different views[1] including xact stat, maybe adding another one > > > > is accepatble ? > > > > > > > > > > These all views are related to untransmitted to the collector but > > > what we really need is a view similar to pg_stat_archiver or > > > pg_stat_bgwriter which gives information about background workers. > > > Now, the problem as I see is if we go that route then > > > pg_stat_subscription will no longer remain dynamic view and one > > > might consider that as a compatibility break. The other idea I have > > > shared is that we display these stats under the new view introduced > > > by Sawada-San's patch [1] and probably rename that view as > > > pg_stat_subscription_worker where all the stats (xact info and last > > > failure information) about each worker will be displayed. Do you > > > have any opinion on that idea or do you see any problem with it? > > > > Personally, I think it seems reasonable to merge the xact stat into > > the view from sawada-san's patch. > > > > One problem I noticed is that pg_stat_subscription_error currently > > have a 'count' column which show how many times the last error > > happened. The xact stat here also have a similar value 'xact_error'. I > > think we might need to rename it or merge them into one in some way. > > > > Besides, if we decide to merge xact stat into > > pg_stat_subscription_error, some column seems need to be renamed. > Maybe like: > > error_message => Last_error_message, command=> last_error_command.. > Yeah, we must make them distinguished clearly. > > I guessed that you are concerned about > amount of renaming codes that could be a bit large or you come up with a > necessity to consider the all column names of the pg_stat_subscription_worker > together all at once in advance. > > It's because my instant impression is, > when we go with the current xact stats column definitions (xact_commit, > xact_commit_bytes, xact_error, xact_error_bytes, xact_abort, > xact_abort_bytes), the renaming problem can be solved if I write one > additional patch or extend the main patch of xact stats to handle renaming. > (This can work to keep both threads independent from each other). > > Did you have some concern that cannot be handled by this way ? Hi,
Currently, I don't find some unsolvable issues in this approach. Best regards, Hou zj
