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> |
Subject: | RE: Logical replication timeout problem |
Date: | 2022-02-28 10:58:15 |
Message-ID: | TYAPR01MB586673CBF482B3F9AD5853DDF5019@TYAPR01MB5866.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Wang,
> Attached a new patch that addresses following improvements I have got so far as
> comments:
> 1. Consider other changes that need to be skipped(truncate, DDL and function
> calls pg_logical_emit_message). [suggestion by Ajin, Amit]
> (Add a new function SendKeepaliveIfNecessary for trying to send keepalive
> message.)
> 2. Set the threshold conservatively to a static value of 10000.[suggestion by Amit,
> Kuroda-San]
> 3. Reset sendTime in function WalSndUpdateProgress when send_keep_alive is
> false. [suggestion by Amit]
Thank you for giving a good patch! I'll check more detail later,
but it can be applied my codes and passed check world.
I put some minor comments:
```
+ * Try to send keepalive message
```
Maybe missing "a"?
```
+ /*
+ * After continuously skipping SKIPPED_CHANGES_THRESHOLD changes, try to send a
+ * keepalive message.
+ */
```
This comments does not follow preferred style:
https://www.postgresql.org/docs/devel/source-format.html
```
@@ -683,12 +683,12 @@ OutputPluginWrite(struct LogicalDecodingContext *ctx, bool last_write)
* Update progress tracking (if supported).
*/
void
-OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx)
+OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx, bool send_keep_alive)
```
This function is no longer doing just tracking.
Could you update the code comment above?
```
if (!is_publishable_relation(relation))
return;
```
I'm not sure but it seems that the function exits immediately if relation
is a sequence, view, temporary table and so on. Is it OK? Does it never happen?
```
+ SendKeepaliveIfNecessary(ctx, false);
```
I think a comment is needed above which clarifies sending a keepalive message.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2022-02-28 11:02:08 | Re: Missed condition-variable wakeups on FreeBSD |
Previous Message | Dagfinn Ilmari Mannsåker | 2022-02-28 10:50:01 | Re: psql: Make SSL info display more compact |