From: | "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com> |
Cc: | Fabrice Chapuis <fabrice636861(at)gmail(dot)com>, Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Ajin Cherian <itsajin(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com> |
Subject: | RE: Logical replication timeout problem |
Date: | 2022-03-09 03:52:03 |
Message-ID: | TYAPR01MB5866BB9000D72CBCAFC76E62F50A9@TYAPR01MB5866.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Wang,
Thank you for updating!
> > Do we need adding a test for them? I think it can be added to 100_bugs.pl.
> > Actually I tried to send PoC, but it does not finish to implement that.
> > I'll send if it is done.
> I'm not sure if it is worth it.
> Because the reproduced test of this bug might take some time and might risk
> making the build farm slow, so I am not sure if others would like the
> reproduced test of this bug.
I was taught from you that it may suggest that it is difficult to stabilize and
minimize the test. I withdraw the above.
I put some comments for v2, mainly cosmetic ones.
1. pgoutput_change
```
+ bool is_send = true;
```
My first impression is that is_send should be initialized to false,
and it will change to true when OutputPluginWrite() is called.
2. pgoutput_change
```
+ {
+ is_send = false;
+ break;
+ }
```
Here are too many indents, but I think they should be removed.
See above comment.
3. WalSndUpdateProgress
```
+ /*
+ * If half of wal_sender_timeout has lapsed without send message standby,
+ * send a keep-alive message to the standby.
+ */
```
The comment seems inconsistency with others.
Here is "keep-alive", but other parts are "keepalive".
4. ReorderBufferProcessTXN
```
+ change->data.inval.ninvalidations,
+ change->data.inval.invalidations);
```
Maybe these lines break 80-columns rule.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2022-03-09 04:29:19 | Re: Optionally automatically disable logical replication subscriptions on error |
Previous Message | Amit Kapila | 2022-03-09 03:36:52 | Re: Optionally automatically disable logical replication subscriptions on error |