Skip to content

[Session] Throw IoTDBConnectionException instead of NPE when retries are exhausted - #18794

Open
yamass wants to merge 1 commit into
apache:masterfrom
yamass:fix/session-npe-on-exhausted-retries
Open

yamass wants to merge 1 commit into
apache:masterfrom
yamass:fix/session-npe-on-exhausted-retries

Conversation

@yamass

@yamass yamass commented Oct 6, 2026

Copy link
Copy Markdown

Description

Problem

When every attempt in SessionConnection.callWithRetryAndReconnect fails with a TException, it returns a RetryResult with a null result. Its callers (queries, TSStatus methods, schema template methods) use the result without checking it and throw a NullPointerException:

NullPointerException: Cannot invoke "TSExecuteStatementResp.getStatus()" because "execResp" is null
  at SessionConnection.executeQueryStatement(...)

As a result:

  • The real cause (for example TTransportException: Cannot write to null outputStream) is lost.
  • SessionPool handles the NPE in its RuntimeException branch and puts the broken session back at the head of its queue, so the next call gets the same session. As long as that session cannot reconnect, each call fails the same way. We saw this continue until the client JVM was restarted.

Fix

callWithRetryAndReconnect now throws IoTDBConnectionException(lastTException) when no attempt produced a result. The result can only be null if the last attempt threw, so the cause is always set.

  • The check is in the shared helper, so it covers all callers without changing any of them.
  • SessionPool now takes its existing IoTDBConnectionException branch, which closes the broken session and creates a new one.
  • No new message strings, so no i18n changes. I don't see a reason to add a message where the exception is thrown.

Tests

New BrokenSessionConnectionTest uses a real, closed Thrift transport and an unreachable reconnect target. It checks that executeQueryStatement and setStorageGroup throw IoTDBConnectionException with the cause attached, and that a broken connection leads to a session eviction. These tests fail on master and pass with this change. All iotdb-session unit tests pass.


This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage.

Key changed/added classes (or packages if there are too many classes) in this PR
  • iotdb-client/session: SessionConnection, BrokenSessionConnectionTest (new)

…are exhausted

callWithRetryAndReconnect returned a null result after all attempts failed
with a TException. Callers dereferenced it and threw a NullPointerException,
losing the real cause. SessionPool treated the NPE as a RuntimeException and
put the broken session back into the pool, so every later call failed the
same way until the client JVM was restarted.

Throw IoTDBConnectionException with the last TException as the cause, so the
error is diagnosable and SessionPool evicts the broken session.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant