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
sleeptime.
Though I could be mistaken.
I'm not sure about moving `if (!pg_is_send_pending())` in a body loop after
WalSndKeepaliveIfNecessary.
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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
