On Wed, Mar 30, 2022 at 7:15 AM Masahiko Sawada wrote:
>
> On Tue, Mar 29, 2022 at 6:15 PM houzj.f...@fujitsu.com
> wrote:
> >
> > Thanks for the comment.
> > Attach the new version patch with this change.
> >
>
> Thank you for updating the patch. Looks good to me.
>
Pushed.
--
With Regards,
A
On Tue, Mar 29, 2022 at 6:15 PM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, March 29, 2022 5:12 PM Amit Kapila
> wrote:
> >
> > On Tue, Mar 29, 2022 at 2:05 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > Attach the new version patch which addressed the above comments and
> > > slightly adju
On Tue, Mar 29, 2022 5:15 PM Hou, Zhijie/侯 志杰 wrote:
>
> Thanks for the comment.
> Attach the new version patch with this change.
>
Hi,
I did a performance test for this patch to see if it affects performance when
publishing empty transactions, based on the v32 patch.
In this test, I use sync
On Tuesday, March 29, 2022 5:12 PM Amit Kapila wrote:
>
> On Tue, Mar 29, 2022 at 2:05 PM houzj.f...@fujitsu.com
> wrote:
> >
> > Attach the new version patch which addressed the above comments and
> > slightly adjusted some code comments.
> >
>
> The patch looks good to me. One minor suggestio
On Tue, Mar 29, 2022 at 2:05 PM houzj.f...@fujitsu.com
wrote:
>
> Attach the new version patch which addressed the
> above comments and slightly adjusted some code comments.
>
The patch looks good to me. One minor suggestion is to change the
function name ProcessPendingWritesAndTimeOut() to
Proce
On Tuesday, March 29, 2022 3:20 PM Masahiko Sawada
wrote:
>
> Some comments:
Thanks for the comments!
>
> + if (skipped_xact &&
> + SyncRepRequested() &&
> + ((volatile WalSndCtlData *)
> WalSndCtl)->sync_standbys_defined)
> + {
> + WalSnd
On Mon, Mar 28, 2022 at 9:22 PM houzj.f...@fujitsu.com
wrote:
>
> On Monday, March 28, 2022 3:08 PM Amit Kapila wrote:
> >
> > On Fri, Mar 25, 2022 at 12:50 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > Attach the new version patch with this change.
> > >
> >
> > Few comments:
>
> Thanks for
On Monday, March 28, 2022 3:08 PM Amit Kapila wrote:
>
> On Fri, Mar 25, 2022 at 12:50 PM houzj.f...@fujitsu.com
> wrote:
> >
> > Attach the new version patch with this change.
> >
>
> Few comments:
Thanks for the comments.
> =
> 1. I think we can move the keep_alive check aft
On Fri, Mar 25, 2022 at 12:50 PM houzj.f...@fujitsu.com
wrote:
>
> Attach the new version patch with this change.
>
Few comments:
=
1. I think we can move the keep_alive check after the tracklag record
check to keep it consistent with another patch [1].
2. Add the comment about th
On Friday, March 25, 2022 8:31 AM houzj.f...@fujitsu.com
wrote:
> On Thursday, March 24, 2022 11:19 AM houzj.f...@fujitsu.com wrote:
> > On Tuesday, March 22, 2022 7:50 PM Amit Kapila
>
> > wrote:
> > > On Tue, Mar 22, 2022 at 7:25 AM houzj.f...@fujitsu.com
> > > wrote:
> > > >
> > > > > On Mon
On Thursday, March 24, 2022 11:19 AM houzj.f...@fujitsu.com wrote:
> On Tuesday, March 22, 2022 7:50 PM Amit Kapila
> wrote:
> > On Tue, Mar 22, 2022 at 7:25 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > > On Monday, March 21, 2022 6:01 PM Amit Kapila
> > > >
> > > > wrote:
> > >
> > > Oh, so
On Thursday, March 24, 2022 11:19 AM Hou, Zhijie/侯 志杰
wrote:
>
> Attach the new version patch which include the following changes:
>
> - Fix a typo
> - Change the requestreply flag of the newly added WalSndKeepalive to false,
> because the subscriber can judge whether it's necessary to post a
On Tuesday, March 22, 2022 7:50 PM Amit Kapila wrote:
> On Tue, Mar 22, 2022 at 7:25 AM houzj.f...@fujitsu.com
> wrote:
> >
> > > On Monday, March 21, 2022 6:01 PM Amit Kapila
> > >
> > > wrote:
> >
> > Oh, sorry, I posted the wrong patch, here is the correct one.
> >
>
> The test change looks
On Tue, Mar 22, 2022 at 7:25 AM houzj.f...@fujitsu.com
wrote:
>
> > On Monday, March 21, 2022 6:01 PM Amit Kapila
> > wrote:
>
> Oh, sorry, I posted the wrong patch, here is the correct one.
>
The test change looks good to me. I think additionally we can verify
that the record is not reflected i
> On Monday, March 21, 2022 6:01 PM Amit Kapila
> wrote:
> >
> > On Sat, Mar 19, 2022 at 9:10 AM Ajin Cherian wrote:
> > >
> > > On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila
> > >
> > wrote:
> > >
> > > > 3. Can we add a simple test for it in one of the existing test
> > > > files(say in 001_rep
On Monday, March 21, 2022 6:01 PM Amit Kapila wrote:
>
> On Sat, Mar 19, 2022 at 9:10 AM Ajin Cherian wrote:
> >
> > On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila
> wrote:
> >
> > > 3. Can we add a simple test for it in one of the existing test
> > > files(say in 001_rep_changes.pl)?
> >
> > add
On Sat, Mar 19, 2022 at 9:10 AM Ajin Cherian wrote:
>
> On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila wrote:
>
> > 3. Can we add a simple test for it in one of the existing test
> > files(say in 001_rep_changes.pl)?
>
> added a simple test.
>
This doesn't verify if the transaction is skipped. I t
On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila wrote:
>
> Review comments/suggestions:
> =
> 1. Isn't it sufficient to call pgoutput_send_begin from
> maybe_send_schema as that is commonplace for all others and is always
> the first message we send? If so, I think we can remo
On Wed, Mar 16, 2022 at 12:33 PM Ajin Cherian wrote:
>
> On Mon, Mar 7, 2022 at 11:44 PM Ajin Cherian wrote:
> >
> > Fixed.
> >
Review comments/suggestions:
=
1. Isn't it sufficient to call pgoutput_send_begin from
maybe_send_schema as that is commonplace for all others a
On Mon, Mar 7, 2022 at 11:44 PM Ajin Cherian wrote:
>
> Fixed.
>
> regards,
> Ajin Cherian
> Fujitsu Australia
Rebased the patch and fixed some whitespace errors.
regards,
Ajin Cherian
Fujitsu Australia
v25-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
v2
On Mon, Mar 7, 2022 at 7:50 PM shiy.f...@fujitsu.com
wrote:
>
> On Fri, Mar 4, 2022 9:41 AM Ajin Cherian wrote:
> >
> > I have split the patch into two. I have kept the logic of skipping
> > streaming changes in the second patch.
> > I will work on the second patch once we can figure out a soluti
On Fri, Mar 4, 2022 9:41 AM Ajin Cherian wrote:
>
> I have split the patch into two. I have kept the logic of skipping
> streaming changes in the second patch.
> I will work on the second patch once we can figure out a solution for
> the COMMIT PREPARED after restart problem.
>
Thanks for updat
On Fri, Mar 4, 2022 at 12:41 PM Ajin Cherian wrote:
>
> I have split the patch into two. I have kept the logic of skipping
> streaming changes in the second patch.
> I will work on the second patch once we can figure out a solution for
> the COMMIT PREPARED after restart problem.
>
Please see bel
I have split the patch into two. I have kept the logic of skipping
streaming changes in the second patch.
I will work on the second patch once we can figure out a solution for
the COMMIT PREPARED after restart problem.
regards,
Ajin Cherian
v23-0001-Skip-empty-transactions-for-logical-replicatio
On Wed, Mar 2, 2022 at 1:01 PM shiy.f...@fujitsu.com
wrote:
>
> Hi,
>
> Here are some comments on the v21 patch.
>
> 1.
> + WalSndKeepalive(false, 0);
>
> Maybe we can use InvalidXLogRecPtr here, instead of 0.
>
Fixed.
> 2.
> + pq_sendint64(&output_message, writePtr ?
On Wed, Mar 2, 2022 at 1:01 PM shiy.f...@fujitsu.com
wrote:
>
> 4.
> @@ -1617,9 +1829,21 @@ pgoutput_stream_prepare_txn(LogicalDecodingContext
> *ctx,
> ReorderBufferTXN *txn,
> XLogRec
Hi,
Here are some comments on the v21 patch.
1.
+ WalSndKeepalive(false, 0);
Maybe we can use InvalidXLogRecPtr here, instead of 0.
2.
+ pq_sendint64(&output_message, writePtr ? writePtr : sentPtr);
Similarly, should we use XLogRecPtrIsInvalid()?
3.
@@ -1183,6 +126
On Fri, Feb 25, 2022 at 9:17 PM Peter Smith wrote:
>
> Hi. Here are my review comments for the v19 patch.
>
> ==
>
> 1. Commit message
>
> The current logical replication behavior is to send every transaction to
> subscriber even though the transaction is empty (because it does not
> contain c
On Fri, Feb 18, 2022 at 9:27 PM Amit Kapila wrote:
>
>
> Yeah, I think there could be multiple ways (a) We can send such a keep
> alive in WalSndUpdateProgress() itself by using ctx->write_location.
> For this, we need to modify WalSndKeepalive() to take sentPtr as
> input. (b) set some flag in Wa
Hi. Here are my review comments for the v19 patch.
==
1. Commit message
The current logical replication behavior is to send every transaction to
subscriber even though the transaction is empty (because it does not
contain changes from the selected publications).
SUGGESTION
"to subscriber ev
On Feb, Wed 23, 2022 at 10:58 PM Ajin Cherian wrote:
>
Few comments to V19-0001:
1. I think we should adjust the alignment format.
git am ../v19-0001-Skip-empty-transactions-for-logical-replication.patch
.git/rebase-apply/patch:197: indent with spaces.
* Before we send schema, make sure that
On Thu, Feb 17, 2022 at 9:42 PM Amit Kapila wrote:
>
> On Mon, Jan 31, 2022 at 6:18 PM Ajin Cherian wrote:
> >
>
> Few comments:
> =
> 1. Is there any particular why the patch is not skipping empty xacts
> for streaming (in-progress) transactions as noted in the commit
> message as we
FYI - the latest v18 patch no longer applies due to a recent push [1].
--
[1]
https://github.com/postgres/postgres/commit/52e4f0cd472d39d07732b99559989ea3b615be78
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Feb 18, 2022 at 3:06 PM osumi.takami...@fujitsu.com
wrote:
>
> On Friday, February 18, 2022 6:18 PM Amit Kapila
> wrote:
> > On Tue, Feb 8, 2022 at 5:27 AM osumi.takami...@fujitsu.com
> > wrote:
> > >
> > > On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote:
> > > > On Mon, Aug 2, 2
On Friday, February 18, 2022 6:18 PM Amit Kapila
wrote:
> On Tue, Feb 8, 2022 at 5:27 AM osumi.takami...@fujitsu.com
> wrote:
> >
> > On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote:
> > > On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila
> > > wrote:
> > Changing the timing to send the keepali
On Tue, Feb 8, 2022 at 5:27 AM osumi.takami...@fujitsu.com
wrote:
>
> On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote:
> > On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila
> > wrote:
> Changing the timing to send the keepalive to the decoding commit
> timing didn't look impossible to me, althoug
On Thu, Feb 17, 2022 at 4:12 PM Amit Kapila wrote:
>
> On Mon, Jan 31, 2022 at 6:18 PM Ajin Cherian wrote:
> >
>
> Few comments:
> =
>
One more comment:
@@ -1546,10 +1557,11 @@ WalSndWaitForWal(XLogRecPtr loc)
* otherwise idle, this keepalive will trigger a reply. Processing the
On Wed, Feb 16, 2022 at 2:15 PM osumi.takami...@fujitsu.com
wrote:
> Another idea would be, to create an empty file under the the
> pg_replslot/slotname
> with a prefix different from "xid" in the DecodePrepare before the shutdown
> if the prepare was empty, and bypass the cleanup of the seriali
On Wed, Feb 16, 2022 at 8:45 AM osumi.takami...@fujitsu.com
wrote:
[ideas to skip empty prepare/commit_prepare ]
>
> I feel if we don't want to change the protocol of commit_prepared,
> we need to make the publisher solely judge whether the prepare was empty or
> not,
> after the restart.
>
On Mon, Jan 31, 2022 at 6:18 PM Ajin Cherian wrote:
>
Few comments:
=
1. Is there any particular why the patch is not skipping empty xacts
for streaming (in-progress) transactions as noted in the commit
message as well?
2.
+static void
+pgoutput_begin(LogicalDecodingContext *ctx, Reo
Hi
I'll quote one other remaining discussion of this thread again
to invoke more attentions from the community.
On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote:
> On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila
> wrote:
> > Few other miscellaneous comments:
> > 1.
> > static void
> > pgoutput
Hi,
Thank you for your updating the patch.
I'll quote one of the past discussions
in order to make this thread go forward or more active.
On Friday, August 13, 2021 8:01 PM Ajin Cherian wrote:
> On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila
> wrote:
> >
> > On Fri, Jul 23, 2021 at 3:39 PM Ajin Ch
On Sun, Jan 30, 2022 at 7:04 PM osumi.takami...@fujitsu.com
wrote:
>
> On Thursday, January 27, 2022 9:57 PM Ajin Cherian wrote:
> Hi, thanks for your patch update.
>
>
> > On Wed, Jan 26, 2022 at 8:33 PM osumi.takami...@fujitsu.com
> > wrote:
> > >
> > > On Tuesday, January 11, 2022 6:43 PM Aji
On Thursday, January 27, 2022 9:57 PM Ajin Cherian wrote:
Hi, thanks for your patch update.
> On Wed, Jan 26, 2022 at 8:33 PM osumi.takami...@fujitsu.com
> wrote:
> >
> > On Tuesday, January 11, 2022 6:43 PM Ajin Cherian
> wrote:
> > (3) Is this patch's reponsibility to intialize the data in
>
On Thu, Jan 27, 2022 at 12:16 AM osumi.takami...@fujitsu.com
wrote:
>
> On Tuesday, January 11, 2022 6:43 PM From: Ajin Cherian
> wrote:
> > Minor update to rebase the patch so that it applies clean on HEAD.
> Hi, let me share some additional comments on v16.
>
>
> (1) comment of pgoutput_change
On Wed, Jan 26, 2022 at 8:33 PM osumi.takami...@fujitsu.com
wrote:
>
> On Tuesday, January 11, 2022 6:43 PM Ajin Cherian wrote:
> > Minor update to rebase the patch so that it applies clean on HEAD.
> Hi, thanks for you rebase.
>
> Several comments.
>
> (1) the commit message
>
> "
> transactions
On Tuesday, January 11, 2022 6:43 PM From: Ajin Cherian
wrote:
> Minor update to rebase the patch so that it applies clean on HEAD.
Hi, let me share some additional comments on v16.
(1) comment of pgoutput_change
@@ -630,11 +688,15 @@ pgoutput_change(LogicalDecodingContext *ctx,
ReorderBuffer
On Tuesday, January 11, 2022 6:43 PM Ajin Cherian wrote:
> Minor update to rebase the patch so that it applies clean on HEAD.
Hi, thanks for you rebase.
Several comments.
(1) the commit message
"
transactions, keepalive messages are sent to keep the LSN locations updated
on the standby.
This pa
On Wed, Sep 1, 2021 at 8:57 PM Ajin Cherian wrote:
>
> Thanks for the comments. Addressed them in the attached patch.
>
> regards,
> Ajin Cherian
> Fujitsu Australia
Minor update to rebase the patch so that it applies clean on HEAD.
regards,
Ajin Cherian
regards,
Ajin Cherian
v16-0001-Skip-em
On Wed, Aug 25, 2021 at 5:15 PM Peter Smith 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.
>
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 LogicalOu
On Mon, Aug 16, 2021 at 4:44 PM Peter Smith wrote:
> I have reviewed the v13-0001 patch.
>
> Apply / build / test was all OK
>
> Below are my code review comments.
>
> //
>
> Comments for v13-0001
> =
>
> 1. Patch comment
>
> =>
>
> Probably this comment should include
On Fri, Aug 13, 2021 at 9:01 PM Ajin Cherian wrote:
>
> On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila wrote:
> >
> > On Fri, Jul 23, 2021 at 3:39 PM Ajin Cherian wrote:
> > >
> >
> > Let's first split the patch for prepared and non-prepared cases as
> > that will help to focus on each of them separ
On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila wrote:
>
> On Fri, Jul 23, 2021 at 3:39 PM Ajin Cherian wrote:
> >
>
> Let's first split the patch for prepared and non-prepared cases as
> that will help to focus on each of them separately. BTW, why haven't
> you considered implementing point 1b as exp
On Sat, Aug 7, 2021 at 12:01 AM Ajin Cherian wrote:
>
> On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila wrote:
> >
> > On Fri, Jul 23, 2021 at 3:39 PM Ajin Cherian wrote:
> > >
> >
> > Let's first split the patch for prepared and non-prepared cases as
> > that will help to focus on each of them separ
On Mon, Aug 2, 2021 at 7:20 PM Amit Kapila wrote:
>
> On Fri, Jul 23, 2021 at 3:39 PM Ajin Cherian wrote:
> >
>
> Let's first split the patch for prepared and non-prepared cases as
> that will help to focus on each of them separately.
As a first shot, I have split the patch into prepared and non
On Fri, Jul 23, 2021 at 3:39 PM Ajin Cherian wrote:
>
Let's first split the patch for prepared and non-prepared cases as
that will help to focus on each of them separately. BTW, why haven't
you considered implementing point 1b as explained by Andres in his
email [1]? I think we can send a keepali
On Friday, July 23, 2021 7:10 PM Ajin Cherian wrote:
> On Fri, Jul 23, 2021 at 7:38 PM Peter Smith wrote:
> >
> > I have reviewed the v10 patch.
The patch v11 looks good to me as well.
Thanks for addressing my past comments.
Best Regards,
Takamichi Osumi
On Fri, Jul 23, 2021 at 8:09 PM Ajin Cherian wrote:
>
> fixed.
The v11 patch LGTM.
Regards,
Greg Nancarrow
Fujitsu Australia
FYI - I have checked the v11 patch. Everything applies, builds, and
tests OK for me, and I have no more review comments. So v11 LGTM.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Jul 23, 2021 at 7:38 PM Peter Smith wrote:
>
> I have reviewed the v10 patch.
>
> Apply / build / test was all OK.
>
> Just one review comment:
>
> //
>
> 1. Typo
>
> @@ -130,6 +132,17 @@ typedef struct RelationSyncEntry
> TupleConversionMap *map;
> } RelationSyncEntry;
>
> +/*
I have reviewed the v10 patch.
Apply / build / test was all OK.
Just one review comment:
//
1. Typo
@@ -130,6 +132,17 @@ typedef struct RelationSyncEntry
TupleConversionMap *map;
} RelationSyncEntry;
+/*
+ * Maintain a per-transaction level variable to track whether the
+ * transac
On Fri, Jul 23, 2021 at 10:13 AM Peter Smith wrote:
>
> I have reviewed the v9 patch and my feedback comments are below:
>
> //
>
> 1. Apply v9 gave multiple whitespace warnings
Fixed.
>
> --
>
> 2. Commit comment - wording
>
> pgoutput will also skip COMMIT PREPARED and ROLLBACK PRE
On Fri, Jul 23, 2021 at 10:26 AM Greg Nancarrow wrote:
>
> On Thu, Jul 22, 2021 at 11:37 PM Ajin Cherian wrote:
> >
>
> I have some minor comments on the v9 patch:
>
> (1) Several whitespace warnings on patch application
>
Fixed.
> (2) Suggested patch comment change:
>
> BEFORE:
> The current l
On Thu, Jul 22, 2021 at 11:37 PM Ajin Cherian wrote:
>
I have some minor comments on the v9 patch:
(1) Several whitespace warnings on patch application
(2) Suggested patch comment change:
BEFORE:
The current logical replication behaviour is to send every transaction to
subscriber even though t
I have reviewed the v9 patch and my feedback comments are below:
//
1. Apply v9 gave multiple whitespace warnings
$ git apply v9-0001-Skip-empty-transactions-for-logical-replication.patch
v9-0001-Skip-empty-transactions-for-logical-replication.patch:479:
indent with spaces.
* If the
On Thu, Jul 22, 2021 at 6:11 PM Peter Smith wrote:
>
> Hi Ajin.
>
> I have reviewed the v8 patch and my feedback comments are below:
>
> //
>
> 1. Apply v8 gave multiple whitespace warnings.
>
> --
>
> 2. Commit comment - wording
>
> If (when processing a COMMIT / PREPARE message) we f
Hi Ajin.
I have reviewed the v8 patch and my feedback comments are below:
//
1. Apply v8 gave multiple whitespace warnings.
--
2. Commit comment - wording
If (when processing a COMMIT / PREPARE message) we find there had been
no other change for that transaction, then do not send
On Thu, Jul 15, 2021 at 3:50 PM osumi.takami...@fujitsu.com
wrote:
> I started to test this patch but will give you some really minor quick
> feedbacks.
>
> (1) pg_logical_slot_get_binary_changes() params.
>
> Technically, looks better to have proto_version 3 & two_phase option for the
> functio
On Mon, Jul 19, 2021 at 3:24 PM Peter Smith wrote:
> 1a. Commit Comment - wording
>
updated.
>
> 1b. Commit Comment - wording
>
updated.
> 2. doc/src/sgml/logicaldecoding.sgml - wording
>
> @@ -884,11 +884,19 @@ typedef void (*LogicalDecodePrepareCB) (struct
> LogicalDecodingContext *ctx,
>
Hi Ajin,
I have reviewed the v7 patch and given my feedback comments below.
Apply OK
Build OK
make check OK
TAP (subscriptions) make check OK
Build PG Docs (html) OK
Although I made lots of review comments below, the important point is
that none of them are functional - they are only minore re-w
On Wednesday, July 14, 2021 9:30 PM Ajin Cherian wrote:
> 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 com
On Thu, May 27, 2021 at 8:58 PM vignesh C 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 als
On Tue, May 25, 2021 at 6:36 PM Ajin Cherian wrote:
>
> On Tue, Apr 27, 2021 at 1:49 PM Ajin Cherian wrote:
>
> Rebased the patch as it was no longer applying.
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 fe
On Tue, Apr 27, 2021 at 1:49 PM Ajin Cherian wrote:
Rebased the patch as it was no longer applying.
regards,
Ajin Cherian
Fujitsu Australia
v6-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
On Mon, Apr 26, 2021 at 4:29 PM Peter Smith wrote:
> The v4 patch applied cleanly.
>
> make check-world completed successfully.
>
> So this patch v4 looks LGTM, apart from the following 2 nitpick comments:
>
> ==
>
> 1. Suggest to add a blank line after the (void)txn; ?
>
> @@ -345,10 +345,29
On Fri, Apr 23, 2021 at 3:46 PM Ajin Cherian wrote:
>
>
>
> On Mon, Apr 19, 2021 at 6:22 PM Peter Smith wrote:
>>
>>
>> Here are a some review comments:
>>
>> --
>>
>> 1. The patch v3 applied OK but with whitespace warnings
>>
>> [postgres@CentOS7-x64 oss_postgres_2PC]$ git apply
>> ../patche
An earlier comment from Anders:
> We could e.g. have a new LogicalDecodingContext callback that is
> called whenever WalSndWaitForWal() would wait. That'd check if there's
> a pending "need" to send out a 'empty transaction'/feedback request
> message. The "need" flag would get cleared whenever we
On Mon, Apr 19, 2021 at 6:22 PM Peter Smith wrote:
>
> Here are a some review comments:
>
> --
>
> 1. The patch v3 applied OK but with whitespace warnings
>
> [postgres@CentOS7-x64 oss_postgres_2PC]$ git apply
>
> ../patches_misc/v3-0001-Skip-empty-transactions-for-logical-replication.patch
>
On Thu, Apr 15, 2021 at 4:39 PM Ajin Cherian wrote:
>
>
>
> On Thu, Apr 15, 2021 at 1:29 PM Ajin Cherian wrote:
>>
>>
>> I've rebased the patch and made changes so that the patch supports
>> "streaming in-progress transactions" and handling of logical decoding
>> messages (transactional and non-
On Thu, Apr 15, 2021 at 1:29 PM Ajin Cherian wrote:
>
> I've rebased the patch and made changes so that the patch supports
> "streaming in-progress transactions" and handling of logical decoding
> messages (transactional and non-transactional).
> I see that this patch not only makes sure that emp
On Thu, Sep 17, 2020 at 3:29 PM Michael Paquier wrote:
> On Wed, Jul 29, 2020 at 08:08:06PM +0530, Rahila Syed wrote:
> > The make check passes.
>
> Since then, the patch is failing to apply, waiting on author and the
> thread has died 6 weeks or so ago, so I am marking it as RwF in the
> CF.
>
>
On Wed, Jul 29, 2020 at 08:08:06PM +0530, Rahila Syed wrote:
> The make check passes.
Since then, the patch is failing to apply, waiting on author and the
thread has died 6 weeks or so ago, so I am marking it as RwF in the
CF.
--
Michael
signature.asc
Description: PGP signature
Hi,
Please see below review of the
0001-Skip-empty-transactions-for-logical-replication.patch
The make check passes.
+ /* output BEGIN if we haven't yet */
+ if (!data->xact_wrote_changes)
+ pgoutput_begin(ctx, txn);
+
+ da
Sorry, I replied in the wrong thread. Please ignore above mail.
>
>
The patch no longer applies, because of additions in the test source.
Otherwise, I have tested the patch and confirmed that updates and deletes on
tables with deferred primary keys work with logical replication.
The new status of this patch is: Waiting on Author
On Tue, 10 Mar 2020 at 02:30, Andres Freund wrote:
> Hi,
>
> On 2020-03-06 13:53:02 +0800, Craig Ringer wrote:
> > On Mon, 2 Mar 2020 at 19:26, Amit Kapila
> wrote:
> >
> > > One thing that is not clear to me is how will we advance restart_lsn
> > > if we don't send any empty xact in a system wh
Hi,
On 2020-03-06 13:53:02 +0800, Craig Ringer wrote:
> On Mon, 2 Mar 2020 at 19:26, Amit Kapila wrote:
>
> > One thing that is not clear to me is how will we advance restart_lsn
> > if we don't send any empty xact in a system where there are many such
> > xacts?
>
> Same way we already do it f
On Mon, 2 Mar 2020 at 19:26, Amit Kapila wrote:
> One thing that is not clear to me is how will we advance restart_lsn
> if we don't send any empty xact in a system where there are many such
> xacts?
Same way we already do it for writes that are not replicated over
logical replication, like vacu
On Thu, 5 Mar 2020 at 05:45, Amit Kapila wrote:
> Euler, can we try to update the patch based on the number of
> transactions threshold and see how it works?
>
> I will do.
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training &
On Wed, Mar 4, 2020 at 4:04 PM Dilip Kumar wrote:
>
> On Wed, Mar 4, 2020 at 3:47 PM Amit Kapila wrote:
> >
> > On Wed, Mar 4, 2020 at 11:16 AM Dilip Kumar wrote:
> > >
> > > On Wed, Mar 4, 2020 at 10:50 AM Amit Kapila
> > > wrote:
> > > >
> > > > On Wed, Mar 4, 2020 at 9:52 AM Dilip Kumar
>
On Wed, Mar 4, 2020 at 3:47 PM Amit Kapila wrote:
>
> On Wed, Mar 4, 2020 at 11:16 AM Dilip Kumar wrote:
> >
> > On Wed, Mar 4, 2020 at 10:50 AM Amit Kapila wrote:
> > >
> > > On Wed, Mar 4, 2020 at 9:52 AM Dilip Kumar wrote:
> > > >
> > > >
> > > > IMHO, the threshold should be based on the co
On Wed, Mar 4, 2020 at 11:16 AM Dilip Kumar wrote:
>
> On Wed, Mar 4, 2020 at 10:50 AM Amit Kapila wrote:
> >
> > On Wed, Mar 4, 2020 at 9:52 AM Dilip Kumar wrote:
> > >
> > >
> > > IMHO, the threshold should be based on the commit LSN. Our main
> > > reason we want to send empty transactions a
On Wed, Mar 4, 2020 at 10:50 AM Amit Kapila wrote:
>
> On Wed, Mar 4, 2020 at 9:52 AM Dilip Kumar wrote:
> >
> > On Wed, Mar 4, 2020 at 9:12 AM Amit Kapila wrote:
> > >
> > > On Wed, Mar 4, 2020 at 7:17 AM Euler Taveira
> > > wrote:
> > > >
> > > > On Tue, 3 Mar 2020 at 05:24, Amit Kapila
> >
On Wed, Mar 4, 2020 at 9:52 AM Dilip Kumar wrote:
>
> On Wed, Mar 4, 2020 at 9:12 AM Amit Kapila wrote:
> >
> > On Wed, Mar 4, 2020 at 7:17 AM Euler Taveira
> > wrote:
> > >
> > > On Tue, 3 Mar 2020 at 05:24, Amit Kapila wrote:
> > >>
> > >>
> > >> Another idea could be that we stream the trans
On Wed, Mar 4, 2020 at 9:12 AM Amit Kapila wrote:
>
> On Wed, Mar 4, 2020 at 7:17 AM Euler Taveira
> wrote:
> >
> > On Tue, 3 Mar 2020 at 05:24, Amit Kapila wrote:
> >>
> >>
> >> Another idea could be that we stream the transaction after some
> >> threshold number (say 100 or anything we think i
On Wed, Mar 4, 2020 at 7:17 AM Euler Taveira
wrote:
>
> On Tue, 3 Mar 2020 at 05:24, Amit Kapila wrote:
>>
>>
>> Another idea could be that we stream the transaction after some
>> threshold number (say 100 or anything we think is reasonable) of empty
>> xacts. This will reduce the traffic withou
On Tue, 3 Mar 2020 at 05:24, Amit Kapila wrote:
>
> Another idea could be that we stream the transaction after some
> threshold number (say 100 or anything we think is reasonable) of empty
> xacts. This will reduce the traffic without tinkering with the core
> design too much.
>
>
> Amit, I sugg
On Tue, Mar 3, 2020 at 2:17 PM Dilip Kumar wrote:
>
> On Tue, Mar 3, 2020 at 1:54 PM Amit Kapila wrote:
> >
> > On Tue, Mar 3, 2020 at 9:35 AM Dilip Kumar wrote:
> > >
> > > On Mon, Mar 2, 2020 at 4:56 PM Amit Kapila
> > > wrote:
> > > >
> > > >
> > > > One thing that is not clear to me is how
On Tue, Mar 3, 2020 at 1:54 PM Amit Kapila wrote:
>
> On Tue, Mar 3, 2020 at 9:35 AM Dilip Kumar wrote:
> >
> > On Mon, Mar 2, 2020 at 4:56 PM Amit Kapila wrote:
> > >
> > >
> > > One thing that is not clear to me is how will we advance restart_lsn
> > > if we don't send any empty xact in a syst
1 - 100 of 107 matches
Mail list logo
