RE: Skipping logical replication transactions on subscriber side

From Mon, Aug 30, 2021 3:07 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I've attached rebased patches. 0004 patch is not the scope of this patch. It's
> borrowed from another thread[1] to fix the assertion failure for newly added
> tests. Please review them.

Hi,

I reviewed the v12-0001 patch, here are some comments:

1)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1441,7 +1441,6 @@ getinternalerrposition(void)
     return edata->internalpos;
 }
 
-

It seems a miss change in elog.c

2)

+    TupleDescInitEntry(tupdesc, (AttrNumber) 10, "stats_reset",
+                       TIMESTAMPTZOID, -1, 0);

The document doesn't mention the column "stats_reset".

3)

+typedef struct PgStat_StatSubErrEntry
+{
+    Oid            subrelid;        /* InvalidOid if the apply worker, otherwise
+                                 * the table sync worker. hash table key. */

From the comments of subrelid, I think one subscription only have one apply
worker error entry, right ? If so, I was thinking can we move the the apply
error entry to PgStat_StatSubEntry. In that approach, we don't need to build a
inner hash table when there are no table sync error entry.

4)
Is it possible to add some testcases to test the subscription error entry delete ?


Best regards,
Hou zj