On Thu, May 27, 2021 at 8:58 PM vignesh C <vignes...@gmail.com> wrote:
> Thanks for the updated patch, few comments: > 1) I'm not sure if we could add some tests for skip empty > transactions, if possible add a few tests. > Added a few tests for prepared transactions as well as the existing test in 020_messages.pl also tests regular transactions. > 2) We could add some debug level log messages for the transaction that > will be skipped. Added. > > 3) You could keep this variable below the other bool variables in the > structure: > + bool sent_begin_txn; /* flag indicating whether begin > + > * has already been sent */ > + I've moved this variable around, so this comment no longer is valid. > > 4) You can split the comments to multi-line as it exceeds 80 chars > + /* output BEGIN if we haven't yet, avoid for streaming and > non-transactional messages */ > + if (!data->sent_begin_txn && !in_streaming && transactional) > + pgoutput_begin(ctx, txn); Done. I've had to rebase the patch after a recent commit by Amit Kapila of supporting two-phase commits in pub-sub [1]. Also I've modified the patch to also skip replicating empty prepared transactions. Do let me know if you have any comments. regards, Ajin Cherian Fujitsu Australia [1]- https://www.postgresql.org/message-id/CAHut+PueG6u3vwG8DU=JhJiWa2TwmZ=bdqpchzkbky7ykza...@mail.gmail.com
v7-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
