On Fri, Jul 30, 2021 at 4:33 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > > On Fri, Jul 30, 2021 at 2:02 PM Peter Smith <smithpb2...@gmail.com> wrote: > > > > Please find attached the latest patch set v100* > > > > v99-0002 --> v100-0001 > > > > A few minor comments: > > (1) doc/src/sgml/protocol.sgml > > In the following description, is the word "large" really needed? Also > "the message ... for a ... message" sounds a bit odd, as does > "two-phase prepare". > > What about the following: > > BEFORE: > + Identifies the message as a two-phase prepare for a > large in-progress transaction message. > AFTER: > + Identifies the message as a prepare for an > in-progress two-phase transaction. >
Updated in v101. The other nearby messages are referring refer to a “streamed transaction” so I’ve changed this to say “Identifies the message as a two-phase prepare for a streamed transaction message.” (e.g. compare this text with the existing similar text for ‘P’). BTW, I agree with you that "the message ... for a ... message" seems odd; it was written in this way only to be consistent with existing documentation, which all uses the same odd phrasing. > (2) src/backend/replication/logical/worker.c > > Similar format comment, but one uses a full-stop and the other > doesn't, looks a bit odd, since the lines are near each other. > > * 1. Replay all the spooled operations - Similar code as for > > * 2. Mark the transaction as prepared. - Similar code as for > Updated in v101 to make the comments consistent. > (3) src/test/subscription/t/023_twophase_stream.pl > > Shouldn't the following comment mention, for example, "with streaming" > or something to that effect? > > # logical replication of 2PC test > Fixed as suggested in v101. ------ Kind Regards, Peter Smith. Fujitsu Australia.
