Horton

Re: BUG #16643: PG13 - Logical replication - initial startup never finishes and gets stuck in startup loop

Peter Smith <smithpb2250@gmail.com>

View thread View on postgr.es
From: Peter Smith <smithpb2250@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Petr Jelinek <petr@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>, Henry Hinze <henry.hinze@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Date: 2020-11-27T01:59:28Z
Lists: pgsql-bugs 
On Thu, Nov 26, 2020 at 11:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

> Okay, I see your point, so changed it accordingly and fixed the code
> comments as suggested by you. Does this address all your
> concerns/suggestions?

Yes. Now the v4 patch addresses everything I previously mentioned.

However, today I did notice one more small thing you may want to change.

===

REVIEW COMMENT:

(1) missing comment?

@@ -928,7 +917,9 @@ apply_handle_stream_abort(StringInfo s)

  /* write the updated subxact list */
  subxact_info_write(MyLogicalRepWorker->subid, xid);
- CommitTransactionCommand();
+
+ if (!am_tablesync_worker())
+ CommitTransactionCommand();
  }
 }

That new condition seems to be missing a comment saying "/* The
synchronization worker runs in a single transaction */". Such a
comment accompanies all other am_tablesync_worker() conditions that
look like this one.

===

I also re-ran the tablesync test using the v4 patch, and have stepped
to see the normal stream file normal cleanup being executed by the
tablesync worker.

So apart from that trivial missing comment, I think now it is all good.

---

Kind Regards,
Peter Smith.
Fujitsu Australia

Commits

  1. Fix replication of in-progress transactions in tablesync worker.

  2. Review logical replication tablesync code

  3. Restore replication protocol's duplicate command tags