On Thu, Jul 1, 2021 at 12:56 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jun 30, 2021 at 4:35 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Jun 28, 2021 at 10:12 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > > > > 0003 patch adds pg_stat_logical_replication_error statistics view > > > discussed on another thread[1]. The apply worker sends the error > > > information to the stats collector if an error happens during applying > > > changes. We can check those errors as follow: > > > > > > postgres(1:25250)=# select * from pg_stat_logical_replication_error; > > > subname | relid | action | xid | last_failure > > > ----------+-------+--------+-----+------------------------------- > > > test_sub | 16384 | INSERT | 736 | 2021-06-27 12:12:45.142675+09 > > > (1 row) > > > > > > I added only columns required for the skipping transaction feature to > > > the view for now. > > > > > > > Isn't it better to add an error message if possible? > > > > Don't we want to clear stats at drop subscription as well? We do drop > database stats in dropdb via pgstat_drop_database, so I think we need > to clear subscription stats at the time of drop subscription.
Yes, it needs to be cleared. In the 0003 patch, pgstat_vacuum_stat() sends the message to clear the stats. I think it's better to have pgstat_vacuum_stat() do that job similar to dropping replication slot statistics rather than relying on the single message send at DROP SUBSCRIPTION. I've considered doing both: sending the message at DROP SUBSCRIPTION and periodical checking by pgstat_vacuum_stat(), but dropping subscription not setting a replication slot is able to rollback. So we need to send it only at commit time. Given that we don’t necessarily need the stats to be updated immediately, I think it’s reasonable to go with only a way of pgstat_vacuum_stat(). > In the 0003 patch, if I am reading it correctly then the patch is not > doing anything for tablesync worker. It is not clear to me at this > stage what exactly we want to do about it? Do we want to just ignore > errors from tablesync worker and let the system behave as it is > without this feature? If we want to do anything then I think the way > to skip the initial table sync would be to behave like the user has > given 'copy_data' option as false. It might be better to have also sync workers report errors, even if SKIP TRANSACTION feature doesn’t support anything for initial table synchronization. From the user perspective, The initial table synchronization is also the part of logical replication operations. If we report only error information of applying logical changes, it could confuse users. But I’m not sure about the way to skip the initial table synchronization. Once we set `copy_data` to false, all table synchronizations are disabled. Some of them might have been able to synchronize successfully. It might be useful if the user can disable the table initialization for the particular tables. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
