On Wed, Aug 25, 2021 at 5:15 PM Peter Smith <smithpb2...@gmail.com> wrote: > > I reviewed the v14-0001 patch. > > All my previous comments have been addressed. > > Apply / build / test was all OK. > > ------ > > More review comments: > > 1. Params names in the function declarations should match the rest of the > code. > > 1a. src/include/replication/logical.h > > @@ -26,7 +26,8 @@ typedef LogicalOutputPluginWriterWrite > LogicalOutputPluginWriterPrepareWrite; > > typedef void (*LogicalOutputPluginWriterUpdateProgress) (struct > LogicalDecodingContext *lr, > XLogRecPtr Ptr, > - TransactionId xid > + TransactionId xid, > + bool send_keep_alive > > => > Change "send_keep_alive" --> "send_keepalive" > > ~~ > > 1b. src/include/replication/output_plugin.h > > @@ -243,6 +243,6 @@ typedef struct OutputPluginCallbacks > /* Functions in replication/logical/logical.c */ > extern void OutputPluginPrepareWrite(struct LogicalDecodingContext > *ctx, bool last_write); > extern void OutputPluginWrite(struct LogicalDecodingContext *ctx, > bool last_write); > -extern void OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx); > +extern void OutputPluginUpdateProgress(struct LogicalDecodingContext > *ctx, bool send_keep_alive); > > => > Change "send_keep_alive" --> "send_keepalive" > > ------ > > 2. Comment should be capitalized - src/backend/replication/walsender.c > > @@ -170,6 +170,9 @@ static TimestampTz last_reply_timestamp = 0; > /* Have we sent a heartbeat message asking for reply, since last reply? */ > static bool waiting_for_ping_response = false; > > +/* force keep alive when skipping transactions in synchronous > replication mode */ > +static bool force_keepalive_syncrep = false; > > => > "force" --> "Force" > > ------ > > Otherwise, v14-0001 LGTM. >
Thanks for the comments. Addressed them in the attached patch. regards, Ajin Cherian Fujitsu Australia
v15-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
