The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

There is no check for (last_reply_timestamp <= 0 || wal_sender_timeout <= 0) as 
in other places
(in WalSndKeepaliveIfNecessary for example).

I don't think moving update of 'now' down to end of loop body is correct:
there are calls to ProcessConfigFile with SyncRepInitConfig, 
ProcessRepliesIfAny that can
last non-negligible time. It could lead to over sleeping due to larger computed 
Though I could be mistaken.

I'm not sure about moving `if (!pg_is_send_pending())` in a body loop after 
Is it necessary? But it looks harmless at least.

Could patch be reduced to check after first `if (!pg_is_sendpending())` ? like:

        if (!pq_is_send_pending())
-               return;
+       {
+               if (last_reply_timestamp <= 0 || wal_sender_timeout <= 0)
+               {
+                       CHECK_FOR_INTERRUPTS();
+                       return;
+               }
+               if (now <= TimestampTzPlusMilliseconds(last_reply_timestamp, 
wal_sender_timeout / 2))
+                       return;
+       }

If not, what problem prevents?

The new status of this patch is: Waiting on Author

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to