Skip to content

JDBC backend: isWorthRetrying() stops its walk at MAX_CHAIN_LENGTH, so a retryable connect failure reported past 32 links is reported as permanent #1076

Description

@vharseko

Describe the bug

CachedConnection.isWorthRetrying(SQLException, ConnectDialect) (CachedConnection.java:1744) is the one
question that decides whether a connect the database refused is waited out or reported as it stands. It walks the
getCause() and getNextException() chains of the failure breadth-first and stops at MAX_CHAIN_LENGTH (:248):

// opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java:1750
for (int links = 0; !pending.isEmpty() && links < MAX_CHAIN_LENGTH; links++) {
    final Throwable t = pending.poll();
    if (t instanceof SQLException) {
        final SQLException sql = (SQLException) t;
        final String sqlState = sql.getSQLState();
        if (CONNECTION_LIMIT_SQL_STATE.equals(sqlState) || NOT_ACCEPTING_YET_SQL_STATE.equals(sqlState)
            || (dialect != null && dialect.isWorthRetrying(sql.getErrorCode()))) {
            return true;
        }
        enqueue(pending, visited, sql.getNextException());
    }
    enqueue(pending, visited, t.getCause());
}
return false;

A 53300, a 57P03 or a dialect-retryable link that is the 33rd throwable polled is never read, and the answer is not
"unknown" but false - the answer of a failure that is the caller's to see. The count does no termination work:
the visited identity set of enqueue() (:1921) polls every throwable at most once, so the walk ends on its own
on any chain a driver can allocate. The count only decides early.

This is the #961 shape on the connect road: a walk whose verdict decides something, given a budget that answers
the question it cannot finish with the verdict of a failure that carries nothing.

Impact

Two callers read that false, and both throw on it:

  • the borrow - CachedConnection.getConnection() at :1397, throw reported(e, connectionString). A
    database at its connection limit, with one of ours on its way back to the pool, or one still coming up, is
    reported to the caller instead of being waited out within POOL_TIMEOUT_PROPERTY.
  • the catalog connect inside a write - JDBCStorage.newCatalogConnection() at JDBCStorage.java:1757, the
    same throw. The javadoc of isWorthRetrying() names the cost on this road: the not-accepting-yet state is
    "the one JDBCStorage.open() has no second attempt of its own for, so a backend that meets it stays locked down
    until the server is restarted".

The walk runs on the raw driver exception, before reported() rebuilds it, so this budget is its own and not the
rebuild budget of #1074.

The direction of the error matters: a count cannot make the walk answer true where no link of the chain is
retryable, it can only hide one that is. Dropping it adds no retry that was not due; it costs a walk of the links
the driver has already allocated, which is what failureScope(), isConnectionFailure() and conflictVerdict()
in JDBCStorage already pay since #1004.

Narrow: it needs a driver that reports a connect failure as a chain of more than 32 links with the retryable one
at the end. mssql-jdbc chains every error of one message through setNextException, which is the driver #961
was measured against; PostgreSQL and MySQL report a refused connect in one or two links.

Not a regression

The count arrived with #876 (2c5d31b11d) together with the visited set that made it redundant: the review of
that PR turned the exhaustion of holdsCredentials() (:1896) into a fail-closed true - a chain longer than the
walk is rebuilt, since the cost of being wrong the other way is the password in the log - and gave
isWorthRetrying() the identity set only, leaving it to answer false past the count. Nothing in that thread says
why. No test pins the count: testTheWholeChainOfTheFailureIsLookedAt (CachedConnectionTestCase.java:1069)
claims the whole chain and exercises two links of it.

Untouched by PR #1004, which widened the walks of JDBCStorage and left CachedConnection.java byte-identical.
Raised as a follow-up in the review of #1004 (round 2, 2026-09-19) rather than blocking it.

Expected behavior

The walk of isWorthRetrying() bounded by visited alone - while (!pending.isEmpty()) - the way every walk
of JDBCStorage whose verdict a decision reads walks with EVERY_LINK since #1004. MAX_CHAIN_LENGTH stays for
the two budgets that are deliberate and say so: the fail-closed threshold of holdsCredentials() and the rebuild
budget of redactedCopy() (whose own truncation is #1074), and its javadoc names them rather than reading as a
bound on every walk of the class.

Pin: a 53300 link standing 33rd in a chain - 32 wrappers in front of it, wrappedTimes() of
JDBCStorageRetryTest gives the shape - is worth retrying. Fails on master, and makes
testTheWholeChainOfTheFailureIsLookedAt say what its name claims.

Line numbers are of origin/master (1af0a1247d).

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

    bugjavaChanges to Java sourcesjdbc

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions