Skip to content

isConnected() liveness probe leaks JolokiaHttpException/CompletionException, aborting metrics refresh #1801

Description

@tommystendahl

Regression introduced by #1767 (commit 9fc8130).

Problem

The active liveness probe added to DistributedJmxConnectionProviderImpl.isConnected() calls mbs.getMBeanCount(). For Jolokia connections this issues a Jolokia search HTTP request. When that request fails at the transport level, the Jolokia client throws a CompletionException wrapping JolokiaHttpException — neither of which is caught by the current handler:

catch (IOException | NullPointerException e)
{
    LOG.debug("JMX connection validation failed", e);
    return false;
}

JolokiaHttpException is not an IOException, and the wrapping CompletionException is a RuntimeException, so the exception escapes isConnected() instead of being converted to return false. It then propagates up through validateJmxConnection()getPercentRepaired()CassandraMetrics.getMetrics() and surfaces as:

WARNING: Exception thrown during refresh
java.util.concurrent.CompletionException: org.jolokia.client.exception.JolokiaHttpException: HTTP error 100 sending search Jolokia request
    at ...JdkHttpClient.execute(JdkHttpClient.java:245)
    at ...RemoteJmxAdapter.queryNames(RemoteJmxAdapter.java:584)
    at ...RemoteJmxAdapter.getMBeanCount(RemoteJmxAdapter.java:523)
    at ...DistributedJmxConnectionProviderImpl.isConnected(DistributedJmxConnectionProviderImpl.java:81)
    at ...AgentJmxConnectionProvider.isConnected(AgentJmxConnectionProvider.java:177)
    at ...JolokiaJmxProxy.validateJmxConnection(JolokiaJmxProxy.java:111)
    at ...AbstractDistributedJmxProxy.getPercentRepaired(AbstractDistributedJmxProxy.java:439)
    at ...CassandraMetrics.getMetrics(CassandraMetrics.java:84)

Impact

  1. Metrics refresh aborts mid-cycle. The escaping exception interrupts CassandraMetrics.getMetrics(), so percentRepaired / maxRepairedAt are not updated for that node on that cycle. This is particularly relevant for incremental repair, which relies on those metrics.
  2. Log noise. A transient transport hiccup produces a WARNING with a full stack trace instead of a graceful "node temporarily unreachable → false".
  3. Wrong health semantics. isConnected() is a boolean health check; any probe failure should mean "not connected", not an exception.

About HTTP 100

HTTP error 100 (Continue) is an interim/informational response the Jolokia JDK/httpclient5 client did not expect for a search request. It can occur with certain reverse-proxy / Expect: 100-continue configurations (e.g. the NGINX PEM setup). It is transient/environmental — exactly the kind of failure isConnected() should treat as "not connected".

Root Cause

Before #1767, isConnected() used a passive check (getConnectionId() + getMBeanServerConnection() != null) that made no network call and therefore never threw transport exceptions. The active getMBeanCount() probe introduced a network call whose failure modes are not fully covered by the IOException | NullPointerException catch clause.

Proposed Fix

Broaden the catch clause so any probe failure yields false:

@Override
public boolean isConnected(final JMXConnector jmxConnector)
{
    if (jmxConnector == null)
    {
        return false;
    }
    try
    {
        jmxConnector.getConnectionId();
        MBeanServerConnection mbs = jmxConnector.getMBeanServerConnection();
        if (mbs == null)
        {
            return false;
        }
        mbs.getMBeanCount();
        return true;
    }
    catch (Exception e)   // was: IOException | NullPointerException
    {
        LOG.debug("JMX connection validation failed", e);
        return false;
    }
}

Catching Exception is appropriate for a boolean health check: any failure means "not healthy", the failure is logged at debug, and the caller (RetrySchedulerService) will attempt reconnection. Error (OOM etc.) still propagates since only Exception is caught.

Files to Modify

File Change
connection.impl/src/.../providers/DistributedJmxConnectionProviderImpl.java Broaden isConnected() catch clause to Exception

Test

Add a unit test to TestDistributedJmxConnectionProviderIsConnected where a mocked MBeanServerConnection.getMBeanCount() throws a RuntimeException (and a CompletionException wrapping a JolokiaHttpException), asserting isConnected() returns false rather than propagating.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions