On Wed, Jul 14, 2021 at 2:03 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Wed, Jul 14, 2021 at 4:23 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Jul 12, 2021 at 9:14 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > On Sun, Jul 11, 2021 at 8:20 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > > > > > On Fri, Jul 9, 2021 at 4:43 AM Peter Smith <smithpb2...@gmail.com> > > > > wrote: > > > > > > > > > > > The patch looks good to me, I don't have any comments. > > > > > > > > > > I tried the v95-0001 patch. > > > > > > > > > > - The patch applied cleanly and all build / testing was OK. > > > > > - The documentation also builds OK. > > > > > - I checked all v95-0001 / v93-0001 differences and found no problems. > > > > > - Furthermore, I noted that v95-0001 patch is passing the cfbot [1]. > > > > > > > > > > So this patch LGTM. > > > > > > > > > > > > > Thanks, I took another pass over it and made a few changes in docs and > > > > comments. I am planning to push this next week sometime (by 14th July) > > > > unless there are more comments from you or someone else. Just to > > > > summarize, this patch will add support for prepared transactions to > > > > built-in logical replication. To add support for streaming > > > > transactions at prepare time into the > > > > built-in logical replication, we need to do the following things: (a) > > > > Modify the output plugin (pgoutput) to implement the new two-phase API > > > > callbacks, by leveraging the extended replication protocol. (b) Modify > > > > the replication apply worker, to properly handle two-phase > > > > transactions by replaying them on prepare. (c) Add a new SUBSCRIPTION > > > > option "two_phase" to allow users to enable > > > > two-phase transactions. We enable the two_phase once the initial data > > > > sync is over. Refer to comments atop worker.c in the patch and commit > > > > message to see further details about this patch. After this patch, > > > > there is a follow-up patch to allow streaming and two-phase options > > > > together which I feel needs some more review and can be committed > > > > separately. > > > > > > > > > > FYI - I repeated the same verification of the v96-0001 patch as I did > > > previously for v95-0001 > > > > > > - The v96 patch applied cleanly and all build / testing was OK. > > > - The documentation also builds OK. > > > - I checked the v95-0001 / v96-0001 differences and found no problems. > > > - Furthermore, I noted that v96-0001 patch is passing the cfbot. > > > > > > LGTM. > > > > > > > Pushed. > > > > Feel free to submit the remaining patches after rebase. Is it possible > > to post patches related to skipping empty transactions in the other > > thread [1] where that topic is being discussed? > > > > [1] - > > https://www.postgresql.org/message-id/CAMkU%3D1yohp9-dv48FLoSPrMqYEyyS5ZWkaZGD41RJr10xiNo_Q%40mail.gmail.com > > > > > Please find attached the latest patch set v97* > > * Rebased v94* to HEAD @ today.
Thanks for the updated patch, the patch applies cleanly and test passes: I had couple of comments: 1) Should we include "stream_prepare_cb" here in logicaldecoding-streaming section of logicaldecoding.sgml documentation: To reduce the apply lag caused by large transactions, an output plugin may provide additional callback to support incremental streaming of in-progress transactions. There are multiple required streaming callbacks (stream_start_cb, stream_stop_cb, stream_abort_cb, stream_commit_cb and stream_change_cb) and two optional callbacks (stream_message_cb and stream_truncate_cb). 2) Should we add an example for stream_prepare_cb here in logicaldecoding-streaming section of logicaldecoding.sgml documentation: One example sequence of streaming callback calls for one transaction may look like this: stream_start_cb(...); <-- start of first block of changes stream_change_cb(...); stream_change_cb(...); stream_message_cb(...); stream_change_cb(...); ... stream_change_cb(...); stream_stop_cb(...); <-- end of first block of changes stream_start_cb(...); <-- start of second block of changes stream_change_cb(...); stream_change_cb(...); stream_change_cb(...); ... stream_message_cb(...); stream_change_cb(...); stream_stop_cb(...); <-- end of second block of changes stream_commit_cb(...); <-- commit of the streamed transaction Regards, Vignesh
