On Thu, Jul 1, 2021 at 12:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Jun 30, 2021 at 4:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Jun 28, 2021 at 10:12 AM Masahiko Sawada <sawada.mshk@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/
