On Friday, April 12, 2024 11:31 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Apr 11, 2024 at 5:04 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > On Thursday, April 11, 2024 12:11 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> >
> > >
> > > 2.
> > > - if (remote_slot->restart_lsn < slot->data.restart_lsn)
> > > + if (remote_slot->confirmed_lsn < slot->data.confirmed_flush)
> > > elog(ERROR,
> > > "cannot synchronize local slot \"%s\" LSN(%X/%X)"
> > >
> > > Can we be more specific in this message? How about splitting it into
> > > error_message as "cannot synchronize local slot \"%s\"" and then
> > > errdetail as "Local slot's start streaming location LSN(%X/%X) is
> > > ahead of remote slot's LSN(%X/%X)"?
> >
> > Your version looks better. Since the above two messages all have
> > errdetail, I used the style of ereport(ERROR, errmsg_internal(),
> > errdetail_internal()... in the patch which is equal to the elog(ERROR but has an
> additional detail message.
> >
>
> makes sense.
>
> > Here is V5 patch set.
> >
>
> I think we should move the check to not advance slot when one of
> remote_slot's restart_lsn or catalog_xmin is lesser than the local slot inside
> update_local_synced_slot() as we want to prevent updating slot in those cases
> even during slot synchronization.
Agreed. Here is the V6 patch which addressed this. I have merged the
two patches into one.
Best Regards,
Hou zj