On Mon, 13 Mar 2023 at 08:17, wangw.f...@fujitsu.com
wrote:
>
> On Fri, Mar 10, 2023 20:17 PM Osumi, Takamichi/大墨 昂道
> wrote:
> > Hi,
> >
> >
> > On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威
> > wrote:
> > > Attach the new patch set.
> > Thanks for updating the patch ! One review comment on
On Fri, Mar 10, 2023 20:17 PM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威
> wrote:
> > Attach the new patch set.
> Thanks for updating the patch ! One review comment on v7-0005.
Thanks for your comment.
> stream_start_cb_wrapper and
Hi,
On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威 wrote:
> Attach the new patch set.
Thanks for updating the patch ! One review comment on v7-0005.
stream_start_cb_wrapper and stream_stop_cb_wrapper don't call the pair of
threshold check and UpdateProgressAndKeepalive unlike other write
On Mon, Mar 10, 2023 14:35 PM Amit Kapila wrote:
> On Fri, Mar 10, 2023 at 11:17 AM Peter Smith wrote:
> >
> > On Fri, Mar 10, 2023 at 3:32 PM Amit Kapila wrote:
> > >
> > > On Thu, Mar 9, 2023 at 10:56 AM Peter Smith
> wrote:
> > > >
> > > > 2. rollback_prepared_cb_wrapper
> > > >
> > > >
On Thur, Mar 9, 2023 13:26 PM Peter Smith wrote:
> Here are some review comments for v6-0001
Thanks for your comments.
> ==
> General.
>
> 1.
> There are lots of new comments saying:
> /* don't call update progress, we didn't really make any */
>
> but is the wording "call update
On Wed, Mar 8, 2023 23:55 PM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Wednesday, March 8, 2023 11:54 AM From: wangw.f...@fujitsu.com
> wrote:
> > Attach the new patch.
> Thanks for sharing v6 ! Few minor comments for the same.
Thanks for your comments.
> (1) commit message
>
> The old
On Wed, Mar 8, 2023 19:06 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
Thanks for your testing and comments.
> ---
> ```
> +/*
> + * Update progress tracking and send keep alive (if required).
> + */
> +static void
> +UpdateProgressAndKeepalive(LogicalDecodingContext *ctx, bool finished_xact)
>
On Mon, Mar 10, 2023 11:56 AM Amit Kapila wrote:
> On Wed, Mar 8, 2023 at 8:24 AM wangw.f...@fujitsu.com
> wrote:
> >
> > Attach the new patch.
> >
>
> I think this combines multiple improvements in one patch. We can
> consider all of them together or maybe it would be better to split
> some of
On Fri, Mar 10, 2023 at 11:17 AM Peter Smith wrote:
>
> On Fri, Mar 10, 2023 at 3:32 PM Amit Kapila wrote:
> >
> > On Thu, Mar 9, 2023 at 10:56 AM Peter Smith wrote:
> > >
> > > 2. rollback_prepared_cb_wrapper
> > >
> > > /*
> > > * If the plugin support two-phase commits then rollback
On Fri, Mar 10, 2023 at 3:32 PM Amit Kapila wrote:
>
> On Thu, Mar 9, 2023 at 10:56 AM Peter Smith wrote:
> >
> > 2. rollback_prepared_cb_wrapper
> >
> > /*
> > * If the plugin support two-phase commits then rollback prepared callback
> > * is mandatory
> > + *
> > + * FIXME: This should
On Thu, Mar 9, 2023 at 10:56 AM Peter Smith wrote:
>
> 2. rollback_prepared_cb_wrapper
>
> /*
> * If the plugin support two-phase commits then rollback prepared callback
> * is mandatory
> + *
> + * FIXME: This should have been caught much earlier.
> */
> if
On Wed, Mar 8, 2023 at 8:24 AM wangw.f...@fujitsu.com
wrote:
>
> Attach the new patch.
>
I think this combines multiple improvements in one patch. We can
consider all of them together or maybe it would be better to split
some of those. Do we think it makes sense to split some of the
Here are some review comments for v6-0001
==
General.
1.
There are lots of new comments saying:
/* don't call update progress, we didn't really make any */
but is the wording "call update progress" meaningful?
Should that be written something more like:
/* No progress has been made so
Hi,
On Wednesday, March 8, 2023 11:54 AM From: wangw.f...@fujitsu.com
wrote:
> Attach the new patch.
Thanks for sharing v6 ! Few minor comments for the same.
(1) commit message
The old function name 'is_skip_threshold_change' is referred currently. We need
to update it to
Dear Wang,
Thank you for updating the patch! I have briefly tested your patch
and it worked well in following case.
* WalSndUpdateProgressAndKeepalive is called when many inserts have come
but the publisher does not publish the insertion. PSA the script for this.
*
On Tue, Mar 7, 2023 15:55 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
>
> Thank you for updating the patch! Followings are my comments.
Thanks for your comments.
> ---
> 01. missing comments
> You might miss the comment from Peter[1]. Or could you pin the related one?
Since I think the
Dear Wang,
Thank you for updating the patch! Followings are my comments.
---
01. missing comments
You might miss the comment from Peter[1]. Or could you pin the related one?
---
02. LogicalDecodingProcessRecord()
Don't we have to call UpdateDecodingProgressAndKeepalive() when there is no
On Fri, Mar 3, 2023 8:18 AM Peter Smith wrote:
>
Thanks for your comments.
> 1.
> +
> +static void UpdateProgressAndKeepalive(LogicalDecodingContext *ctx,
> +bool finished_xact);
> +
> +static bool is_keepalive_threshold_exceeded(LogicalDecodingContext *ctx);
>
> 1a.
> There is an
On Fri, Mar 3, 2023 at 1:27 PM houzj.f...@fujitsu.com
wrote:
>
> On Friday, March 3, 2023 8:18 AM Peter Smith wrote:
...
> > Anyway, I think this exposes another problem. If you still want the patch
> > to pass
> > the 'finshed_xact' parameter separately then AFAICT the first parameter
> >
On Friday, March 3, 2023 8:18 AM Peter Smith wrote:
> On Wed, Mar 1, 2023 at 9:16 PM wangw.f...@fujitsu.com
> wrote:
> >
> > On Tues, Feb 28, 2023 at 9:12 AM Peter Smith
> wrote:
> > > Here are some comments for the v2-0001 patch.
> > >
> > > (I haven't looked at the v3 that was posted
Hi,
On 2023-03-03 11:18:04 +1100, Peter Smith wrote:
> - Why is reducing members of LogicalDecodingContext even a goal? I
> thought the LogicalDecodingContext is intended to be the one-stop
> place to hold *all* things related to the "Context" (including that
> member that was deleted).
There's
On Wed, Mar 1, 2023 at 9:16 PM wangw.f...@fujitsu.com
wrote:
>
> On Tues, Feb 28, 2023 at 9:12 AM Peter Smith wrote:
> > Here are some comments for the v2-0001 patch.
> >
> > (I haven't looked at the v3 that was posted overnight; maybe some of
> > my comments have already been addressed.)
>
>
On Tues, Feb 28, 2023 at 11:31 AM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Monday, February 27, 2023 6:30 PM wangw.f...@fujitsu.com
> wrote:
> > Attach the new patch.
> Thanks for sharing v3. Minor review comments and question.
Thanks for your comments.
> (1)
On Tues, Feb 28, 2023 at 9:12 AM Peter Smith wrote:
> Here are some comments for the v2-0001 patch.
>
> (I haven't looked at the v3 that was posted overnight; maybe some of
> my comments have already been addressed.)
Thanks for your comments.
> ==
> General
>
> 1. (Info from the commit
Hi,
On Monday, February 27, 2023 6:30 PM wangw.f...@fujitsu.com
wrote:
> Attach the new patch.
Thanks for sharing v3. Minor review comments and question.
(1) UpdateDecodingProgressAndKeepalive header comment
The comment should be updated to explain maybe why we reset some other flags as
Here are some comments for the v2-0001 patch.
(I haven't looked at the v3 that was posted overnight; maybe some of
my comments have already been addressed.)
==
General
1. (Info from the commit message)
Since we can know whether the change is an end of transaction change in the
common code,
On Thur, Feb 23, 2023 at 18:41 PM Kuroda, Hayato/�\田 隼人
wrote:
> Dear Wang,
>
> Thank you for making the patch. IIUC your patch basically can achieve that
> output plugin
> does not have to call UpdateProgress.
Thanks for your review and comments.
> I think the basic approach is as follows,
Dear Wang,
Thank you for making the patch. IIUC your patch basically can achieve that
output plugin
does not have to call UpdateProgress.
I think the basic approach is as follows, is it right?
1. In *_cb_wrapper, set ctx->did_write to false
2. In OutputPluginWrite() set ctx->did_write to true.
On Sun, Feb 19, 2023 at 21:06 PM Wang, Wei/王 威 wrote:
> On Thur, Feb 14, 2023 at 2:03 AM Andres Freund wrote:
> > On 2023-02-13 14:06:57 +0530, Amit Kapila wrote:
> > > > > The patch calls update_progress in change_cb_wrapper and other
> > > > > wrappers which will miss the case of DDLs that
On Thur, Feb 14, 2023 at 2:03 AM Andres Freund wrote:
> On 2023-02-13 14:06:57 +0530, Amit Kapila wrote:
> > > > The patch calls update_progress in change_cb_wrapper and other
> > > > wrappers which will miss the case of DDLs that generates a lot of data
> > > > that is not processed by the
Hi,
On 2023-02-13 14:06:57 +0530, Amit Kapila wrote:
> > > The patch calls update_progress in change_cb_wrapper and other
> > > wrappers which will miss the case of DDLs that generates a lot of data
> > > that is not processed by the plugin. I think for that we either need
> > > to call
Hi,
On 2023-02-13 08:22:34 +0530, Amit Kapila wrote:
> On Sat, Feb 11, 2023 at 3:04 AM Andres Freund wrote:
> >
> > > One difference I see with the patch is that I think we will end up
> > > sending keepalive for empty prepared transactions even though we don't
> > > skip sending begin/prepare
On Sat, Feb 11, 2023 at 2:34 AM Andres Freund wrote:
>
> On 2023-02-09 11:21:41 +0530, Amit Kapila wrote:
> > On Thu, Feb 9, 2023 at 1:33 AM Andres Freund wrote:
> > >
> > > Hacking on a rough prototype how I think this should rather look, I had a
> > > few
> > > questions / remarks:
> > >
> >
On Sat, Feb 11, 2023 at 3:04 AM Andres Freund wrote:
>
> > One difference I see with the patch is that I think we will end up
> > sending keepalive for empty prepared transactions even though we don't
> > skip sending begin/prepare messages for those.
>
> With the proposed approach we reliably
Hi,
Replying on the new thread. Original message at
https://www.postgresql.org/message-id/CAA4eK1%2BH2m95HhzfpRkwv2-GtFwtbcVp7837X49%2Bvs0RXX3dBA%40mail.gmail.com
On 2023-02-09 15:54:19 +0530, Amit Kapila wrote:
> One thing to note about the changes we are discussing here is that
> some of the
Hi,
This is a reply to:
https://www.postgresql.org/message-id/CAA4eK1%2BDB66cYRRVyGcaMm7%2BtQ_u%3Dq%3D%2BHWGjpu9X0pqMFWbsZQ%40mail.gmail.com
split off, so patches to address some of my concerns don't confuse cfbot.
On 2023-02-09 11:21:41 +0530, Amit Kapila wrote:
> On Thu, Feb 9, 2023 at 1:33
On Thu, Feb 9, 2023 at 11:21 AM Amit Kapila wrote:
>
>
> How about renaming ProcessPendingWrites to WaitToSendPendingWrites or
> WalSndWaitToSendPendingWrites?
>
How about renaming WalSndUpdateProgress() to
WalSndUpdateProgressAndSendKeepAlive() or
WalSndUpdateProgressAndKeepAlive()?
One thing
On Thu, Feb 9, 2023 at 1:33 AM Andres Freund wrote:
>
> Hacking on a rough prototype how I think this should rather look, I had a few
> questions / remarks:
>
> - We probably need to call UpdateProgress from a bunch of places in decode.c
> as well? Indicating that we're lagging by a lot, just
38 matches
Mail list logo
