The patch looks good to me. Thanks for improving comments around that code. I like the change to set waiting_for_ping_response in WalSndKeepalive. Thanks.
On Fri, 7 Aug 2020 at 04:26, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > Ashutosh Bapat noticed that WalSndWaitForWal() is setting > waiting_for_ping_response after sending a keepalive that does *not* > request a reply. The bad consequence is that other callers that do > require a reply end up in not sending a keepalive, because they think it > was already sent previously. So the whole thing gets stuck. > > He found that commit 41d5f8ad734 failed to remove the setting of > waiting_for_ping_response after changing the "request" parameter > WalSndKeepalive from true to false; that seems to have been an omission > and it breaks the algorithm. Thread at [1]. > > The simplest fix is just to remove the line that sets > waiting_for_ping_response, but I think it is less error-prone to have > WalSndKeepalive set the flag itself, instead of expecting its callers to > do it (and know when to). Patch attached. Also rewords some related > commentary. > > [1] > https://postgr.es/m/flat/blu436-smtp25712b7ef9fc2adeb87c522dc...@phx.gbl > > -- > Álvaro Herrera Valdivia, Chile > -- Best Wishes, Ashutosh
