On Tuesday, March 29, 2022 3:20 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > Some comments:
Thanks for the comments!
>
> + if (skipped_xact &&
> + SyncRepRequested() &&
> + ((volatile WalSndCtlData *)
> WalSndCtl)->sync_standbys_defined)
> + {
> + WalSndKeepalive(false, ctx->write_location);
>
> I think we can use 'lsn' since it is actually ctx->write_location.
Agreed, and changed.
> ---
> + if (!sent_begin_txn)
> + {
> + elog(DEBUG1, "Skipped replication of an empty
> transaction with XID: %u", txn->xid);
> + return;
> + }
>
> The log message should start with lowercase.
Changed.
> ---
> +# Note that the current location of the log file is not grabbed
> +immediately # after reloading the configuration, but after sending one
> +SQL command to # the node so as we are sure that the reloading has taken
> effect.
> +$log_location = -s $node_subscriber->logfile;
> +
> +$node_publisher->safe_psql('postgres', "INSERT INTO tab_notrep VALUES
> +(11)");
> +
> +$node_publisher->wait_for_catchup('tap_sub');
> +
> +$logfile = slurp_file($node_publisher->logfile, $log_location);
>
> I think we should get the log location of the publisher node, not subscriber
> node.
Changed.
Attach the new version patch which addressed the
above comments and slightly adjusted some code comments.
Best regards,
Hou zj
v33-0001-Skip-empty-transactions-for-logical-replication.patch
Description: v33-0001-Skip-empty-transactions-for-logical-replication.patch
