RE: Logical replication timeout problem

2023-03-01 Thread wangw.f...@fujitsu.com
On Thu, Mar 2, 2023 at 4:19 AM Gregory Stark (as CFM) wrote: > On Wed, 8 Feb 2023 at 15:04, Andres Freund wrote: > > > > Attached is a current, quite rough, prototype. It addresses some of the > > points > > raised, but far from all. There's also several XXXs/FIXMEs in it. I changed > >

Re: Logical replication timeout problem

2023-03-01 Thread Gregory Stark (as CFM)
On Wed, 8 Feb 2023 at 15:04, Andres Freund wrote: > > Attached is a current, quite rough, prototype. It addresses some of the points > raised, but far from all. There's also several XXXs/FIXMEs in it. I changed > the file-ending to .txt to avoid hijacking the CF entry. It looks like this patch

Re: Logical replication timeout problem

2023-02-08 Thread Andres Freund
Hi, On 2023-02-08 10:30:37 -0800, Andres Freund wrote: > On 2023-02-08 10:18:41 -0800, Andres Freund wrote: > > I don't think the syncrep logic in WalSndUpdateProgress really works as-is - > > consider what happens if e.g. the origin filter filters out entire > > transactions. We'll afaics never

Re: Logical replication timeout problem

2023-02-08 Thread Andres Freund
Hi, On 2023-02-08 10:18:41 -0800, Andres Freund wrote: > I don't think the syncrep logic in WalSndUpdateProgress really works as-is - > consider what happens if e.g. the origin filter filters out entire > transactions. We'll afaics never get to WalSndUpdateProgress(). In some cases > we'll be

Re: Logical replication timeout problem

2023-02-08 Thread Andres Freund
Hi, On 2023-02-08 13:36:02 +0530, Amit Kapila wrote: > On Wed, Feb 8, 2023 at 10:57 AM Andres Freund wrote: > > > > On 2023-02-03 10:13:54 +0530, Amit Kapila wrote: > > > I am planning to push this to HEAD sometime next week (by Wednesday). > > > To backpatch this, we need to fix it in some

Re: Logical replication timeout problem

2023-02-08 Thread Amit Kapila
On Wed, Feb 8, 2023 at 10:57 AM Andres Freund wrote: > > On 2023-02-03 10:13:54 +0530, Amit Kapila wrote: > > I am planning to push this to HEAD sometime next week (by Wednesday). > > To backpatch this, we need to fix it in some non-standard way, like > > without introducing a callback which I am

Re: Logical replication timeout problem

2023-02-07 Thread Andres Freund
Hi, On 2023-02-03 10:13:54 +0530, Amit Kapila wrote: > I am planning to push this to HEAD sometime next week (by Wednesday). > To backpatch this, we need to fix it in some non-standard way, like > without introducing a callback which I am not sure is a good idea. If > some other committers vote

Re: Logical replication timeout problem

2023-02-02 Thread Amit Kapila
On Wed, Feb 1, 2023 at 10:04 AM Amit Kapila wrote: > > On Tue, Jan 31, 2023 at 8:24 PM Ashutosh Bapat > wrote: > > > > On Tue, Jan 31, 2023 at 5:12 PM Amit Kapila wrote: > > > > > > On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat > > > wrote: > > > > > > > > On Tue, Jan 31, 2023 at 4:58 PM Amit

Re: Logical replication timeout problem

2023-01-31 Thread Amit Kapila
On Tue, Jan 31, 2023 at 8:24 PM Ashutosh Bapat wrote: > > On Tue, Jan 31, 2023 at 5:12 PM Amit Kapila wrote: > > > > On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat > > wrote: > > > > > > On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila > > > wrote: > > > > > > > Thanks, the patch looks good to me.

Re: Logical replication timeout problem

2023-01-31 Thread Amit Kapila
On Wed, Feb 1, 2023 at 4:43 AM Peter Smith wrote: > > Here are my review comments for v13-1. > > == > Commit message > > 1. > The DDLs like Refresh Materialized views that generate lots of temporary > data due to rewrite rules may not be processed by output plugins (for > example

Re: Logical replication timeout problem

2023-01-31 Thread Peter Smith
Here are my review comments for v13-1. == Commit message 1. The DDLs like Refresh Materialized views that generate lots of temporary data due to rewrite rules may not be processed by output plugins (for example pgoutput). So, we won't send keep-alive messages for a long time while

Re: Logical replication timeout problem

2023-01-31 Thread Ashutosh Bapat
On Tue, Jan 31, 2023 at 5:12 PM Amit Kapila wrote: > > On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat > wrote: > > > > On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila wrote: > > > > > Thanks, the patch looks good to me. I have slightly adjusted one of > > > the comments and ran pgindent. See

Re: Logical replication timeout problem

2023-01-31 Thread Amit Kapila
On Tue, Jan 31, 2023 at 5:03 PM Ashutosh Bapat wrote: > > On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila wrote: > > > Thanks, the patch looks good to me. I have slightly adjusted one of > > the comments and ran pgindent. See attached. As mentioned in the > > commit message, we shouldn't backpatch

Re: Logical replication timeout problem

2023-01-31 Thread Ashutosh Bapat
On Tue, Jan 31, 2023 at 4:58 PM Amit Kapila wrote: > Thanks, the patch looks good to me. I have slightly adjusted one of > the comments and ran pgindent. See attached. As mentioned in the > commit message, we shouldn't backpatch this as this requires a new > callback and moreover, users can

Re: Logical replication timeout problem

2023-01-31 Thread Amit Kapila
On Tue, Jan 31, 2023 at 2:53 PM wangw.f...@fujitsu.com wrote: > > On Mon, Jan 30, 2023 at 17:50 PM I wrote: > > Attach the new patch. > > When invoking the function ReorderBufferProcessTXN, the threshold-related > counter "changes_count" may have some random value from the previous >

RE: Logical replication timeout problem

2023-01-31 Thread wangw.f...@fujitsu.com
On Mon, Jan 30, 2023 at 17:50 PM I wrote: > Attach the new patch. When invoking the function ReorderBufferProcessTXN, the threshold-related counter "changes_count" may have some random value from the previous transaction's processing. To fix this, I moved the definition of the counter

RE: Logical replication timeout problem

2023-01-30 Thread wangw.f...@fujitsu.com
On Mon, Jan 30, 2023 at 14:55 PM Amit Kapila wrote: > On Mon, Jan 30, 2023 at 10:36 AM wangw.f...@fujitsu.com > wrote: > > > > On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨 > wrote: > > > On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com > > > wrote: > > > > Yes, I think you are right. > > Fixed

Re: Logical replication timeout problem

2023-01-29 Thread Amit Kapila
On Mon, Jan 30, 2023 at 10:36 AM wangw.f...@fujitsu.com wrote: > > On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨 wrote: > > On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com > > wrote: > > Yes, I think you are right. > Fixed this problem. > + /* + * Trying to send keepalive message after every

RE: Logical replication timeout problem

2023-01-29 Thread wangw.f...@fujitsu.com
On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨 wrote: > On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com > wrote: > > > > I tested a mix transaction of transactional and non-transactional messages > > on > > the current HEAD and reproduced the timeout problem. I think this result is > OK. > >

RE: Logical replication timeout problem

2023-01-29 Thread shiy.f...@fujitsu.com
On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com wrote: > > I tested a mix transaction of transactional and non-transactional messages on > the current HEAD and reproduced the timeout problem. I think this result is > OK. > Because when decoding a transaction, non-transactional changes are

RE: Logical replication timeout problem

2023-01-28 Thread wangw.f...@fujitsu.com
On Fri, Jan 27, 2023 at 19:55 PM Amit Kapila wrote: > On Fri, Jan 27, 2023 at 5:18 PM houzj.f...@fujitsu.com > wrote: > > > > On Wednesday, January 25, 2023 7:26 PM Amit Kapila > > > > > > > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com > > > wrote: > > > > > > > > Attach the new

Re: Logical replication timeout problem

2023-01-27 Thread Amit Kapila
On Fri, Jan 27, 2023 at 5:18 PM houzj.f...@fujitsu.com wrote: > > On Wednesday, January 25, 2023 7:26 PM Amit Kapila > > > > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com > > wrote: > > > > > > Attach the new patch. > > > > > > > I think the patch missed to handle the case of

RE: Logical replication timeout problem

2023-01-27 Thread houzj.f...@fujitsu.com
On Wednesday, January 25, 2023 7:26 PM Amit Kapila > > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com > wrote: > > > > Attach the new patch. > > > > I think the patch missed to handle the case of non-transactional messages > which > was previously getting handled. I have tried to

Re: Logical replication timeout problem

2023-01-25 Thread Amit Kapila
On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com wrote: > > Attach the new patch. > I think the patch missed to handle the case of non-transactional messages which was previously getting handled. I have tried to address that in the attached. Is there a reason that shouldn't be handled?

Re: Logical replication timeout problem

2023-01-23 Thread Peter Smith
On Tue, Jan 24, 2023 at 1:45 PM wangw.f...@fujitsu.com wrote: > > On Tues, Jan 24, 2023 at 8:28 AM Peter Smith wrote: > > Hi Hou-san, Here are my review comments for v5-0001. > > Thanks for your comments. ... > > Changed as suggested. > > Attach the new patch. Thanks! Patch v6 LGTM. --

RE: Logical replication timeout problem

2023-01-23 Thread wangw.f...@fujitsu.com
On Tues, Jan 24, 2023 at 8:28 AM Peter Smith wrote: > Hi Hou-san, Here are my review comments for v5-0001. Thanks for your comments. > == > src/backend/replication/logical/reorderbuffer.c > > 1. > @@ -2446,6 +2452,23 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, > ReorderBufferTXN *txn, >

Re: Logical replication timeout problem

2023-01-23 Thread Peter Smith
Hi Hou-san, Here are my review comments for v5-0001. == src/backend/replication/logical/reorderbuffer.c 1. @@ -2446,6 +2452,23 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, elog(ERROR, "tuplecid value in changequeue"); break; } + + /* + * Sending keepalive

RE: Logical replication timeout problem

2023-01-23 Thread houzj.f...@fujitsu.com
On Monday, January 23, 2023 8:51 AM Peter Smith wrote: > > Here are my review comments for patch v4-0001 > == > Commit message > > 2. > > The problem is when there is a DDL in a transaction that generates lots of > temporary data due to rewrite rules, these temporary data will not be >

Re: Logical replication timeout problem

2023-01-22 Thread Amit Kapila
On Mon, Jan 23, 2023 at 6:21 AM Peter Smith wrote: > > 1. > > It makes no real difference, but I was wondering about: > "update txn progress" versus "update progress txn" > Yeah, I think we can go either way but I still prefer "update progress txn" as that is more closer to

Re: Logical replication timeout problem

2023-01-22 Thread Peter Smith
Here are my review comments for patch v4-0001 == General 1. It makes no real difference, but I was wondering about: "update txn progress" versus "update progress txn" I thought that the first way sounds more natural. YMMV. If you change this then there is impact for the typedef, function

RE: Logical replication timeout problem

2023-01-19 Thread wangw.f...@fujitsu.com
On Fri, Jan 20, 2023 at 10:10 AM Peter Smith wrote: > Here are some review comments for patch v3-0001. Thanks for your comments. > == > Commit message > > 1. > The problem is when there is a DDL in a transaction that generates lots of > temporary data due to rewrite rules, these temporary

RE: Logical replication timeout problem

2023-01-19 Thread wangw.f...@fujitsu.com
On Fri, Jan 20, 2023 at 12:35 PM Amit Kapila wrote: > On Fri, Jan 20, 2023 at 7:40 AM Peter Smith wrote: > > > > Here are some review comments for patch v3-0001. > > > > == > > src/backend/replication/logical/logical.c > > > > 3. forward declaration > > > > +/* update progress callback */ >

RE: Logical replication timeout problem

2023-01-19 Thread wangw.f...@fujitsu.com
On Thu, Jan 19, 2023 at 19:37 PM Amit Kapila wrote: > On Thu, Jan 19, 2023 at 4:13 PM Ashutosh Bapat > wrote: > > > > On Wed, Jan 18, 2023 at 6:00 PM Amit Kapila > wrote: > > > > > + */ > > > + ReorderBufferUpdateProgressCB update_progress; > > > > > > Are you suggesting changing the name of

Re: Logical replication timeout problem

2023-01-19 Thread Peter Smith
On Fri, Jan 20, 2023 at 3:35 PM Amit Kapila wrote: > > On Fri, Jan 20, 2023 at 7:40 AM Peter Smith wrote: > > > > Here are some review comments for patch v3-0001. > > > > == > > src/backend/replication/logical/logical.c > > > > 3. forward declaration > > > > +/* update progress callback */ >

Re: Logical replication timeout problem

2023-01-19 Thread Amit Kapila
On Fri, Jan 20, 2023 at 7:40 AM Peter Smith wrote: > > Here are some review comments for patch v3-0001. > > == > src/backend/replication/logical/logical.c > > 3. forward declaration > > +/* update progress callback */ > +static void update_progress_cb_wrapper(ReorderBuffer *cache, > +

Re: Logical replication timeout problem

2023-01-19 Thread Peter Smith
Here are some review comments for patch v3-0001. == Commit message 1. The problem is when there is a DDL in a transaction that generates lots of temporary data due to rewrite rules, these temporary data will not be processed by the pgoutput - plugin. Therefore, the previous fix (f95d53e) for

Re: Logical replication timeout problem

2023-01-19 Thread Amit Kapila
On Thu, Jan 19, 2023 at 4:13 PM Ashutosh Bapat wrote: > > On Wed, Jan 18, 2023 at 6:00 PM Amit Kapila wrote: > > > + */ > > + ReorderBufferUpdateProgressCB update_progress; > > > > Are you suggesting changing the name of the above variable? If so, how > > about apply_progress, progress, or

Re: Logical replication timeout problem

2023-01-19 Thread Ashutosh Bapat
On Wed, Jan 18, 2023 at 6:00 PM Amit Kapila wrote: > + */ > + ReorderBufferUpdateProgressCB update_progress; > > Are you suggesting changing the name of the above variable? If so, how > about apply_progress, progress, or updateprogress? If you don't like > any of these then feel free to suggest

Re: Logical replication timeout problem

2023-01-18 Thread Amit Kapila
On Wed, Jan 18, 2023 at 5:37 PM Ashutosh Bapat wrote: > > On Wed, Jan 18, 2023 at 1:49 PM wangw.f...@fujitsu.com > wrote: > > > > On Wed, Jan 18, 2023 at 13:29 PM Amit Kapila > > wrote: > > > On Tue, Jan 17, 2023 at 6:41 PM Ashutosh Bapat > > > wrote: > > > > > > > > On Tue, Jan 17, 2023 at

Re: Logical replication timeout problem

2023-01-18 Thread Ashutosh Bapat
On Wed, Jan 18, 2023 at 1:49 PM wangw.f...@fujitsu.com wrote: > > On Wed, Jan 18, 2023 at 13:29 PM Amit Kapila wrote: > > On Tue, Jan 17, 2023 at 6:41 PM Ashutosh Bapat > > wrote: > > > > > > On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila > > > wrote: > > > > > > > > > > > > > I am a bit worried

RE: Logical replication timeout problem

2023-01-18 Thread wangw.f...@fujitsu.com
On Wed, Jan 18, 2023 at 13:29 PM Amit Kapila wrote: > On Tue, Jan 17, 2023 at 6:41 PM Ashutosh Bapat > wrote: > > > > On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila wrote: > > > > > > > > > > I am a bit worried about the indirections that the wrappers and hooks > > > > create. Output plugins call

Re: Logical replication timeout problem

2023-01-17 Thread Amit Kapila
On Tue, Jan 17, 2023 at 6:41 PM Ashutosh Bapat wrote: > > On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila wrote: > > > > > > > I am a bit worried about the indirections that the wrappers and hooks > > > create. Output plugins call OutputPluginUpdateProgress() in callbacks > > > but I don't see why

Re: Logical replication timeout problem

2023-01-17 Thread Ashutosh Bapat
On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila wrote: > > > > I am a bit worried about the indirections that the wrappers and hooks > > create. Output plugins call OutputPluginUpdateProgress() in callbacks > > but I don't see why ReorderBufferProcessTXN() needs a callback to > > call

Re: Logical replication timeout problem

2023-01-17 Thread Amit Kapila
On Mon, Jan 16, 2023 at 10:06 PM Ashutosh Bapat wrote: > > On Wed, Jan 11, 2023 at 4:11 PM wangw.f...@fujitsu.com > wrote: > > > > On Mon, Jan 9, 2023 at 13:04 PM Amit Kapila wrote: > > > > > > > Thanks for your comments. > > > > > One more thing, I think it would be better to expose a new

Re: Logical replication timeout problem

2023-01-16 Thread Ashutosh Bapat
On Wed, Jan 11, 2023 at 4:11 PM wangw.f...@fujitsu.com wrote: > > On Mon, Jan 9, 2023 at 13:04 PM Amit Kapila wrote: > > > > Thanks for your comments. > > > One more thing, I think it would be better to expose a new callback > > API via reorder buffer as suggested previously [2] similar to other

Re: Logical replication timeout problem

2023-01-16 Thread Ashutosh Bapat
On Mon, Jan 9, 2023 at 4:08 PM wangw.f...@fujitsu.com wrote: > > On Fri, Jan 6, 2023 at 15:06 PM Ashutosh Bapat > wrote: > > I'm not sure if we need to add global variables or member variables for a > cumulative count that is only used here. How would you feel if I add some > comments when

RE: Logical replication timeout problem

2023-01-11 Thread wangw.f...@fujitsu.com
On Mon, Jan 9, 2023 at 13:04 PM Amit Kapila wrote: > Thanks for your comments. > One more thing, I think it would be better to expose a new callback > API via reorder buffer as suggested previously [2] similar to other > reorder buffer APIs instead of directly using reorderbuffer API to >

RE: Logical replication timeout problem

2023-01-09 Thread wangw.f...@fujitsu.com
On Fri, Jan 6, 2023 at 15:06 PM Ashutosh Bapat wrote: > Hi Wang, > Thanks for working on this. One of our customer faced a similar > situation when running BDR with PostgreSQL. > > I tested your patch and it solves the problem. > > Please find some review comments below Thanks for your

Re: Logical replication timeout problem

2023-01-08 Thread Amit Kapila
On Fri, Jan 6, 2023 at 12:35 PM Ashutosh Bapat wrote: > > + > +/* > + * We don't want to try sending a keepalive message after processing each > + * change as that can have overhead. Tests revealed that there is no > + * noticeable overhead in doing it after continuously

Re: Logical replication timeout problem

2023-01-05 Thread Ashutosh Bapat
Hi Wang, Thanks for working on this. One of our customer faced a similar situation when running BDR with PostgreSQL. I tested your patch and it solves the problem. Please find some review comments below On Tue, Nov 8, 2022 at 8:34 AM wangw.f...@fujitsu.com wrote: > > > Attach the patch. > +/*

RE: Logical replication timeout problem

2022-11-07 Thread wangw.f...@fujitsu.com
On Fri, Nov 4, 2022 at 18:13 PM Fabrice Chapuis wrote: > Hello Wang, > > I tested the draft patch in my lab for Postgres 14.4, the refresh of the > materialized view ran without generating the timeout on the worker. > Do you plan to propose this patch at the next commit fest. Thanks for your

Re: Logical replication timeout problem

2022-11-04 Thread Fabrice Chapuis
Hello Wang, I tested the draft patch in my lab for Postgres 14.4, the refresh of the materialized view ran without generating the timeout on the worker. Do you plan to propose this patch at the next commit fest. Regards, Fabrice On Wed, Oct 19, 2022 at 10:15 AM wangw.f...@fujitsu.com <

RE: Logical replication timeout problem

2022-10-20 Thread wangw.f...@fujitsu.com
On Thurs, Oct 20, 2022 at 13:47 PM Fabrice Chapuis wrote: > Yes the refresh of MV is on the Publisher Side. > Thanks for your draft patch, I'll try it > I'll back to you as soonas possible Thanks a lot. > One question: why the refresh of the MV is a DDL not a DML? Since in the source, the

Re: Logical replication timeout problem

2022-10-19 Thread Fabrice Chapuis
Yes the refresh of MV is on the Publisher Side. Thanks for your draft patch, I'll try it I'll back to you as soonas possible One question: why the refresh of the MV is a DDL not a DML? Regards Fabrice On Wed, 19 Oct 2022, 10:15 wangw.f...@fujitsu.com wrote: > On Tue, Oct 18, 2022 at 22:35 PM

RE: Logical replication timeout problem

2022-10-19 Thread wangw.f...@fujitsu.com
On Tue, Oct 18, 2022 at 22:35 PM Fabrice Chapuis wrote: > Hello Amit, > > In version 14.4 the timeout problem for logical replication happens again > despite > the patch provided for this issue in this version. When bulky materialized > views > are reloaded it broke logical replication. It is

Re: Logical replication timeout problem

2022-10-18 Thread Fabrice Chapuis
Hello Amit, In version 14.4 the timeout problem for logical replication happens again despite the patch provided for this issue in this version. When bulky materialized views are reloaded it broke logical replication. It is possible to solve this problem by using your new "streaming" option. Have

Re: Logical replication timeout problem

2022-05-11 Thread Amit Kapila
On Mon, May 9, 2022 at 2:17 PM Masahiko Sawada wrote: > > The patches look good to me too. > Pushed. -- With Regards, Amit Kapila.

RE: Logical replication timeout problem

2022-05-09 Thread wangw.f...@fujitsu.com
On Mon, May 9, 2022 at 11:23 AM Amit Kapila wrote: > On Mon, May 9, 2022 at 7:01 PM Euler Taveira wrote: > > > > On Mon, May 9, 2022, at 3:47 AM, Amit Kapila wrote: > > > > Thanks. The patch LGTM. I'll push and back-patch this after the > > current minor release is done unless there are more

Re: Logical replication timeout problem

2022-05-09 Thread Amit Kapila
On Mon, May 9, 2022 at 7:01 PM Euler Taveira wrote: > > On Mon, May 9, 2022, at 3:47 AM, Amit Kapila wrote: > > Thanks. The patch LGTM. I'll push and back-patch this after the > current minor release is done unless there are more comments related > to this work. > > Looks sane to me. (I only

Re: Logical replication timeout problem

2022-05-09 Thread Euler Taveira
On Mon, May 9, 2022, at 3:47 AM, Amit Kapila wrote: > Thanks. The patch LGTM. I'll push and back-patch this after the > current minor release is done unless there are more comments related > to this work. Looks sane to me. (I only tested the HEAD version) + boolend_xact = ctx->end_xact;

Re: Logical replication timeout problem

2022-05-09 Thread Masahiko Sawada
On Mon, May 9, 2022 at 3:47 PM Amit Kapila wrote: > > On Fri, May 6, 2022 at 12:42 PM wangw.f...@fujitsu.com > wrote: > > > > On Fri, May 6, 2022 at 9:54 AM Masahiko Sawada > > wrote: > > > On Wed, May 4, 2022 at 7:18 PM Amit Kapila > > > wrote: > > > > > > > > On Mon, May 2, 2022 at 8:07 AM

Re: Logical replication timeout problem

2022-05-09 Thread Amit Kapila
On Fri, May 6, 2022 at 12:42 PM wangw.f...@fujitsu.com wrote: > > On Fri, May 6, 2022 at 9:54 AM Masahiko Sawada wrote: > > On Wed, May 4, 2022 at 7:18 PM Amit Kapila wrote: > > > > > > On Mon, May 2, 2022 at 8:07 AM Masahiko Sawada > > wrote: > > > > > > > > On Mon, May 2, 2022 at 11:32 AM

RE: Logical replication timeout problem

2022-05-06 Thread wangw.f...@fujitsu.com
On Fri, May 6, 2022 at 9:54 AM Masahiko Sawada wrote: > On Wed, May 4, 2022 at 7:18 PM Amit Kapila wrote: > > > > On Mon, May 2, 2022 at 8:07 AM Masahiko Sawada > wrote: > > > > > > On Mon, May 2, 2022 at 11:32 AM Amit Kapila > wrote: > > > > > > > > > > > > So, shall we go back to the

Re: Logical replication timeout problem

2022-05-05 Thread Masahiko Sawada
On Wed, May 4, 2022 at 7:18 PM Amit Kapila wrote: > > On Mon, May 2, 2022 at 8:07 AM Masahiko Sawada wrote: > > > > On Mon, May 2, 2022 at 11:32 AM Amit Kapila wrote: > > > > > > > > > So, shall we go back to the previous approach of using a separate > > > function update_replication_progress?

Re: Logical replication timeout problem

2022-05-04 Thread Amit Kapila
On Mon, May 2, 2022 at 8:07 AM Masahiko Sawada wrote: > > On Mon, May 2, 2022 at 11:32 AM Amit Kapila wrote: > > > > > > So, shall we go back to the previous approach of using a separate > > function update_replication_progress? > > Ok, agreed. > Attached, please find the updated patch

Re: Logical replication timeout problem

2022-05-01 Thread Masahiko Sawada
On Mon, May 2, 2022 at 11:32 AM Amit Kapila wrote: > > On Mon, May 2, 2022 at 7:33 AM Masahiko Sawada wrote: > > On Thu, Apr 28, 2022 at 7:01 PM houzj.f...@fujitsu.com > > wrote: > > > > > > Hi Sawada-san, Wang > > > > > > I was looking at the patch and noticed that we moved some logic from > >

Re: Logical replication timeout problem

2022-05-01 Thread Amit Kapila
On Mon, May 2, 2022 at 7:33 AM Masahiko Sawada wrote: > On Thu, Apr 28, 2022 at 7:01 PM houzj.f...@fujitsu.com > wrote: > > > > Hi Sawada-san, Wang > > > > I was looking at the patch and noticed that we moved some logic from > > update_replication_progress() to OutputPluginUpdateProgress() like

Re: Logical replication timeout problem

2022-05-01 Thread Masahiko Sawada
On Thu, Apr 28, 2022 at 7:01 PM houzj.f...@fujitsu.com wrote: > > On Wednesday, April 20, 2022 3:21 PM Masahiko Sawada > wrote: > > > > BTW the changes in > > REL_14_v1-0001-Fix-the-logical-replication-timeout-during-large-.patch, > > adding end_xact to LogicalDecodingContext, seems good to me

RE: Logical replication timeout problem

2022-04-28 Thread wangw.f...@fujitsu.com
On Thur, Apr 28, 2022 at 6:26 PM Amit Kapila wrote: > On Thu, Apr 21, 2022 at 3:21 PM wangw.f...@fujitsu.com > wrote: > > > > I think it is better to keep the new variable 'end_xact' at the end of > the struct where it belongs for HEAD. In back branches, we can keep it > at the place as you

Re: Logical replication timeout problem

2022-04-28 Thread Amit Kapila
On Thu, Apr 21, 2022 at 3:21 PM wangw.f...@fujitsu.com wrote: > I think it is better to keep the new variable 'end_xact' at the end of the struct where it belongs for HEAD. In back branches, we can keep it at the place as you have. Apart from that, I have made some cosmetic changes and changed a

RE: Logical replication timeout problem

2022-04-28 Thread houzj.f...@fujitsu.com
On Wednesday, April 20, 2022 3:21 PM Masahiko Sawada wrote: > > BTW the changes in > REL_14_v1-0001-Fix-the-logical-replication-timeout-during-large-.patch, > adding end_xact to LogicalDecodingContext, seems good to me and it > might be better than the approach of v17 patch from plugin

RE: Logical replication timeout problem

2022-04-21 Thread wangw.f...@fujitsu.com
On Wed, Apr 21, 2022 at 10:15 AM I wrote: > The comments by Sawada-San sound reasonable to me. > After doing check, I found that padding in HEAD is the same as in REL14. > So I change the approach of patch for HEAD just like the patch for REL14. Also attach the back-branch patches for

Re: Logical replication timeout problem

2022-04-20 Thread Masahiko Sawada
On Thu, Apr 21, 2022 at 11:19 AM Amit Kapila wrote: > > On Wed, Apr 20, 2022 at 6:22 PM Masahiko Sawada wrote: > > > > On Wed, Apr 20, 2022 at 7:12 PM Amit Kapila wrote: > > > > > > > > I think it would > > > be then better to have it in the same place in HEAD as well? > > > > As far as I can

Re: Logical replication timeout problem

2022-04-20 Thread Amit Kapila
On Wed, Apr 20, 2022 at 6:22 PM Masahiko Sawada wrote: > > On Wed, Apr 20, 2022 at 7:12 PM Amit Kapila wrote: > > > > > I think it would > > be then better to have it in the same place in HEAD as well? > > As far as I can see in the v17 patch, which is for HEAD, we don't add > a variable to

RE: Logical replication timeout problem

2022-04-20 Thread wangw.f...@fujitsu.com
On Wed, Apr 20, 2022 at 6:13 PM Amit Kapila wrote: > On Wed, Apr 20, 2022 at 2:38 PM Amit Kapila wrote: > > > > On Wed, Apr 20, 2022 at 12:51 PM Masahiko Sawada > wrote: > > > > > > On Wed, Apr 20, 2022 at 11:46 AM wangw.f...@fujitsu.com > > > wrote: > > > > ``` > > > > > > I'm concerned that

Re: Logical replication timeout problem

2022-04-20 Thread Masahiko Sawada
On Wed, Apr 20, 2022 at 7:12 PM Amit Kapila wrote: > > On Wed, Apr 20, 2022 at 2:38 PM Amit Kapila wrote: > > > > On Wed, Apr 20, 2022 at 12:51 PM Masahiko Sawada > > wrote: > > > > > > On Wed, Apr 20, 2022 at 11:46 AM wangw.f...@fujitsu.com > > > wrote: > > > > ``` > > > > > > I'm concerned

Re: Logical replication timeout problem

2022-04-20 Thread Amit Kapila
On Wed, Apr 20, 2022 at 2:38 PM Amit Kapila wrote: > > On Wed, Apr 20, 2022 at 12:51 PM Masahiko Sawada > wrote: > > > > On Wed, Apr 20, 2022 at 11:46 AM wangw.f...@fujitsu.com > > wrote: > > > ``` > > > > I'm concerned that this 4-byte padding at the end of the struct could > > depend on

Re: Logical replication timeout problem

2022-04-20 Thread Amit Kapila
On Wed, Apr 20, 2022 at 12:51 PM Masahiko Sawada wrote: > > On Wed, Apr 20, 2022 at 11:46 AM wangw.f...@fujitsu.com > wrote: > > ``` > > I'm concerned that this 4-byte padding at the end of the struct could > depend on platforms (there might be no padding in 32-bit platforms?). > Good point,

Re: Logical replication timeout problem

2022-04-20 Thread Masahiko Sawada
On Wed, Apr 20, 2022 at 11:46 AM wangw.f...@fujitsu.com wrote: > > On Mon, Apr 18, 2022 at 00:36 PM Amit Kapila wrote: > > On Mon, Apr 18, 2022 at 9:29 AM Amit Kapila wrote: > > > > > > On Thu, Apr 14, 2022 at 5:52 PM Euler Taveira wrote: > > > > > > > > On Wed, Apr 13, 2022, at 7:45 AM, Amit

RE: Logical replication timeout problem

2022-04-19 Thread wangw.f...@fujitsu.com
On Mon, Apr 18, 2022 at 00:36 PM Amit Kapila wrote: > On Mon, Apr 18, 2022 at 9:29 AM Amit Kapila wrote: > > > > On Thu, Apr 14, 2022 at 5:52 PM Euler Taveira wrote: > > > > > > On Wed, Apr 13, 2022, at 7:45 AM, Amit Kapila wrote: > > > > > > Sawada-San, Euler, do you have any opinion on this

RE: Logical replication timeout problem

2022-04-18 Thread wangw.f...@fujitsu.com
On Mon, Apr 19, 2022 at 9:32 AM Masahiko Sawada wrote: > Thank you for updating the patch. Thanks for your comments. > + * For a large transaction, if we don't send any change to the > + downstream for a > + * long time(exceeds the wal_receiver_timeout of standby) then it can > timeout. > + *

Re: Logical replication timeout problem

2022-04-18 Thread Masahiko Sawada
On Mon, Apr 18, 2022 at 3:16 PM wangw.f...@fujitsu.com wrote: > > On Mon, Apr 18, 2022 at 00:35 PM Masahiko Sawada > wrote: > > On Mon, Apr 18, 2022 at 1:01 PM Amit Kapila wrote: > > > > > > On Thu, Apr 14, 2022 at 5:50 PM Masahiko Sawada > > wrote: > > > > > > > > On Wed, Apr 13, 2022 at

RE: Logical replication timeout problem

2022-04-18 Thread wangw.f...@fujitsu.com
On Thur, Apr 14, 2022 at 8:21 PM Euler Taveira wrote: > Thanks for your comments. > + * For a large transaction, if we don't send any change to the downstream > for a > + * long time then it can timeout. This can happen when all or most of the > + * changes are either not published or got

RE: Logical replication timeout problem

2022-04-18 Thread wangw.f...@fujitsu.com
On Mon, Apr 18, 2022 at 00:35 PM Masahiko Sawada wrote: > On Mon, Apr 18, 2022 at 1:01 PM Amit Kapila wrote: > > > > On Thu, Apr 14, 2022 at 5:50 PM Masahiko Sawada > wrote: > > > > > > On Wed, Apr 13, 2022 at 7:45 PM Amit Kapila > wrote: > > > > > > > > On Mon, Apr 11, 2022 at 12:09 PM

Re: Logical replication timeout problem

2022-04-17 Thread Amit Kapila
On Mon, Apr 18, 2022 at 9:29 AM Amit Kapila wrote: > > On Thu, Apr 14, 2022 at 5:52 PM Euler Taveira wrote: > > > > On Wed, Apr 13, 2022, at 7:45 AM, Amit Kapila wrote: > > > > Sawada-San, Euler, do you have any opinion on this approach? I > > personally still prefer the approach implemented in

Re: Logical replication timeout problem

2022-04-17 Thread Masahiko Sawada
On Mon, Apr 18, 2022 at 1:01 PM Amit Kapila wrote: > > On Thu, Apr 14, 2022 at 5:50 PM Masahiko Sawada wrote: > > > > On Wed, Apr 13, 2022 at 7:45 PM Amit Kapila wrote: > > > > > > On Mon, Apr 11, 2022 at 12:09 PM wangw.f...@fujitsu.com > > > wrote: > > > > > > > > So I skip tracking lag

Re: Logical replication timeout problem

2022-04-17 Thread Amit Kapila
On Thu, Apr 14, 2022 at 5:50 PM Masahiko Sawada wrote: > > On Wed, Apr 13, 2022 at 7:45 PM Amit Kapila wrote: > > > > On Mon, Apr 11, 2022 at 12:09 PM wangw.f...@fujitsu.com > > wrote: > > > > > > So I skip tracking lag during a transaction just like the current HEAD. > > > Attach the new

Re: Logical replication timeout problem

2022-04-17 Thread Amit Kapila
On Thu, Apr 14, 2022 at 5:52 PM Euler Taveira wrote: > > On Wed, Apr 13, 2022, at 7:45 AM, Amit Kapila wrote: > > Sawada-San, Euler, do you have any opinion on this approach? I > personally still prefer the approach implemented in v10 [1] especially > due to the latest finding by Wang-San that we

Re: Logical replication timeout problem

2022-04-14 Thread Euler Taveira
On Wed, Apr 13, 2022, at 7:45 AM, Amit Kapila wrote: > On Mon, Apr 11, 2022 at 12:09 PM wangw.f...@fujitsu.com > wrote: > > > > So I skip tracking lag during a transaction just like the current HEAD. > > Attach the new patch. > > > > Thanks, please find the updated patch where I have slightly

Re: Logical replication timeout problem

2022-04-14 Thread Masahiko Sawada
On Wed, Apr 13, 2022 at 7:45 PM Amit Kapila wrote: > > On Mon, Apr 11, 2022 at 12:09 PM wangw.f...@fujitsu.com > wrote: > > > > So I skip tracking lag during a transaction just like the current HEAD. > > Attach the new patch. > > > > Thanks, please find the updated patch where I have slightly

Re: Logical replication timeout problem

2022-04-13 Thread Amit Kapila
On Mon, Apr 11, 2022 at 12:09 PM wangw.f...@fujitsu.com wrote: > > So I skip tracking lag during a transaction just like the current HEAD. > Attach the new patch. > Thanks, please find the updated patch where I have slightly modified the comments. Sawada-San, Euler, do you have any opinion on

RE: Logical replication timeout problem

2022-04-11 Thread wangw.f...@fujitsu.com
On Mon, Apr 11, 2022 at 2:39 PM I wrote: > Attach the new patch. Also, share test results and details. To check that the lsn information used for the calculation is what we expected, I get some information by adding logs in the function LagTrackerRead. Summary of test results: - In current HEAD

RE: Logical replication timeout problem

2022-04-11 Thread wangw.f...@fujitsu.com
On Wed, Apr 7, 2022 at 1:34 PM Amit Kapila wrote: > On Wed, Apr 6, 2022 at 6:30 PM wangw.f...@fujitsu.com > wrote: > > > > On Wed, Apr 6, 2022 at 1:58 AM Amit Kapila wrote: > > On Wed, Apr 6, 2022 at 4:32 AM Amit Kapila wrote: > > > Also, let's try to evaluate how it impacts lag functionality

RE: Logical replication timeout problem

2022-04-07 Thread wangw.f...@fujitsu.com
On Wed, Apr 7, 2022 at 1:34 PM Amit Kapila wrote: > Thanks for your comments. > One comment: > +static void > +update_progress(LogicalDecodingContext *ctx, bool skipped_xact, bool > end_xact) > +{ > + static int changes_count = 0; > + > + if (end_xact) > + { > + /* Update progress tracking at

Re: Logical replication timeout problem

2022-04-06 Thread Amit Kapila
On Wed, Apr 6, 2022 at 6:30 PM wangw.f...@fujitsu.com wrote: > > On Wed, Apr 6, 2022 at 1:59 AM Amit Kapila wrote: > On Wed, Apr 6, 2022 at 4:32 AM Amit Kapila wrote: > > > Thanks for your comments. > > > typedef void (*LogicalOutputPluginWriterUpdateProgress) (struct > >

RE: Logical replication timeout problem

2022-04-06 Thread wangw.f...@fujitsu.com
On Wed, Apr 6, 2022 at 1:59 AM Amit Kapila wrote: On Wed, Apr 6, 2022 at 4:32 AM Amit Kapila wrote: > Thanks for your comments. > typedef void (*LogicalOutputPluginWriterUpdateProgress) (struct > LogicalDecodingContext *lr, > XLogRecPtr Ptr, > TransactionId xid, > - bool skipped_xact > +

Re: Logical replication timeout problem

2022-04-06 Thread Amit Kapila
On Wed, Apr 6, 2022 at 11:28 AM Amit Kapila wrote: > > On Wed, Apr 6, 2022 at 11:09 AM wangw.f...@fujitsu.com > wrote: > > > > According to your suggestion, improve the patch to make it more generic. > > Attach the new patch. > > > > typedef void (*LogicalOutputPluginWriterUpdateProgress)

Re: Logical replication timeout problem

2022-04-05 Thread Amit Kapila
On Wed, Apr 6, 2022 at 11:09 AM wangw.f...@fujitsu.com wrote: > > On Fri, Apr 1, 2022 at 12:09 AM Amit Kapila wrote: > > On Fri, Apr 1, 2022 at 8:28 AM Euler Taveira wrote: > > > > > > It seems I didn't make myself clear. The callbacks I'm referring to the > > > *_cb_wrapper functions. After

RE: Logical replication timeout problem

2022-04-05 Thread wangw.f...@fujitsu.com
On Fri, Apr 1, 2022 at 12:09 AM Amit Kapila wrote: > On Fri, Apr 1, 2022 at 8:28 AM Euler Taveira wrote: > > > > On Thu, Mar 31, 2022, at 11:27 PM, Amit Kapila wrote: > > > > This is exactly our initial analysis and we have tried a patch on > > these lines and it has a noticeable overhead. See

Re: Logical replication timeout problem

2022-03-31 Thread Amit Kapila
On Fri, Apr 1, 2022 at 8:28 AM Euler Taveira wrote: > > On Thu, Mar 31, 2022, at 11:27 PM, Amit Kapila wrote: > > This is exactly our initial analysis and we have tried a patch on > these lines and it has a noticeable overhead. See [1]. Calling this > for each change or each skipped change can

  1   2   >