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
