On Tue, Mar 15, 2022 at 10:09 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Mar 15, 2022 at 3:34 AM Melanie Plageman > <melanieplage...@gmail.com> wrote: > > > > On Mon, Mar 14, 2022 at 4:02 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > On Mon, Mar 14, 2022 at 2:05 AM Melanie Plageman > > > <melanieplage...@gmail.com> wrote: > > > > > > > > On Sat, Mar 12, 2022 at 3:15 PM Andres Freund <and...@anarazel.de> > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > On 2022-03-12 08:28:35 +0530, Amit Kapila wrote: > > > > > > On Sat, Mar 12, 2022 at 2:14 AM Melanie Plageman > > > > > > <melanieplage...@gmail.com> wrote: > > > > > > > > > > > > > > So, I noticed that pg_stat_reset_subscription_stats() wasn't > > > > > > > working > > > > > > > properly, and, upon further investigation, I'm not sure the view > > > > > > > pg_stat_subscription_stats is being properly populated. > > > > > > > > > > > > > > > > > > > I have tried the below scenario based on this: > > > > > > Step:1 Create some data that generates conflicts and lead to apply > > > > > > failures and then check in the view: > > > > > > > > > > I think the problem is present when there was *no* conflict > > > > > previously. Because nothing populates the stats entry without an > > > > > error, the > > > > > reset doesn't have anything to set the stats_reset field in, which > > > > > then means > > > > > that the stats_reset field is NULL even though stats have been reset. > > > > > > > > Yes, this is what I meant. stats_reset is not initialized and without > > > > any conflict happening to populate the stats, after resetting the stats, > > > > the field still does not get populated. I think this is a bit > > > > unexpected. > > > > > > > > psql (15devel) > > > > Type "help" for help. > > > > > > > > mplageman=# select * from pg_stat_subscription_stats ; > > > > subid | subname | apply_error_count | sync_error_count | stats_reset > > > > -------+---------+-------------------+------------------+------------- > > > > 16398 | mysub | 0 | 0 | > > > > (1 row) > > > > > > > > mplageman=# select pg_stat_reset_subscription_stats(16398); > > > > pg_stat_reset_subscription_stats > > > > ---------------------------------- > > > > > > > > (1 row) > > > > > > > > mplageman=# select * from pg_stat_subscription_stats ; > > > > subid | subname | apply_error_count | sync_error_count | stats_reset > > > > -------+---------+-------------------+------------------+------------- > > > > 16398 | mysub | 0 | 0 | > > > > (1 row) > > > > > > > > > > Looking at other statistics such as replication slots, shared stats, > > > and SLRU stats, it makes sense that resetting it populates the stats. > > > So we need to fix this issue. > > > > > > However, I think the proposed fix has two problems; it can create an > > > entry for non-existing subscriptions if the user directly calls > > > function pg_stat_get_subscription_stats(), and stats_reset value is > > > not updated in the stats file as it is not done by the stats > > > collector. > > > > You are right. My initial patch was incorrect. > > > > Thinking about it more, the initial behavior is technically the same for > > pg_stat_database. It is just that I didn't notice because you end up > > creating stats for pg_stat_database so quickly that you usually never > > see them before. > > > > In pg_stat_get_db_stat_reset_time(): > > > > if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL) > > result = 0; > > else > > result = dbentry->stat_reset_timestamp; > > > > if (result == 0) > > PG_RETURN_NULL(); > > else > > PG_RETURN_TIMESTAMPTZ(result); > > > > and in pgstat_recv_resetcounter(): > > > > dbentry = pgstat_get_db_entry(msg->m_databaseid, false); > > > > if (!dbentry) > > return; > > > > Thinking about it now, though, maybe an alternative solution would be to > > have all columns or all columns except the subid/subname or dbname/dboid > > be NULL until the statistics have been created, at which point the > > reset_timestamp is populated with the current timestamp. > > It's true that stats_reset is NULL if the statistics of database are > not created yet. >
So, if the behavior is the same as pg_stat_database, do we really want to change anything in this regard? -- With Regards, Amit Kapila.
