From 5f62889f1d2dfb8c762afbaa18dd98d78e22ab0d Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 19 Sep 2026 16:59:46 +0300 Subject: [PATCH] [#1076] Walk every link of a connect failure where the verdict decides the retry CachedConnection.isWorthRetrying() decides whether a connect the database refused is waited out or reported as it stands, and stopped its walk of the cause and getNextException() chains at MAX_CHAIN_LENGTH. The count did no termination work - the visited set polls every throwable once - it only answered the link it never reached with "the caller's to see": a 53300, a 57P03 or a dialect-retryable link standing 33rd in the failure reported the connect as permanent on a database that would have taken it a moment later, and on the catalog connect of a write left the backend locked down until a restart. The walk now ends where the chains end, the way the walks of JDBCStorage whose verdict a decision reads have since #1004. MAX_CHAIN_LENGTH stays for the two budgets that have a reason to stop and say so - the fail-closed threshold of holdsCredentials() and the rebuild budget of redactedCopy() - and its javadoc names them rather than reading as a bound on every walk. testARetryableLinkPastTheOldBudgetOfTheWalkIsLookedAt borrows through a failure that carries the connection-limit state 33rd in its getNextException() chain and pins the second attempt: it fails on master with the failure reported, and passes here. --- .../backends/jdbc/CachedConnection.java | 17 +++++++++-- .../jdbc/CachedConnectionTestCase.java | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java index b08ce80112..c711776ffe 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java @@ -244,7 +244,15 @@ private static int standingReadBoundMillis(String connectionString, ConnectDiale static final long STALL_WARNING_AFTER_MS = 1000; static final long STALL_WARNING_INTERVAL_MS = 10000; - /** How many links of the cause and getNextException() chains of a failure are looked at. */ + /** + * How many links of the cause and getNextException() chains of a failure the two walks that + * have a reason to stop look at: {@link #holdsCredentials} answers "yes" past it, since the + * cost of the other answer is the password of the backend in the log, and + * {@link #redactedCopy} rebuilds that far and names the rest in one link. The walk whose verdict + * decides something, {@link #isWorthRetrying}, is not one of them: a count does not leave that + * question unanswered, it answers it with the verdict of a failure that carries nothing + * (issue #1076), and the visited set of every walk here terminates it on its own. + */ private static final int MAX_CHAIN_LENGTH = 32; /** What a connection string is cut down to where this cannot tell its credentials from the rest of it. */ @@ -1743,11 +1751,14 @@ private static boolean setNetworkTimeout(Connection con, int millis, String cons */ static boolean isWorthRetrying(SQLException e, ConnectDialect dialect) { // a failure of the driver is often wrapped, and a SQLException carries two chains of its - // own: the causes behind it and the further exceptions of getNextException() + // own: the causes behind it and the further exceptions of getNextException(). Walked to + // their end rather than to MAX_CHAIN_LENGTH: the visited set already terminates the walk, + // and a count answered the link it never reached with "the caller's to see" - the connect + // reported as permanent on a database that would have taken it a moment later (issue #1076) final Deque pending = new ArrayDeque<>(); final Set visited = Collections.newSetFromMap(new IdentityHashMap()); enqueue(pending, visited, e); - for (int links = 0; !pending.isEmpty() && links < MAX_CHAIN_LENGTH; links++) { + while (!pending.isEmpty()) { final Throwable t = pending.poll(); if (t instanceof SQLException) { final SQLException sql = (SQLException) t; diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java index 5d2295477d..44617b44fa 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java @@ -1077,6 +1077,34 @@ public void testTheWholeChainOfTheFailureIsLookedAt() throws Exception { assertEquals(stub.attempts.get(), 2, "the failure behind the one reported must be looked at"); } + /** + * ... and however far down the chain the driver put it. The walk that decides ends where the + * chain ends, not at a count of links: mssql-jdbc chains every error of one message through + * setNextException, and a walk that stopped at 32 of them answered "the caller's to see" for + * the link it never reached - the connect reported as permanent on a database that would have + * taken it a moment later, and a backend that stays locked down for it (issue #1076). + */ + @Test(timeOut = 120000) + public void testARetryableLinkPastTheOldBudgetOfTheWalkIsLookedAt() throws Exception { + final String url = StubDriver.PREFIX + "deep-chain"; + // 32 links that say nothing of the moment in front of the one that says the database is at its limit + SQLException chain = tooManyConnections(); + for (int link = 32; link > 0; link--) { + final SQLException inFront = new SQLException("error " + link + " of the same message", "08006", link); + inFront.setNextException(chain); + chain = inFront; + } + stub.failWith(chain, 1); + System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30"); + + try { + assertNotNull(CachedConnection.getConnection(url)); + } catch (SQLException reported) { + fail("a database at its limit 33 links down the failure must be waited out, not reported: " + reported, reported); + } + assertEquals(stub.attempts.get(), 2, "the link past the 32nd must be looked at"); + } + /** * The rest of the insufficient_resources class is not worth waiting out: a server out of disk * is not made whole by a connection of ours coming back to the pool.