Keep-alive timeouts and failover

[Jonathan Patchell]

I have been testing failover scenarios on AIX where I bring down the network on the member in a cluster the client is connected to. It looks like there is a bug with the keep-alive timeouts where the timer isn’t being set. This causes the failover to take fairly long (~75 seconds) on AIX as there aren’t any OS-level socket errors until then.

I have found that the following change causes the keep-alive timeouts to work properly causing quicker failovers respecting the GRPC_ARG_KEEPALIVE_TIMEOUT_MS value on AIX.

I have been using grpc v1.80.0.

diff --git a/src/core/ext/transport/chttp2/transport/writing.cc b/src/core/ext/transport/chttp2/transport/writing.cc

index 30f6caa033..7a288c7066 100644

--- a/src/core/ext/transport/chttp2/transport/writing.cc

+++ b/src/core/ext/transport/chttp2/transport/writing.cc

@@ -799,7 +799,7 @@ void grpc_chttp2_end_write(grpc_chttp2_transport* t, grpc_error_handle error) {

 

     if (t->keepalive_incoming_data_wanted &&

         t->keepalive_timeout < t->ping_timeout &&

-        t->keepalive_ping_timeout_handle !=

+        t->keepalive_ping_timeout_handle ==

             grpc_event_engine::experimental::EventEngine::TaskHandle::

                 kInvalid) {

       if (GRPC_TRACE_FLAG_ENABLED(http2_ping) ||

`keepalive_ping_timeout_handle != kInvalid` can never be true when the handle is first being armed (it starts as `kInvalid`), so the watchdog timer is never scheduled.

It looks like this was a bug that was identified and fixed in https://github.com/grpc/grpc/commit/b086f689effef7276bef6456f599aee1c12489d4. However, there was an automated rollback to the original version in https://github.com/grpc/grpc/commit/4f9b6f15f391433ef12952fe9578b9b50e159990.

The bug is platform-agnostic dead code, but the bug seems to be only visible on AIX (I don’t see such issues on Linux).

Could someone confirm whether this is indeed a bug worth fixing upstream? Happy to submit a PR with this fix.