RE: Skipping logical replication transactions on subscriber side

On July 20, 2021 9:26 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Mon, Jul 19, 2021 at 8:38 PM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On July 19, 2021 2:40 PM Masahiko Sawada <sawada.mshk@gmail.com>
> wrote:
> > > I've attached the updated version patch that incorporated all
> > > comments I got so far except for the clearing error details part I
> > > mentioned above. After getting a consensus on those parts, I'll
> > > incorporate the idea into the patches.
> >
> > 3) For 0003 patch, if user set skip_xid to a wrong xid which have not been
> >    assigned, and then will the change be skipped when the xid is assigned in
> >    the future even if it doesn't cause any conflicts ?
> 
> Yes. Currently, setting a correct xid is the user's responsibility. I think it would
> be better to disable it or emit WARNING/ERROR when the user mistakenly set
> the wrong xid if we find out a convenient way to detect that.

Thanks for the explanation. As Amit suggested, it seems we can document the
risk of misusing skip_xid. Besides, I found some minor things in the patch.

1) In 0002 patch

+ */
+static void
+pgstat_recv_subscription_purge(PgStat_MsgSubscriptionPurge *msg, int len)
+{
+    if (subscriptionErrHash != NULL)
+        return;
+

+static void
+pgstat_recv_subscription_error(PgStat_MsgSubscriptionErr *msg, int len)
+{

the second paramater "len" seems not used in the function
pgstat_recv_subscription_purge() and pgstat_recv_subscription_error().


2) in 0003 patch

  * Helper function for apply_handle_commit and apply_handle_stream_commit.
  */
 static void
-apply_handle_commit_internal(StringInfo s, LogicalRepCommitData *commit_data)
+apply_handle_commit_internal(LogicalRepCommitData *commit_data)
 {

This looks like a separate change which remove unused paramater in existing
code, maybe we can get this committed first ?

Best regards,
Houzj