Close the connection on heartbeat timeouts in addition to SocketExceptions

[fakefeik]

Title

Heartbeat timeouts (OperationTimedOutException) never close the connection, unlike SocketException — connection can stay "alive" indefinitely against a host that accepts TCP but never responds

Description

When a heartbeat request (OptionsRequest sent by Connection.IdleTimeoutHandler) fails, the connection is only closed if the failure is a SocketException. If the heartbeat instead fails with a client-side OperationTimedOutException (i.e. the TCP connection is still open/ESTABLISHED, but the peer never responds within ReadTimeoutMillis), the connection is not closed — it just logs a warning and keeps retrying the heartbeat every HeartBeatInterval, forever.

This is exactly the failure mode you get from a full/partial network outage where the remote host still accepts new TCP connections (or an intermediate device/LB in front of it does) but application-level responses never arrive — e.g. an entire datacenter going dark behind a device that still completes the TCP handshake. In that scenario:

• Connections that are carrying real application traffic do eventually get evicted, via HostConnectionPool.CheckHealth once SocketOptions.DefunctReadTimeoutThreshold timed-out operations accumulate on them (triggered from RequestExecution when an application request times out).
• Connections that are mostly idle (so they rely on the heartbeat rather than application traffic) never trigger CheckHealth at all (nothing calls it except a live app-request timeout), and their heartbeat timeouts are silently ignored by OnIdleRequestException's type check — so they never get closed and never contribute to the host being marked DOWN.

We observed this in production during a real DC outage: log lines like

Received heartbeat request exception Cassandra.OperationTimedOutException: The host 192.168.10.13:9042 did not reply before timeout 12000ms

repeating every ~30 seconds (HeartBeatInterval default) for over half an hour on a connection to a host in the dead DC, well past the point the host had already been marked DOWN via its other, busier connections.

Relevant code

src/Cassandra/Connections/Connection.cs, IdleTimeoutHandler (heartbeat callback):

private void IdleTimeoutHandler(object state)
{
    ...
    var request = new OptionsRequest();
    Send(request, (error, response) =>
    {
        if (error?.Exception == null)
        {
            return TaskHelper.Completed;
        }
        Connection.Logger.Warning("Received heartbeat request exception " + error.Exception.ToString());
        if (error.Exception is SocketException)
        {
            OnIdleRequestException?.Invoke(error.Exception);
        }
        return TaskHelper.Completed;
    });
}

The heartbeat is sent via Send(request, callback), which uses Configuration.DefaultRequestOptions.ReadTimeoutMillis as its timeout and goes through the same OperationState/OnTimeout machinery as any other request. When it times out, the callback receives an OperationTimedOutException, not a SocketException — so OnIdleRequestException (below) is never invoked:

src/Cassandra/Connections/HostConnectionPool.cs:

private void OnIdleRequestException(IConnection c, Exception ex)
{
    if (c.IsDisposed)
    {
        HostConnectionPool.Logger.Info("Idle timeout exception, connection to {0} is disposed. Exception: {1}",
            _host.Address, ex);
    }
    else
    {
        HostConnectionPool.Logger.Warning("Connection to {0} considered as unhealthy after idle timeout exception: {1}",
            _host.Address, ex);
        c.Close();
    }
}

This method (the only place that closes a connection specifically because of a heartbeat failure) is unreachable for a client-side timeout, only for a genuine socket-level error.

Separately, CheckHealth (same file) does track timed-out operations against SocketOptions.DefunctReadTimeoutThreshold, and heartbeat timeouts do increment the shared Connection.TimedOutOperations counter (via the same OnTimeout path used by regular requests) — but CheckHealth is only invoked reactively, from RequestExecution, when an application request times out on that connection:

// RequestExecution.cs
if (ex is OperationTimedOutException)
{
    ...
    _session.CheckHealth(nodeRequestInfo.Host, connection);
}

So a connection whose only failures are heartbeat timeouts never gets its counter evaluated at all.

Comparison with the Java driver

The Java driver closes the channel on any heartbeat failure, regardless of cause:

HeartbeatHandler.java:

@Override
void fail(String message, Throwable cause) {
  if (cause instanceof HeartbeatException) {
    // Ignore: this happens when the heartbeat query times out and the inflight handler aborts
    // all queries (including the heartbeat query itself)
    return;
  }
  HeartbeatHandler.this.request = null;
  ...
  // Notify InFlightHandler.
  ctx.fireExceptionCaught(new HeartbeatException(ctx.channel().remoteAddress(), message, cause));
}

InFlightHandler.java:

@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable exception) throws Exception {
  ...
  } else {
    // Otherwise fail all pending requests
    abortAllInFlight(
        (exception instanceof HeartbeatException)
            ? (HeartbeatException) exception
            : new ClosedConnectionException("Unexpected error on channel", exception));
    ctx.close();
  }
}

There is no type check gating the ctx.close() call — any heartbeat failure (including a plain timeout with the socket still open) closes the channel. This matches the documented behavior ("If a heartbeat query fails, the connection is closed, and all pending queries are aborted").

Expected behavior

A heartbeat failure — whether caused by a SocketException or by a client-side OperationTimedOutException — should close the connection (and let the normal pool/reconnection logic take over), consistent with the Java driver and with the purpose of the heartbeat feature (detecting connections that look alive but aren't).

Suggested fix

The minimal, low-risk fix is to also treat OperationTimedOutException as a reason to close the connection, alongside the existing SocketException check, in Connection.IdleTimeoutHandler:

Connection.Logger.Warning("Received heartbeat request exception " + error.Exception.ToString());
if (error.Exception is SocketException || error.Exception is OperationTimedOutException)
{
    OnIdleRequestException?.Invoke(error.Exception);
}

This directly covers the observed case (OperationTimedOutException: The host ... did not reply before timeout ...ms) without changing behavior for any other exception type the heartbeat callback might see, so it shouldn't introduce any new risk of closing connections for reasons unrelated to the peer being unresponsive.

A broader fix would be to invoke OnIdleRequestException for any error.Exception, matching the Java driver's unconditional close-on-any-heartbeat-failure behavior:

Connection.Logger.Warning("Received heartbeat request exception " + error.Exception.ToString());
OnIdleRequestException?.Invoke(error.Exception);

Either way, HostConnectionPool.OnIdleRequestException already just logs and calls c.Close(), so no further change should be needed there.

Impact

Low under normal conditions (transient blips are usually caught by DefunctReadTimeoutThreshold via real traffic), but significant during a "TCP alive, application dead" outage on low-traffic connections/hosts (e.g. remote-DC connections with few concurrent requests): those connections never get evicted by the heartbeat mechanism at all, and can sit logging warnings every HeartBeatInterval indefinitely, well past the point the rest of the pool has already marked the host down.