From a4c85dec55394fe3ef6005acff6a66b7fbfc464a Mon Sep 17 00:00:00 2001
From: Valera V Harseko <vharseko@3a-systems.ru>
Date: Tue, 01 Sep 2026 18:18:00 +0000
Subject: [PATCH] Merge remote-tracking branch 'origin/master' into issues/903-jdbc-deadlock-retry-window
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java | 23
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 117 +
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java | 1870 +++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java | 1363 ++++++++++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java | 776 +++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java | 34
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 585 ++++++-
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java | 16
8 files changed, 4,628 insertions(+), 156 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 188f7ba..22d63d4 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
@@ -23,21 +23,169 @@
import java.sql.*;
import java.time.Duration;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
public class CachedConnection implements Connection {
private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+ // What has been reported once already. Every one of these reports a setting rather than an
+ // event - a property that is not a number, a url no bound of this class can reach, a driver
+ // whose property names are not known here - so it does not become truer by being repeated,
+ // and every operation of the backend comes through here.
+ // Declared above every field whose initializer can reach warnOnce(): class variable
+ // initializers run in textual order (JLS 12.4.2), so a set declared below aliveBypassNanos
+ // would still be null the moment a property this class reports on carries a value worth
+ // warning about - a window longer than the ttl, or one that is not a number - and the report
+ // would leave the class uninitializable rather than merely configured oddly.
+ static final Set<String> warnedOnce = ConcurrentHashMap.newKeySet();
+
static final String TTL_PROPERTY = "org.openidentityplatform.opendj.jdbc.ttl";
static final long DEFAULT_TTL_MS = 15000;
+ /**
+ * How long a pooled connection is handed out without being validated after it was last proven
+ * alive, in ms; 0 validates every borrow, the way this pool did before the window existed.
+ * <p>
+ * The validation of a connection is a round trip of its own - an empty query on postgresql, a
+ * ping on mysql, a round trip of its own on oracle and sql server - and every operation of
+ * this backend pays it next to the single statement the operation came for. It earns that on a
+ * connection that has been sitting in the pool, which the database or a firewall may have
+ * dropped in the meantime; it earns nothing on one that answered a moment ago, which is most
+ * of them under load. So a connection proven alive within this window is trusted rather than
+ * validated, the way the aliveBypassWindow of HikariCP does it.
+ */
+ static final String ALIVE_BYPASS_PROPERTY = "org.openidentityplatform.opendj.jdbc.alive.bypass";
+ static final long DEFAULT_ALIVE_BYPASS_MS = 500;
+
+ /**
+ * The longest window this class uses, whatever {@value #ALIVE_BYPASS_PROPERTY} and the
+ * {@value #TTL_PROPERTY} it is clamped to say. The clamp to the ttl alone does not bound it:
+ * the ttl has no upper bound of its own, and with both set high enough the conversion to
+ * nanoseconds saturates - the window then outlasts every reading it is compared against, and
+ * no connection of the pool is ever validated again. An hour is already far past what this
+ * window is about, which is a connection that answered a moment ago.
+ * <p>
+ * A compile-time constant, so that it holds its value wherever it is read from: the initializer
+ * of {@link #aliveBypassNanos} reaches it, and a field initialized in declaration order would
+ * still be 0 there if it were ever moved below (JLS 12.4.2).
+ */
+ static final long MAX_ALIVE_BYPASS_MS = 60 * 60 * 1000L;
+
+ // Read once, at class initialization: every operation of this backend borrows a connection,
+ // and the borrow is not the place to parse a system property. Not final so that a test can
+ // vary the window without a class loader of its own, and volatile because a non-final static
+ // long is written neither atomically nor visibly to the threads reading it (JLS 17.7) - every
+ // worker of the backend and every replay thread reads this one.
+ static volatile long aliveBypassNanos = TimeUnit.MILLISECONDS.toNanos(getAliveBypassMillis());
+
+ /**
+ * Bounds the connect and the login of one attempt to establish a connection, in seconds; 0 for
+ * no bound of its own - the deadline of {@value #POOL_TIMEOUT_PROPERTY} still bounds the
+ * attempt, since it stands for the whole borrow. Setting both to 0 is what leaves a connect
+ * unbounded.
+ */
+ static final String CONNECT_TIMEOUT_PROPERTY = "org.openidentityplatform.opendj.jdbc.connect.timeout";
+ static final long DEFAULT_CONNECT_TIMEOUT_SECONDS = 30;
+
+ /**
+ * Bounds a whole borrow - every connect attempt and every wait for a pooled connection - in
+ * seconds; 0 for no bound. Not to the millisecond: the connection in hand is validated
+ * whatever the deadline says, and an attempt is never given less than a second, so a borrow
+ * can return a validation and a last attempt past it.
+ */
+ static final String POOL_TIMEOUT_PROPERTY = "org.openidentityplatform.opendj.jdbc.pool.timeout";
+ static final long DEFAULT_POOL_TIMEOUT_SECONDS = 60;
+
+ /** Bound of the validation of a pooled connection: isValid(0) means "no timeout" in the JDBC contract. */
+ static final int VALIDATION_TIMEOUT_SECONDS = 5;
+
+ /** 08001, sqlclient_unable_to_establish_sqlconnection: the state of a connect that did not happen. */
+ private static final String CONNECT_FAILED_SQL_STATE = "08001";
+ /** 53300, too_many_connections: how the standard - and postgresql - reports a server taking no further connection. */
+ private static final String CONNECTION_LIMIT_SQL_STATE = "53300";
+ /** 57P03, cannot_connect_now: postgresql starting up, shutting down or in recovery. */
+ private static final String NOT_ACCEPTING_YET_SQL_STATE = "57P03";
+
+ static final long MAX_BACKOFF_MS = 1000;
+ 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. */
+ 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. */
+ static final String CREDENTIALS_HIDDEN = "<credentials hidden>";
+ /**
+ * A password standing where neither the userinfo nor the parameters of a url are looked for.
+ * The name goes by more than one spelling: mysql numbers the factors of a multi-factor login
+ * (password1, password2), and a wallet or a key store carries one under a name of its own
+ * (oracle.net.wallet_password, javax.net.ssl.keyStorePassword).
+ * The value of one ends at a separator of a connection string or at the first space: a driver
+ * is free to name a parameter in the middle of a sentence ("password=hunter2 for user u at
+ * h:5432"), and a value class running to the end of the string would take the host, the port
+ * and the cause of the failure into the blank along with the password.
+ */
+ private static final Pattern SECRET_PARAMETER =
+ Pattern.compile("(?i)([\\w.]*(password|passwd|pwd)\\d*)\\s*=([^\\s,)&;?]*)");
+
+ /** The parameters of a connection string worth keeping in a message: which database, not who connects. */
+ private static final Set<String> IDENTIFYING_PARAMETERS = Collections.unmodifiableSet(new HashSet<>(
+ Arrays.asList("databasename", "database", "instancename", "currentschema", "servicename")));
+
+ // setNetworkTimeout() takes the executor its timeout handling runs on: the drivers it is used
+ // with here only set a socket option in it, so it costs a call rather than a thread.
+ private static final Executor DIRECT_EXECUTOR = Runnable::run;
+
+ // Throttled per connection string: two JDBC backends stalling at once have a stall of their
+ // own to report, and a single timestamp would let one of them starve the other. Keyed by the
+ // safe form of it, the way warnedOnce below is: a static field of this class outlives every
+ // borrow, and the password of the backend has no business in one.
+ private static final Map<String, AtomicLong> lastStallWarning = new ConcurrentHashMap<>();
+ private static final AtomicLong lastReadBoundWarning = new AtomicLong();
+
+ /**
+ * When an operation last reported that the database had dropped a connection of a pool, as a
+ * {@link System#nanoTime()} reading per connection string. A connection proven alive before
+ * that moment is validated on its next borrow whatever the window says: whatever dropped one
+ * connection - a restart, a failover, a network that went away - dropped every connection
+ * established before it, and the window would otherwise hand out the rest of that generation
+ * one by one until the pool runs out of them. It is set by the caller that saw the failure
+ * ({@code JDBCStorage}), never by a validation that failed here: an idle connection the server
+ * reaped is a routine event, and it says nothing about the connection in use that the pool is
+ * about to hand out.
+ */
+ private static final Map<String, Long> poolDistrustedAt = new ConcurrentHashMap<>();
+
final Connection parent;
- static LoadingCache<String, BlockingQueue<CachedConnection>> cached = Caffeine.newBuilder()
+ // A deque handed out from the end it is returned to: the connection borrowed next is the one
+ // returned last, so under any load the pool keeps reusing its hottest connections instead of
+ // walking round every one it ever opened. That is what gives the window above anything to
+ // bypass - a connection reached only after a whole cycle of the pool has been idle far longer
+ // than the window - and it leaves the connections nothing needs at the cold end of the deque,
+ // where the per-connection idle expiry of #878 can find them. Until that lands, the cold end
+ // is reached only when the whole pool expires, after DEFAULT_TTL_MS with the backend idle.
+ // A deque takes one lock for both of its ends where the queue it replaces took one for each,
+ // so a borrow and a return no longer proceed side by side - against the round trip the window
+ // above saves, and the connect the reuse saves, that lock is not worth a FIFO handoff.
+ static LoadingCache<String, BlockingDeque<CachedConnection>> cached = Caffeine.newBuilder()
.expireAfterAccess(Duration.ofMillis(getCacheTtlMillis()))
- .removalListener((String key, BlockingQueue<CachedConnection> value, RemovalCause cause) -> {
+ .removalListener((String key, BlockingDeque<CachedConnection> value, RemovalCause cause) -> {
for (CachedConnection con : value) {
try {
if (!con.isClosed()) {
@@ -48,66 +196,1198 @@
}
}
})
- .build(conStr -> new LinkedBlockingQueue<>());
+ .build(conStr -> new LinkedBlockingDeque<>());
/**
* Returns the time after which an idle pooled connection is closed, as configured by the
* {@value #TTL_PROPERTY} system property. An invalid value is ignored in favor of the default.
*/
private static long getCacheTtlMillis() {
- final String ttl = System.getProperty(TTL_PROPERTY);
- if (ttl != null) {
+ return getNonNegativeProperty(TTL_PROPERTY, DEFAULT_TTL_MS, "ms");
+ }
+
+ /**
+ * Returns the alive window, clamped to the {@value #TTL_PROPERTY} an idle pooled connection is
+ * kept for and to {@link #MAX_ALIVE_BYPASS_MS} behind it. A window longer than the ttl is one
+ * the pool cannot back: it goes on trusting the last answer of a connection past the point the
+ * pool would have closed and replaced it, which is a claim about a connection that is no longer
+ * there. The ttl has no upper bound of its own, though, so the second clamp is what keeps a
+ * value the unit conversion saturates on from leaving every connection of the pool trusted for
+ * the life of the server.
+ * <p>
+ * Read at class initialization, like the ttl it is clamped to, so a value set after that
+ * changes neither.
+ */
+ static long getAliveBypassMillis() {
+ long configured = getNonNegativeProperty(ALIVE_BYPASS_PROPERTY, DEFAULT_ALIVE_BYPASS_MS, "ms");
+ final long ttl = getCacheTtlMillis();
+ if (configured > ttl) {
+ warnOnce(ALIVE_BYPASS_PROPERTY + "=" + configured + ">" + ttl,
+ "The %s window of %d ms is longer than the %d ms of %s a pooled connection is kept for,"
+ + " and is used as %d ms: a connection trusted for longer than the pool keeps it would"
+ + " be trusted past the point the pool closed it",
+ ALIVE_BYPASS_PROPERTY, configured, ttl, TTL_PROPERTY, ttl);
+ configured = ttl;
+ }
+ if (configured > MAX_ALIVE_BYPASS_MS) { // the ttl it was just clamped to has no upper bound of its own
+ warnOnce(ALIVE_BYPASS_PROPERTY + ">" + MAX_ALIVE_BYPASS_MS,
+ "The %s window of %d ms is longer than the %d ms this pool trusts a connection for at most,"
+ + " and is used as %d ms: a longer one saturates the arithmetic it is compared in and"
+ + " leaves every connection of the pool trusted for the life of the server",
+ ALIVE_BYPASS_PROPERTY, configured, MAX_ALIVE_BYPASS_MS, MAX_ALIVE_BYPASS_MS);
+ return MAX_ALIVE_BYPASS_MS;
+ }
+ return configured;
+ }
+
+ /**
+ * Returns the value of a numeric system property, ignoring a value that is not a non-negative
+ * number in favor of the default. The unit is the one the property is read in, so that the
+ * value the message names is not mistaken for another.
+ */
+ private static long getNonNegativeProperty(String name, long defaultValue, String unit) {
+ final String value = System.getProperty(name);
+ if (value != null) {
try {
- final long millis = Long.parseLong(ttl.trim());
- if (millis >= 0) {
- return millis;
+ final long parsed = Long.parseLong(value.trim());
+ if (parsed >= 0) {
+ return parsed;
}
} catch (NumberFormatException ignored) {
}
- logger.warn(LocalizableMessage.raw("Ignoring invalid value \"%s\" of the %s property, using %d ms",
- ttl, TTL_PROPERTY, DEFAULT_TTL_MS));
+ // reported once for this value: both properties are read on every borrow, so a
+ // "30s" of a typo would otherwise put two lines in the log per backend operation
+ warnOnce(name + "=" + value, "Ignoring invalid value \"%s\" of the %s property, using %d %s",
+ value, name, defaultValue, unit);
}
- return DEFAULT_TTL_MS;
+ return defaultValue;
+ }
+
+ /** Reports something about a setting once for the life of the jvm, however many borrows meet it. */
+ private static void warnOnce(String key, String format, Object... args) {
+ if (warnedOnce.add(key)) {
+ logger.warn(LocalizableMessage.raw(format, args));
+ }
+ }
+
+ /**
+ * The drivers this backend is used with, recognized by the prefix of the connection string,
+ * together with the properties that bound one attempt to establish a connection. Not one of
+ * them bounds the attempt with a single property: the one named first covers the socket
+ * connect, and the login behind it - the reads of the prelogin handshake, of TLS and of
+ * authentication, the phase a proxy at its connection limit or a moved VIP leaves unanswered -
+ * needs the read bound behind it. That holds for the SQL Server driver too, whose loginTimeout
+ * leaves the read of the prelogin answer unbounded - and for pgjdbc, whose loginTimeout is not
+ * a bound of the socket at all: Driver.connect hands the login to a daemon thread of its own
+ * and abandons it at the timeout, so an unbounded read there leaks a thread and a socket per
+ * borrow instead of failing one (CachedConnectionTestCase covers every one of them against a
+ * socket that never answers).
+ */
+ enum ConnectDialect {
+ /**
+ * postgresql: every property of the three takes seconds. connectTimeout covers the socket
+ * connect and socketTimeout the reads of the login: pgjdbc puts an SO_TIMEOUT on the login
+ * socket only where socketTimeout is set (ConnectionFactoryImpl.tryConnect, both before and
+ * after enableSSL), and it defaults to none. loginTimeout is kept on top of the two for a
+ * url naming more than one host, where each of them costs a login of its own - the connect
+ * is one budget for all of them, taken from the single System.nanoTime() in front of the
+ * loop over the hosts - but it is not a bound this class could rely on alone: Driver.connect
+ * runs the login on a daemon thread, gives up on the thread rather than on the login, and
+ * the thread stays parked in the read for as long as the read lasts.
+ */
+ POSTGRES("jdbc:postgresql:", '?',
+ new String[]{"connectTimeout", "loginTimeout"}, 1, 0,
+ new String[]{"socketTimeout"}, 1, true,
+ new int[]{}, new int[]{}),
+ /** mysql: both properties take milliseconds; socketTimeout is a socket read timeout that outlives the login. */
+ MYSQL("jdbc:mysql:", '?',
+ new String[]{"connectTimeout"}, 1000, 0,
+ new String[]{"socketTimeout"}, 1000, true,
+ new int[]{1040, 1203}, new int[]{1053}),
+ /** oracle: both properties take milliseconds; ReadTimeout is a socket read timeout that outlives the login. */
+ ORACLE("jdbc:oracle:", '?',
+ new String[]{"oracle.net.CONNECT_TIMEOUT"}, 1000, 0,
+ // the read bound goes by two names the driver reads: the property set here and the
+ // property of oracle net it stands for, inside a tns descriptor by the last segment of
+ // either. A bound under one of them is a bound of the administrator, so ours is not set
+ // on top of it - and neither of theirs is lifted with ours once the login is through.
+ // Only the first of the two is a name the driver also reads out of the system
+ // properties (SYSTEM_PROPERTY_NAMES below): oracle.net.READ_TIMEOUT reaches the socket
+ // from the connection properties alone, so a -D of it bounds nothing and must not be
+ // taken for a bound of theirs.
+ // RECV_TIMEOUT is not one of them: it is a parameter of sqlnet.ora and of the listener,
+ // and the name does not appear in ojdbc8 at all, so a descriptor carrying one would
+ // have taken our bound off a connection that never had one of its own.
+ new String[]{"oracle.jdbc.ReadTimeout", "oracle.net.READ_TIMEOUT"}, 1000, true,
+ // ORA-01033 and ORA-01034: the instance is starting up or not there yet; ORA-01089:
+ // it is shutting down. ORA-12514 is left out of these on purpose - a listener that
+ // does not know the service is also what a service name of a typo looks like, forever
+ new int[]{20, 12516, 12518, 12519, 12520}, new int[]{1033, 1034, 1089}),
+ /**
+ * ms sql server: loginTimeout takes seconds, socketTimeout milliseconds; the latter is a
+ * socket read timeout that outlives the login. loginTimeout is the one property of the
+ * four with a range of its own - SQLServerDriverIntProperty.LOGIN_TIMEOUT is validated
+ * against [0, 65535], and a value beyond it fails every connect the driver is asked for.
+ */
+ MICROSOFT("jdbc:sqlserver:", ';',
+ new String[]{"loginTimeout"}, 1, 65535,
+ new String[]{"socketTimeout"}, 1000, true,
+ // 921 and 922: the database has not been recovered yet, or is being recovered; 927: it
+ // is in the middle of a restore; 40613: azure sql reporting it not available for now
+ new int[]{17809, 10928, 10929}, new int[]{921, 922, 927, 40613});
+
+ final String urlPrefix;
+ /** the character that separates the parameters of this dialect from the url in front of them */
+ final char parameterSeparator;
+ /** the properties bounding the connect: the socket connect, and whatever the driver wraps it in */
+ final String[] connectProperties;
+ final int connectUnitsPerSecond;
+ /** the largest value the driver accepts for a connect property, 0 for a driver that takes any */
+ final long maxConnectSeconds;
+ /** the read bound of the login: the first name is the one set here, the rest are the names it also goes by */
+ final String[] readProperties;
+ final int readUnitsPerSecond;
+ /** whether the read bound of the login stays in force for every statement issued afterwards */
+ final boolean readBoundOutlivesLogin;
+ /** the vendor codes of this dialect for "no further connection is accepted" */
+ final int[] connectionLimitCodes;
+ /** the vendor codes of this dialect for "not accepting connections yet": a database on its way up */
+ final int[] notAcceptingYetCodes;
+
+ ConnectDialect(String urlPrefix, char parameterSeparator,
+ String[] connectProperties, int connectUnitsPerSecond, long maxConnectSeconds,
+ String[] readProperties, int readUnitsPerSecond, boolean readBoundOutlivesLogin,
+ int[] connectionLimitCodes, int[] notAcceptingYetCodes) {
+ this.urlPrefix = urlPrefix;
+ this.parameterSeparator = parameterSeparator;
+ this.connectProperties = connectProperties;
+ this.connectUnitsPerSecond = connectUnitsPerSecond;
+ this.maxConnectSeconds = maxConnectSeconds;
+ this.readProperties = readProperties;
+ this.readUnitsPerSecond = readUnitsPerSecond;
+ this.readBoundOutlivesLogin = readBoundOutlivesLogin;
+ this.connectionLimitCodes = connectionLimitCodes;
+ this.notAcceptingYetCodes = notAcceptingYetCodes;
+ }
+
+ /** The dialect of a connection string, or null for a driver whose property names are not known here. */
+ static ConnectDialect of(String connectionString) {
+ final String url = connectionString.toLowerCase(Locale.ROOT);
+ for (final ConnectDialect dialect : values()) {
+ if (url.startsWith(dialect.urlPrefix)) {
+ return dialect;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Fills in the properties bounding one connect attempt, leaving out what the administrator
+ * bounded themselves - an explicit setting of theirs keeps precedence, on the SQL Server,
+ * mysql and oracle drivers because a supplied property outranks the url, and on postgresql
+ * because the url outranks the property. A driver with a range of its own for its connect
+ * property is not handed a value beyond it: a bound it rejects is no bound at all, it is a
+ * connect that never happens.
+ * Returns whether a read bound outliving the login was set and has to be lifted once the
+ * connection is established.
+ */
+ boolean bound(String connectionString, Properties properties, long timeoutSeconds) {
+ final long connectSeconds = maxConnectSeconds > 0
+ ? Math.min(timeoutSeconds, maxConnectSeconds) : timeoutSeconds;
+ // The connect side is one budget rather than a set of independent knobs, so a bound of
+ // the administrator under any of its names leaves all of them alone. On postgresql
+ // connectTimeout bounds the socket connect and loginTimeout the login behind it:
+ // filling in the one they left out caps the one they set, and a "?connectTimeout=300"
+ // answered with a loginTimeout of ours is a login pgjdbc gives up on at 30 s - Driver
+ // .connect branches into its own thread as soon as loginTimeout is anything but 0.
+ if (!declared(connectionString, connectProperties)) {
+ for (final String property : connectProperties) {
+ properties.setProperty(property, Long.toString(connectSeconds * connectUnitsPerSecond));
+ }
+ }
+ reportBoundTurnedOffInUrl(connectionString);
+ if (!declared(connectionString, readProperties)) {
+ properties.setProperty(readProperties[0], Long.toString(timeoutSeconds * readUnitsPerSecond));
+ return readBoundOutlivesLogin;
+ }
+ return false;
+ }
+
+ /**
+ * Reports a url that turns a bound off where nothing this class supplies can put one back.
+ * A parameter of a postgresql url outranks the property this class hands the driver, so a
+ * "socketTimeout=0" there is not a default to be replaced - it is the administrator asking
+ * for an unbounded read, and a borrow that meets a database accepting the connection and
+ * answering nothing is then parked with no deadline able to reach it.
+ */
+ private void reportBoundTurnedOffInUrl(String connectionString) {
+ if (!urlOutranksProperties()) {
+ return;
+ }
+ // Every one of them, and keyed by the property rather than by the url alone: a url
+ // turns off the read bound and the login bound both ("?socketTimeout=0&loginTimeout=0",
+ // where the second is the per-host budget of a failover url), and safeUrl() keeps none
+ // of the timeout parameters - so a single key would report the first offender and
+ // leave the administrator to find the rest of them on their own.
+ for (final String[] properties : new String[][]{readProperties, connectProperties}) {
+ for (final String property : properties) {
+ final String value = parameterValue(connectionString, property);
+ if (value != null && !isBound(value)) {
+ warnOnce(safeUrl(connectionString) + "|unbounded|" + property,
+ "%s sets \"%s=%s\": a parameter of a postgresql url outranks the property this backend"
+ + " supplies, so that phase of a connect carries no bound. An operation reaching a"
+ + " database that accepts the connection and does not answer stays parked",
+ safeUrl(connectionString), property, value);
+ }
+ }
+ }
+ }
+
+ /** Whether a vendor code of this dialect is one that waiting for the database can clear. */
+ boolean isWorthRetrying(int errorCode) {
+ return contains(connectionLimitCodes, errorCode) || contains(notAcceptingYetCodes, errorCode);
+ }
+
+ private static boolean contains(int[] codes, int code) {
+ for (final int candidate : codes) {
+ if (candidate == code) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // Whether the administrator bounded one of these properties themselves. The dialects
+ // separate their parameters differently - "?a=1&b=2" (postgresql, mysql), ";a=1;b=2" (sql
+ // server), "(A=1)" inside the descriptor of an oracle tns url, where the property also goes
+ // by the last segment of its name alone - so a parameter is recognized by the delimiter in
+ // front of it and the "=" behind it rather than by parsing the url syntax of every driver.
+ // The connection string is not the only channel of theirs: the oracle driver reads some of
+ // its properties out of the system properties as well, which is how a whole jvm is bounded
+ // with -Doracle.jdbc.ReadTimeout, and a property supplied to a driver outranks the system
+ // property without a word - and would then be lifted after the login as if it were ours,
+ // leaving a connection with no read bound where the administrator had set one.
+ private boolean declared(String connectionString, String... properties) {
+ for (final String property : properties) {
+ if (declaredInUrl(connectionString, property) || setAsSystemProperty(property)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** Whether the connection string bounds this property, under its own name or the last segment of it. */
+ private boolean declaredInUrl(String connectionString, String property) {
+ if (containsParameter(connectionString, property)) {
+ return true;
+ }
+ final int dot = property.lastIndexOf('.');
+ return dot >= 0 && containsParameter(connectionString, property.substring(dot + 1));
+ }
+
+ // Which names a driver reads out of the system properties, listed rather than told from
+ // the shape of the name: ojdbc8 resolves oracle.jdbc.ReadTimeout and
+ // oracle.net.CONNECT_TIMEOUT in three tiers (the properties it was supplied, then
+ // System.getProperty, then the properties of the data source), while
+ // oracle.net.READ_TIMEOUT - a dotted name of the same driver - is read out of the
+ // connection properties alone: the six classes carrying the literal hand it to
+ // Properties.get, and none of them to System.getProperty. Taking a -D of it for a bound of
+ // the administrator would leave the login with no read bound at all - theirs not read by
+ // the driver and ours not set, because we believed theirs was in force.
+ private static final Set<String> SYSTEM_PROPERTY_NAMES = Collections.unmodifiableSet(new HashSet<>(
+ Arrays.asList("oracle.jdbc.ReadTimeout", "oracle.net.CONNECT_TIMEOUT")));
+
+ private static boolean setAsSystemProperty(String property) {
+ return SYSTEM_PROPERTY_NAMES.contains(property) && isBound(System.getProperty(property));
+ }
+
+ // pgjdbc parses the url over the properties it was handed - Driver.connect copies them
+ // into a flat map and parseURL then writes the parameters of the url on top - so a value
+ // standing in a postgresql url is the value the driver uses, and the one supplied here
+ // never reaches the socket. A zero there is not a default of the driver to be replaced: it
+ // cannot be replaced, and setting ours on top of it would leave this class lifting a read
+ // bound the login never had. The other three let a supplied property win, so a zero of
+ // theirs is ours to override.
+ private boolean urlOutranksProperties() {
+ return this == POSTGRES;
+ }
+
+ /** Whether this property is bounded by the connection string, as the driver of this dialect reads it. */
+ private boolean containsParameter(String connectionString, String property) {
+ final String value = parameterValue(connectionString, property);
+ return value != null && (urlOutranksProperties() || isBound(value));
+ }
+
+ // Matched the way the driver of this dialect matches it: pgjdbc and Connector/J look their
+ // properties up by their exact name - PropertyKey.fromValue answers null for a name of
+ // another case and the parameter is then a parameter of nobody, so "?SocketTimeout=" must
+ // not be taken for a bound of the administrator - while the SQL Server driver
+ // (getNormalizedPropertyName) and the keywords of an oracle descriptor match either way.
+ private String parameterValue(String connectionString, String property) {
+ final boolean exact = this == POSTGRES || this == MYSQL;
+ final String url = exact ? connectionString : connectionString.toLowerCase(Locale.ROOT);
+ final String name = exact ? property : property.toLowerCase(Locale.ROOT);
+ String value = null;
+ for (int i = url.indexOf(name); i >= 0; i = url.indexOf(name, i + name.length())) {
+ final int end = i + name.length();
+ if (i > 0 && "?&;(,".indexOf(url.charAt(i - 1)) >= 0 && end < url.length() && url.charAt(end) == '=') {
+ // the last of them: a driver parsing a url into a map lets the last assignment stand
+ value = valueOf(url, end + 1);
+ }
+ }
+ return value;
+ }
+
+ /** The value of the parameter that starts here: up to the delimiter in front of the next one. */
+ private static String valueOf(String url, int from) {
+ int end = from;
+ while (end < url.length() && "&;),?".indexOf(url.charAt(end)) < 0) {
+ end++;
+ }
+ return url.substring(from, end);
+ }
+
+ /**
+ * Whether a value of the administrator bounds anything. Every one of these drivers reads 0
+ * as "wait as long as it takes", so a property set to it is not a bound of theirs to stay
+ * out of the way of - it is the default this class exists to replace, and one of ours goes
+ * on top of it wherever a supplied property outranks the url. Where it does not, on
+ * postgresql, the zero stands and is reported instead of being written over. A value that
+ * is no number is left to the driver it belongs to.
+ */
+ private static boolean isBound(String value) {
+ if (value == null || value.trim().isEmpty()) {
+ return false;
+ }
+ try {
+ return Double.parseDouble(value.trim()) != 0; // pgjdbc takes a float for its loginTimeout
+ } catch (NumberFormatException notANumber) {
+ return true;
+ }
+ }
}
final String connectionString;
+ /**
+ * Whether this connection may go back into the pool once it is closed. A connection carrying a
+ * read bound that could not be lifted serves the borrower waiting for it and is closed
+ * afterwards: left in the pool it would fail every statement slower than that bound - an
+ * import batch among them - for every borrow the pool hands it to.
+ */
+ private final boolean poolable;
+
+ /**
+ * When this connection last answered the database, as a {@link System#nanoTime()} reading:
+ * established - the login and the two round trips that set it up have just answered - or
+ * validated. It is never stamped on the way back into the pool, although that is where a
+ * connection has most recently been used: {@link #close()} ends the transaction, and pgjdbc
+ * short-circuits both {@code rollback()} and {@code commit()} when the transaction state is
+ * IDLE, so on a borrow that issued no statement - {@code JDBCStorage.open()}, a configuration
+ * change that leaves the base DNs alone, an import of nothing - not a byte reaches the server
+ * and the stamp would prove nothing, while marking a connection the database may have dropped
+ * as the freshest one in the pool. Stamping proof rather than use makes the window mean
+ * "validated at most once per window", which is a claim this class can always back.
+ * <p>
+ * It stands for the moment the connection was <em>asked</em>, not the moment its answer was
+ * filed: {@link #distrustPool} is compared against it as an ordering of two moments, and a
+ * proof that took a second to arrive would otherwise outlive a drop reported while it was
+ * still in flight. Reading it early only ever ages the proof, which costs a validation and
+ * never skips one.
+ */
+ private volatile long lastKnownAliveNanos;
+
public CachedConnection(String connectionString, Connection parent) {
+ this(connectionString, parent, true);
+ }
+
+ CachedConnection(String connectionString, Connection parent, boolean poolable) {
this.connectionString = connectionString;
this.parent = parent;
+ this.poolable = poolable;
+ this.lastKnownAliveNanos = System.nanoTime();
}
+ /**
+ * Borrows a connection: a usable one out of the pool, or a newly established one. Bounded in
+ * both phases - every operation of this backend, the open of a backend and the import
+ * included, comes through here, and an unbounded borrow turns a database that listens but does
+ * not answer into a hang rather than into an error the caller can report.
+ */
static Connection getConnection(String connectionString) throws Exception {
- return getConnection(connectionString, 0);
+ return getConnection(connectionString, true);
}
- static Connection getConnection(String connectionString, final int waitTime) throws Exception {
- CachedConnection con = cached.get(connectionString).poll(waitTime, TimeUnit.MILLISECONDS);
-
- while (con != null) {
- if (!con.isValid(0)) {
- try {
- con.parent.close();
- } catch (SQLException e) {
- con = null;
+ /**
+ * Borrows a connection, either trusting the alive window of {@value #ALIVE_BYPASS_PROPERTY} or
+ * validating whatever comes out of the pool.
+ *
+ * @param trusted false for a borrow nothing compensates a dropped connection on. What the
+ * window trades away is the connection that breaks inside it, and {@code JDBCStorage} takes
+ * that off the caller where it can - a write is replayed, a read tells the pool - but the
+ * borrows that open a backend, remove its files or start an import have neither: they issue
+ * their statements far from the borrow, and the one that opens a backend issues none at all,
+ * so a dropped connection would surface out of the {@code rollback()} of its release. Each of
+ * them is one borrow of a cold path, where the round trip the window saves is worth nothing.
+ */
+ static Connection getConnection(String connectionString, boolean trusted) throws Exception {
+ final ConnectDialect dialect = ConnectDialect.of(connectionString);
+ reportUnknownDialect(connectionString, dialect);
+ final long connectTimeoutSeconds = Math.min(
+ getNonNegativeProperty(CONNECT_TIMEOUT_PROPERTY, DEFAULT_CONNECT_TIMEOUT_SECONDS, "s"),
+ Integer.MAX_VALUE / 1000);
+ final long poolTimeoutSeconds = getNonNegativeProperty(POOL_TIMEOUT_PROPERTY, DEFAULT_POOL_TIMEOUT_SECONDS, "s");
+ final long startedAt = System.currentTimeMillis();
+ final long deadline = (poolTimeoutSeconds == 0 || poolTimeoutSeconds >= Long.MAX_VALUE / 1000)
+ ? Long.MAX_VALUE : startedAt + poolTimeoutSeconds * 1000;
+ long waitMs = 0;
+ long backoffMs = 0;
+ int attempts = 0;
+ while (true) {
+ final CachedConnection pooled = poll(connectionString, waitMs, deadline, trusted);
+ if (pooled != null) {
+ return pooled;
+ }
+ attempts++;
+ try {
+ return connect(connectionString, dialect, attemptSeconds(connectTimeoutSeconds, deadline));
+ } catch (SQLException e) {
+ // A database that takes no connection for the moment is the failure worth waiting
+ // out: it is at its connection limit, and one of ours is going to come back to the
+ // pool - or it is on its way up, and the state clears itself in seconds. Everything
+ // else - a password that is not accepted, a database that is down, a driver that is
+ // not on the classpath - is reported to the caller instead of being retried behind
+ // its back.
+ if (!isWorthRetrying(e, dialect)) {
+ throw reported(e, connectionString);
}
- con = cached.get(connectionString).poll();
- } else {
- return con;
+ final long remaining = deadline - System.currentTimeMillis();
+ if (remaining <= 0) {
+ // 08001, the state of a connect that did not happen, rather than none at all:
+ // this is the failure of a borrow, and a caller reading the state of what it
+ // caught would otherwise see null where the driver's own exception carried one
+ final SQLTimeoutException timeout = new SQLTimeoutException("no connection to "
+ + safeUrl(connectionString) + " could be borrowed within " + poolTimeoutSeconds + "s ("
+ + attempts + " attempts): the database took no connection for the moment and none was"
+ + " returned to the pool, last error: " + redact(e.getMessage(), connectionString),
+ CONNECT_FAILED_SQL_STATE);
+ timeout.initCause(reported(e, connectionString));
+ throw timeout;
+ }
+ backoffMs = Math.min(backoffMs == 0 ? 1 : backoffMs * 2, MAX_BACKOFF_MS);
+ waitMs = Math.min(backoffMs, remaining);
+ warnStall(connectionString, attempts, startedAt, e);
+ } catch (RuntimeException e) {
+ // a driver reporting a connect it will not make as an unchecked failure carries the
+ // connection string of the backend in its message as readily as a SQLException does
+ throw reportedUnchecked(e, connectionString);
}
}
- Connection conNew = null;
+ }
+
+ /**
+ * Reports a connection string this class knows no bound for. The properties bounding a connect
+ * are the ones of a driver, so a driver outside the four leaves every attempt unbounded - and
+ * the deadline of the borrow cannot reach into a connect that is already under way, since the
+ * driver is the only thing holding the socket.
+ */
+ private static void reportUnknownDialect(String connectionString, ConnectDialect dialect) {
+ if (dialect != null) {
+ return;
+ }
+ final StringBuilder known = new StringBuilder();
+ for (final ConnectDialect candidate : ConnectDialect.values()) {
+ known.append(known.length() > 0 ? ", " : "").append(candidate.urlPrefix);
+ }
+ warnOnce(safeUrl(connectionString) + "|unknown-dialect",
+ "%s names a driver whose timeout properties are not known to this backend (%s are): a connect to a"
+ + " database that accepts it and does not answer is left without a bound, and the %s property"
+ + " cannot end it",
+ safeUrl(connectionString), known, POOL_TIMEOUT_PROPERTY);
+ }
+
+ /**
+ * The bound of one connect attempt. The deadline of the borrow bounds it as well - the
+ * {@value #POOL_TIMEOUT_PROPERTY} property stands for the whole borrow, and an attempt of its
+ * own left to run out would overrun it by a full connect timeout. That holds for an attempt
+ * the {@value #CONNECT_TIMEOUT_PROPERTY} property gives no bound of its own, too: turning the
+ * per-attempt bound off must not turn the bound of the borrow off with it. Never 0 for an
+ * attempt that is bounded at all: 0 is the value that stands for no bound. And never past what
+ * an int of milliseconds takes - the pool timeout has no upper bound of its own, while the SQL
+ * Server driver rejects a socketTimeout beyond Integer.MAX_VALUE outright, failing every
+ * connect of that backend with the name of a property nobody typed.
+ */
+ static long attemptSeconds(long connectTimeoutSeconds, long deadline) {
+ if (deadline == Long.MAX_VALUE) {
+ // 0 stands for an attempt with no bound of its own and stays 0; anything else is
+ // clamped here as well, so that the range holds whichever branch answers
+ return connectTimeoutSeconds == 0 ? 0 : Math.min(connectTimeoutSeconds, Integer.MAX_VALUE / 1000);
+ }
+ final long remainingSeconds = (deadline - System.currentTimeMillis() + 999) / 1000;
+ final long bound = connectTimeoutSeconds == 0
+ ? remainingSeconds : Math.min(connectTimeoutSeconds, remainingSeconds);
+ return Math.max(1, Math.min(bound, Integer.MAX_VALUE / 1000));
+ }
+
+ /**
+ * Takes a usable connection out of the pool, waiting up to waitMs for one to be returned to it.
+ * The validation of a connection costs a round trip, and the pool has no upper bound on the
+ * number of them it holds, so draining a pool the database no longer answers is given the
+ * deadline of the borrow as well: past it, establishing a connection is the faster answer.
+ * The connection in hand is always looked at first - trusted or validated, see
+ * {@link #isKnownAlive} - whatever the deadline says: a database at its connection limit has
+ * no other source of connections than the ones coming back, and one returned to the pool a
+ * moment before the deadline is the very connection this borrow waited for. Only a connection
+ * the database no longer answers is closed here.
+ */
+ private static CachedConnection poll(String connectionString, long waitMs, long deadline, boolean trusted)
+ throws InterruptedException {
+ CachedConnection con = cached.get(connectionString).pollFirst(waitMs, TimeUnit.MILLISECONDS);
+ while (con != null) {
+ if (isUsable(con, trusted)) {
+ return con;
+ }
+ closeQuietly(con.parent);
+ if (System.currentTimeMillis() >= deadline) {
+ return null;
+ }
+ con = cached.get(connectionString).pollFirst();
+ }
+ return null;
+ }
+
+ private static boolean isUsable(CachedConnection con, boolean trusted) {
+ if (trusted && isKnownAlive(con)) {
+ return true;
+ }
+ // The validation needs a bound of its own: isValid(0) means "no timeout" in the JDBC
+ // contract, and a connection whose socket is half-open answers it no sooner than it
+ // answers anything else. isValid(n) is not that bound on every driver either - the SQL
+ // Server driver turns it into a query timeout (setQueryTimeout, then "SELECT 1"), which
+ // needs an answer from the server to fire at all - so the socket is bounded here, for the
+ // validation only.
+ final int restore = boundValidation(con.parent);
+ if (restore == VALIDATION_BOUND_FAILED) {
+ // the bound of the validation is not in force, and the driver may well have applied it
+ // before failing: validating here would be the unbounded isValid() this exists to
+ // avoid, and pooling it would hand out a connection carrying a bound of ours
+ return false;
+ }
+ // Read before the round trip rather than after it: this stamp is what distrustPool() is
+ // compared against, as an ordering of two moments. A validation is allowed
+ // VALIDATION_TIMEOUT_SECONDS, so a stamp filed once the answer is in can be younger than a
+ // drop another operation reported while it was still in flight - and the connection would
+ // then be trusted for the rest of the window by the very check that exists to stop it.
+ final long provenAt = System.nanoTime();
+ boolean usable;
try {
- conNew = DriverManager.getConnection(connectionString);
+ usable = con.isValid(VALIDATION_TIMEOUT_SECONDS);
+ } catch (SQLException | RuntimeException e) { // a driver reporting the validation as an error: discard it
+ // an unchecked failure out of a driver would unwind through poll(), which stands
+ // outside every try of the borrow, and leave this connection dequeued and unclosed
+ usable = false;
+ }
+ if (!usable) {
+ // On its way out, and the driver knows it: Connector/J answers a failed validation by
+ // aborting the connection and the SQL Server driver by terminating it, so putting the
+ // previous bound back would fail as well - and warn about a bound of a connection that
+ // is about to be closed, over a reaped idle connection that is nobody's problem.
+ return false;
+ }
+ if (restore >= 0 && !setNetworkTimeout(con.parent, restore,
+ "the connection is closed rather than pooled")) {
+ return false; // it would carry the bound of the validation into every statement
+ }
+ con.lastKnownAliveNanos = provenAt;
+ return true;
+ }
+
+ /** A connection left alone by {@link #boundValidation}: no bound of ours to put back afterwards. */
+ private static final int VALIDATION_BOUND_LEFT_ALONE = -1;
+ /** A connection {@link #boundValidation} could not bound, which may still carry the bound it failed to report. */
+ private static final int VALIDATION_BOUND_FAILED = -2;
+
+ /**
+ * Whether a connection can be handed out on the strength of the last answer it gave, without a
+ * round trip to ask for another. Three things have to hold: the window is on, the answer is
+ * younger than it, and nothing has reported since that the database dropped a connection of
+ * this pool.
+ * <p>
+ * What the window trades away is the connection that breaks inside it: it is handed out, and
+ * the failure surfaces on the statement of the caller rather than on the borrow. That is where
+ * a connection breaking mid-operation surfaces anyway - but not every caller of this backend
+ * reports such a failure to the client, so the trade is not the caller's alone to bear.
+ * {@code JDBCStorage} answers it on both sides: a write is replayed on a connection the next
+ * attempt borrows of its own, and a read as much as a write marks the pool distrusted, which
+ * closes the window for the rest of the generation the dropped connection belonged to.
+ */
+ private static boolean isKnownAlive(CachedConnection con) {
+ final long window = aliveBypassNanos;
+ if (window <= 0) {
+ return false;
+ }
+ final long provenAt = con.lastKnownAliveNanos;
+ if (System.nanoTime() - provenAt >= window) { // the overflow safe form of the comparison
+ return false;
+ }
+ final Long distrusted = poolDistrustedAt.get(con.connectionString);
+ if (distrusted != null && provenAt - distrusted <= 0) { // the overflow safe form of the comparison
+ return false;
+ }
+ // What the validation this replaces also answered: the removalListener above closes every
+ // connection it finds in the deque when the pool expires, and it iterates a weakly
+ // consistent view, so a connection taken out by a borrow running at the same time can be
+ // closed under it. Answered by the driver out of a flag of its own, not by a round trip.
+ return !isClosed(con.parent);
+ }
+
+ /** Whether the driver reports the connection as closed; one that cannot say is not one to trust. */
+ private static boolean isClosed(Connection con) {
+ try {
+ return con.isClosed();
+ } catch (SQLException e) {
+ return true;
+ }
+ }
+
+ /**
+ * Reports that the database dropped a connection of this pool, so that no connection proven
+ * alive before now is handed out unvalidated again. It is called by the operation that saw the
+ * failure: this class only ever learns of one from the statement it broke, since a borrow
+ * inside the window asks the database nothing.
+ */
+ static void distrustPool(String connectionString) {
+ // merge(later of the two) rather than computeIfAbsent().set(): two operations reporting a
+ // drop at once would otherwise move the distrust point backwards - the later reading is
+ // written first and the earlier one overwrites it - and the AtomicLong of computeIfAbsent
+ // is published holding its initial 0 before set() runs, which a borrow racing it reads as
+ // "never". Not Math.max: nanoTime() has no defined origin, so the readings are compared by
+ // their difference, the way every other comparison of one in this class is.
+ poolDistrustedAt.merge(connectionString, System.nanoTime(),
+ (reported, now) -> now - reported > 0 ? now : reported);
+ }
+
+ /**
+ * Bounds the socket of a pooled connection for the length of its validation, returning the
+ * network timeout to put back afterwards - or {@link #VALIDATION_BOUND_LEFT_ALONE} for a
+ * connection left alone, either because the driver does not take a network timeout or because
+ * it is bounded at least as tightly already, by a read timeout of the connection string that
+ * is not ours to widen.
+ * A driver that takes the call and then fails inside it is told apart from both: it is free to
+ * have applied the bound before failing, and a connection put back into the pool carrying five
+ * seconds of ours fails every statement slower than that for the rest of its life.
+ */
+ private static int boundValidation(Connection con) {
+ final int bound = VALIDATION_TIMEOUT_SECONDS * 1000;
+ final int previous;
+ try {
+ previous = con.getNetworkTimeout();
+ } catch (SQLException | RuntimeException e) { // a driver that does not take one: nothing was changed
+ return VALIDATION_BOUND_LEFT_ALONE;
+ }
+ if (previous > 0 && previous <= bound) {
+ return VALIDATION_BOUND_LEFT_ALONE;
+ }
+ try {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, bound);
+ } catch (SQLException | RuntimeException e) {
+ return VALIDATION_BOUND_FAILED;
+ }
+ // A driver answering a negative timeout is outside the contract of getNetworkTimeout(),
+ // where 0 stands for no limit and nothing below it stands for anything. Handed back as it
+ // is, it would be one of the two sentinels above: the bound just set would be read as a
+ // bound that was never set, and the connection would go into the pool carrying five
+ // seconds of ours into every statement of whoever borrows it next.
+ return previous < 0 ? 0 : previous;
+ }
+
+ static CachedConnection connect(String connectionString, ConnectDialect dialect, long connectTimeoutSeconds)
+ throws SQLException {
+ // A driver is free to write into the map it is handed, so it gets one of its own.
+ final Properties properties = new Properties();
+ final boolean readBoundSet = dialect != null && connectTimeoutSeconds > 0
+ && dialect.bound(connectionString, properties, connectTimeoutSeconds);
+ // Read before the connect rather than after it, for the reason isUsable() reads it before
+ // the validation: the login answered somewhere inside this attempt, and a stamp taken once
+ // it returned could outlive a drop reported while it was still going on.
+ final long provenAt = System.nanoTime();
+ final Connection conNew = DriverManager.getConnection(connectionString, properties);
+ boolean poolable = true;
+ try {
+ // still under the read bound: both of these are round trips of their own
conNew.setAutoCommit(false);
conNew.setTransactionIsolation(TRANSACTION_READ_COMMITTED);
- return new CachedConnection(connectionString, conNew);
- } catch (SQLException e) { // max_connection server error: try recursion for reuse connection
- if (conNew != null) { // the connection was established but not set up: nothing else would close it
- try {
- conNew.close();
- } catch (SQLException e2) {}
+ if (readBoundSet) {
+ // a driver that will not take the bound back has warned about it already: the
+ // connection serves the borrower that is waiting for it and is closed rather than
+ // pooled, so the bound of the login does not outlive it in the pool
+ poolable = relaxReadBound(conNew, connectTimeoutSeconds);
}
- return getConnection(connectionString, (waitTime == 0) ? 1 : waitTime * 2);
+ } catch (SQLException | RuntimeException e) { // nothing holds this connection yet: it would leak
+ closeQuietly(conNew);
+ throw e;
+ }
+ final CachedConnection established = new CachedConnection(connectionString, conNew, poolable);
+ established.lastKnownAliveNanos = provenAt;
+ return established;
+ }
+
+ // The second bound of the login is a socket read timeout on mysql, oracle and sql server, in
+ // force for the whole life of the connection: left in place it would break every statement
+ // slower than it - an import batch, the statistics of a freshly loaded table - so it is lifted
+ // as soon as the login is through, restoring the behaviour of a connection this class
+ // established before. A read bound the connection string sets itself is never touched here:
+ // it is not set at all, so nothing of the administrator's is lifted along with it. Returns
+ // whether the bound is gone - a connection still carrying it must not be pooled.
+ // Named by the bound the login was given rather than by the property it came from: with
+ // CONNECT_TIMEOUT_PROPERTY at 0 the attempt takes its bound from what is left of the deadline
+ // of the borrow, so naming that property would point at the one setting that is not in force.
+ private static boolean relaxReadBound(Connection con, long boundSeconds) {
+ return setNetworkTimeout(con, 0, "statements taking longer than the " + boundSeconds
+ + "s the login of this connection was bounded by fail on it, and it is closed rather than pooled");
+ }
+
+ /**
+ * Puts a network timeout on a connection, reporting a driver that will not take one. The
+ * consequence is the caller's to name: the same failure ends a freshly established connection
+ * carrying the read bound of its login and a pooled one whose bound could not be put back.
+ */
+ private static boolean setNetworkTimeout(Connection con, int millis, String consequence) {
+ try {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, millis);
+ return true;
+ } catch (SQLException | RuntimeException e) {
+ // Throttled rather than reported once for the life of the JVM: every connection this
+ // happens to carries a read bound it was never meant to keep, and a statement dying of
+ // it hours later needs a warning of its own to be traced back to here.
+ final long now = System.currentTimeMillis();
+ final long last = lastReadBoundWarning.get();
+ if (now - last >= STALL_WARNING_INTERVAL_MS && lastReadBoundWarning.compareAndSet(last, now)) {
+ logger.warn(LocalizableMessage.raw(
+ "The read bound of a JDBC connection could not be set to %d ms (%s): %s",
+ millis, e.getMessage(), consequence));
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Whether the database took no connection for the moment, rather than refusing one for good:
+ * it is at its connection limit - one of our own connections is on its way back to the pool -
+ * or it is not accepting connections yet, the state a database on its way up reports while it
+ * recovers - 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. Both clear themselves in
+ * seconds; every other failure is the caller's to see.
+ */
+ 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()
+ final Deque<Throwable> pending = new ArrayDeque<>();
+ final Set<Throwable> visited = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
+ enqueue(pending, visited, e);
+ 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 stall has to reach the server log: without it a database accepting no further connection
+ // is indistinguishable from a hang. Throttled, since every operation of the backend borrows
+ // through here and would otherwise log a copy of its own.
+ private static void warnStall(String connectionString, int attempts, long startedAt, SQLException cause) {
+ final long now = System.currentTimeMillis();
+ if (now - startedAt < STALL_WARNING_AFTER_MS) {
+ return;
+ }
+ final AtomicLong lastOfThisUrl =
+ lastStallWarning.computeIfAbsent(safeUrl(connectionString), url -> new AtomicLong());
+ final long last = lastOfThisUrl.get();
+ if (now - last >= STALL_WARNING_INTERVAL_MS && lastOfThisUrl.compareAndSet(last, now)) {
+ logger.warn(LocalizableMessage.raw("%s", stallMessage(connectionString, attempts, now - startedAt, cause)));
+ }
+ }
+
+ /**
+ * The stall as it reaches the log. Built apart from the logging of it so that the rule it has
+ * to keep - neither the connection string nor the message of the driver reaches a log as it
+ * stands - is a rule a test can hold it to.
+ */
+ static String stallMessage(String connectionString, int attempts, long waitedMs, SQLException cause) {
+ return String.format("%s takes no further connection: waiting %d ms for a pooled one so far (%d attempts),"
+ + " last error: %s", safeUrl(connectionString), waitedMs, attempts,
+ redact(cause.getMessage(), connectionString));
+ }
+
+ /**
+ * The failure of a connect as it may leave this class: the exception itself where nothing of it
+ * names the credentials of the backend, and a redacted rebuild of its whole chain where
+ * something does. Rebuilt rather than wrapped: a wrapper keeps its cause, and everything that
+ * prints a failure prints the causes along with it - a debug build of
+ * stackTraceToSingleLineString walks them, the config manager traces them, and
+ * RootContainer.open() makes the message of the cause the message of what it throws - so a
+ * link left as it stands would carry the password past the wrapper. The SQLState and the
+ * vendor code of every link survive it: they are what tells a caller what happened.
+ */
+ static SQLException reported(SQLException e, String connectionString) {
+ return holdsCredentials(e, connectionString)
+ ? redactedCopy(e, connectionString, new int[] { MAX_CHAIN_LENGTH })
+ : e;
+ }
+
+ /** The same of an unchecked failure: a driver is free to report a connect it will not make as one. */
+ static Exception reportedUnchecked(RuntimeException e, String connectionString) {
+ if (!holdsCredentials(e, connectionString)) {
+ return e;
+ }
+ final SQLException redacted = new SQLNonTransientConnectionException(e.getClass().getName()
+ + (e.getMessage() == null ? "" : ": " + redact(e.getMessage(), connectionString)),
+ CONNECT_FAILED_SQL_STATE);
+ redacted.setStackTrace(e.getStackTrace());
+ return redacted;
+ }
+
+ /**
+ * Whether anything in the chain of a failure names what a connection string keeps out of the
+ * log. A chain longer than this walk is given answers "yes": what is reported unredacted is
+ * what this class has looked at whole, and a link it never reached is not that. The cost of
+ * being wrong that way is a chain rebuilt - bounded in its turn - while the cost of being
+ * wrong the other way is the password of the backend in the server error log.
+ */
+ private static boolean holdsCredentials(Throwable failure, String connectionString) {
+ final Deque<Throwable> pending = new ArrayDeque<>();
+ final Set<Throwable> visited = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
+ enqueue(pending, visited, failure);
+ for (int links = 0; !pending.isEmpty(); links++) {
+ if (links >= MAX_CHAIN_LENGTH) {
+ return true;
+ }
+ final Throwable t = pending.poll();
+ final String message = t.getMessage();
+ if (message != null && !message.equals(redact(message, connectionString))) {
+ return true;
+ }
+ if (t instanceof SQLException) {
+ enqueue(pending, visited, ((SQLException) t).getNextException());
+ }
+ enqueue(pending, visited, t.getCause());
+ }
+ return false;
+ }
+
+ // By identity rather than by equals(): a link of a chain carries a cause and a next exception
+ // both, and a driver is free to make the two the same failure. Enqueued twice, a chain of
+ // those fans out into a copy of itself at every step and spends the budget of a walk on links
+ // it has already looked at - five levels of one are enough to exhaust MAX_CHAIN_LENGTH.
+ private static void enqueue(Deque<Throwable> pending, Set<Throwable> visited, Throwable t) {
+ if (t != null && visited.add(t)) {
+ pending.add(t);
+ }
+ }
+
+ // The budget counts the links this rebuilds, the way holdsCredentials() counts the ones it
+ // visits - not how deep it has gone. A link of a chain carries a cause and a next exception
+ // both, and a driver is free to make them the same failure, so a bound on depth alone leaves
+ // room for a chain that fans out into two copies of itself at every step.
+ private static SQLException redactedCopy(SQLException e, String connectionString, int[] budget) {
+ budget[0]--;
+ final SQLException copy =
+ new SQLException(redact(e.getMessage(), connectionString), e.getSQLState(), e.getErrorCode());
+ copy.setStackTrace(e.getStackTrace());
+ if (e.getNextException() != null) {
+ copy.setNextException(budget[0] > 0
+ ? redactedCopy(e.getNextException(), connectionString, budget) : droppedTail());
+ }
+ if (e.getCause() != null) {
+ copy.initCause(budget[0] > 0 ? redactedLink(e.getCause(), connectionString, budget) : droppedTail());
+ }
+ return copy;
+ }
+
+ // What stands where the budget ran out. Without it the same failure logs its root cause when
+ // the url of the backend has no password in it and loses it without a word when it has, which
+ // is a report of a connect nobody can read against a report of one they can.
+ private static SQLException droppedTail() {
+ return new SQLException("the rest of this failure was left out: a chain of more than "
+ + MAX_CHAIN_LENGTH + " links is rebuilt only that far");
+ }
+
+ // A link that is no SQLException keeps its class name in the message: its type is not one this
+ // can rebuild, and the name of the failure is what a reader of the log is after.
+ private static Throwable redactedLink(Throwable t, String connectionString, int[] budget) {
+ if (t instanceof SQLException) {
+ return redactedCopy((SQLException) t, connectionString, budget);
+ }
+ budget[0]--;
+ final Throwable copy = new Throwable(t.getClass().getName()
+ + (t.getMessage() == null ? "" : ": " + redact(t.getMessage(), connectionString)));
+ copy.setStackTrace(t.getStackTrace());
+ if (t.getCause() != null) {
+ copy.initCause(budget[0] > 0 ? redactedLink(t.getCause(), connectionString, budget) : droppedTail());
+ }
+ return copy;
+ }
+
+ /**
+ * A message of a driver as it may be logged. A driver is free to put the connection string it
+ * was handed into it - the jdk itself does, "No suitable driver found for " + url, which is
+ * what the ordinary oracle misconfiguration of a driver jar left out of lib/extensions arrives
+ * as - and that connection string is where the credentials of this backend live.
+ * What it cannot answer for is a driver quoting back a part of a url it failed to parse:
+ * a whole credential is replaced, a fragment of one is not.
+ */
+ static String redact(String message, String connectionString) {
+ if (message == null || message.isEmpty()) {
+ return message;
+ }
+ String redacted = message.replace(connectionString, safeUrl(connectionString));
+ // The parameter before the values: a password blanked here is one the loop below no longer
+ // finds, while the other way round a "<credentials hidden>" standing behind a "password="
+ // would be cut in half by a pattern that ends its value at the first space.
+ redacted = SECRET_PARAMETER.matcher(redacted).replaceAll("$1=***");
+ for (final String secret : secretsOf(connectionString)) {
+ redacted = secretPattern(secret).matcher(redacted)
+ .replaceAll(Matcher.quoteReplacement(CREDENTIALS_HIDDEN));
+ }
+ return redacted;
+ }
+
+ /**
+ * A secret as it is looked for in a message: the value itself, wherever it does not stand
+ * inside a longer run of letters and digits. A password is free to be one character long, and
+ * a bare one of those is a substring of half the lines a driver writes - a password of "1"
+ * takes "ORA-12541: TNS:no listener" apart into a line nobody can read, and it makes every
+ * failure of that backend one this class believes names the credentials, so the whole chain is
+ * rebuilt as well. A redaction that destroys the diagnostic it protects is the worse of the
+ * two failures. What a driver quotes back is a credential standing on its own - between the
+ * delimiters of a url, or in a sentence of its own - and that is still replaced.
+ */
+ private static Pattern secretPattern(String secret) {
+ final String before = isAlphanumeric(secret.charAt(0)) ? "(?<![A-Za-z0-9])" : "";
+ final String after = isAlphanumeric(secret.charAt(secret.length() - 1)) ? "(?![A-Za-z0-9])" : "";
+ return Pattern.compile(before + Pattern.quote(secret) + after);
+ }
+
+ private static boolean isAlphanumeric(char c) {
+ return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9');
+ }
+
+ /** What of a connection string must not stand in a message: the credentials safeUrl() takes out of it. */
+ private static List<String> secretsOf(String connectionString) {
+ final List<String> secrets = new ArrayList<>();
+ final ConnectDialect dialect = ConnectDialect.of(connectionString);
+ final String separators = dialect == null ? "?;" : String.valueOf(dialect.parameterSeparator);
+ final int scheme = connectionString.indexOf(':', "jdbc:".length()) + 1;
+ if (scheme <= 0) {
+ return secrets;
+ }
+ final int authority = startOfAuthority(connectionString, scheme);
+ if (authority < 0) {
+ final int at = connectionString.indexOf('@', scheme);
+ if (at > scheme) {
+ addSecret(secrets, connectionString.substring(credentialsStart(connectionString, scheme, at), at));
+ }
+ } else {
+ final int end = endOfAuthority(connectionString, authority, separators);
+ for (final String host : connectionString.substring(authority, end).split(",", -1)) {
+ final int at = host.lastIndexOf('@');
+ if (at > 0) {
+ addSecret(secrets, host.substring(0, at));
+ }
+ }
+ }
+ final Matcher parameter = SECRET_PARAMETER.matcher(connectionString);
+ while (parameter.find()) {
+ addSecret(secrets, parameter.group(3));
+ }
+ return secrets;
+ }
+
+ // The credentials of one host, and the password inside them without the user name in front of
+ // it: a driver quoting a url back names either.
+ private static void addSecret(List<String> secrets, String credentials) {
+ if (credentials.isEmpty()) {
+ return;
+ }
+ secrets.add(credentials);
+ final int password = indexOfAny(credentials, ":/", 0);
+ if (password >= 0 && password + 1 < credentials.length()) {
+ secrets.add(credentials.substring(password + 1));
+ }
+ }
+
+ // The connection string carries the credentials of the backend - JDBCStorage hands the whole
+ // db-directory of the configuration to this class, so the url is the only place they live -
+ // and it is never logged as it stands. Three shapes hold them and all three are taken off: the
+ // "user/password@" in front of an oracle descriptor; the userinfo of an authority, one per
+ // host of it, since a url of Connector/J gives every host credentials of its own
+ // ("//u:p@h1:3306,u2:p2@h2:3306"); and the parameters behind their first separator,
+ // "?user=...&password=..." on postgresql, mysql and oracle, ";password=..." on sql server.
+ // What is left is looked over once more: the key-value host syntax of Connector/J puts a
+ // password inside the authority itself ("//address=(host=h)(user=u)(password=p)"), where
+ // neither of the first two shapes stands, so a "password=" of any case is blanked out wherever
+ // it is left standing.
+ //
+ // A password is free to hold either of the delimiters, so neither of them is looked for in the
+ // whole string. The credentials of an oracle url stand between the subprotocol and the first
+ // "@", which is the delimiter of its descriptor, so a "?" of one is part of the password
+ // rather than the start of the parameters. Everywhere else they stand inside the authority,
+ // between "//" and the path behind it, so a "?" of a password is inside them and an "@" of a
+ // parameter value ("?user=u@example.com") is not mistaken for the end of them: the host
+ // survives in the message either way.
+ //
+ // And a url none of this took apart is not logged past its subprotocol. An "@" left standing
+ // anywhere but where the credentials of an oracle url ended is one this did not recognize - a
+ // password holding a "/" inside an authority, a quoted one holding an "@" - and the host of a
+ // stall report is worth less than a password in the server log.
+ static String safeUrl(String connectionString) {
+ final ConnectDialect dialect = ConnectDialect.of(connectionString);
+ final String separators = dialect == null ? "?;" : String.valueOf(dialect.parameterSeparator);
+ final int scheme = connectionString.indexOf(':', "jdbc:".length()) + 1; // the end of "jdbc:<subprotocol>:"
+ if (scheme <= 0) {
+ return CREDENTIALS_HIDDEN;
+ }
+ final String stripped = stripCredentials(connectionString, scheme, separators);
+ final int parameters = indexOfAny(stripped, separators, scheme);
+ final String url = parameters < 0 ? stripped : stripped.substring(0, parameters);
+ final String redacted = SECRET_PARAMETER.matcher(url).replaceAll("$1=***");
+ // an "@" standing anywhere but where the credentials of an oracle url ended is one this
+ // did not recognize - a password holding a "/" inside an authority, a quoted one holding
+ // an "@" - and the host of a stall report is worth less than a password in the server log
+ if (redacted.lastIndexOf('@') > endOfRecognizedCredentials(connectionString, scheme)) {
+ return redacted.substring(0, scheme) + CREDENTIALS_HIDDEN;
+ }
+ return parameters < 0 ? redacted
+ : redacted + identifyingParameters(stripped.substring(parameters), dialect);
+ }
+
+ /**
+ * The parameters worth keeping in a message: the ones naming the database rather than whoever
+ * connects to it. Two backends of one host answer to the same url up to their parameters, and
+ * a stall report that cannot tell them apart is a stall report of neither. Everything else is
+ * dropped rather than looked at - a name this does not know is a name free to carry a secret.
+ */
+ private static String identifyingParameters(String parameters, ConnectDialect dialect) {
+ final char separator = dialect == null ? ';' : dialect.parameterSeparator;
+ final StringBuilder kept = new StringBuilder();
+ for (final String parameter : parameters.split("[?&;]")) {
+ final int equals = parameter.indexOf('=');
+ if (equals > 0
+ && IDENTIFYING_PARAMETERS.contains(parameter.substring(0, equals).toLowerCase(Locale.ROOT))) {
+ kept.append(kept.length() == 0 || separator != '?' ? separator : '&').append(parameter);
+ }
+ }
+ return kept.toString();
+ }
+
+ private static String stripCredentials(String url, int scheme, String separators) {
+ final int authority = startOfAuthority(url, scheme);
+ if (authority < 0) {
+ // no authority: the credentials of an oracle url stand between the subprotocol and the
+ // first "@", which is the delimiter of the descriptor behind it - a password holding
+ // an "@" of its own has to be quoted for the driver itself
+ final int at = url.indexOf('@', scheme);
+ return at < 0 ? url : url.substring(0, credentialsStart(url, scheme, at)) + url.substring(at);
+ }
+ final int end = endOfAuthority(url, authority, separators);
+ return url.substring(0, authority) + withoutUserinfo(url.substring(authority, end)) + url.substring(end);
+ }
+
+ /**
+ * Where the credentials of a url that names no authority start: behind the token naming the
+ * kind of driver, which stands in front of them ("jdbc:oracle:thin:user/pw@...") and is worth
+ * keeping - thin against oci is a first question of an oracle connect. The token is the one
+ * right behind the subprotocol rather than the last one in front of the "@", since a password
+ * is free to hold a ":" of its own.
+ */
+ private static int credentialsStart(String url, int scheme, int at) {
+ final int driverType = url.indexOf(':', scheme);
+ return driverType >= 0 && driverType < at ? driverType + 1 : scheme;
+ }
+
+ /**
+ * The last position a stripped url may still carry an "@" at: where the credentials of an
+ * oracle url ended, since the "@" is the delimiter of the descriptor behind them and stays.
+ * An authority keeps none of its own - every userinfo of it is taken off, delimiter included.
+ */
+ private static int endOfRecognizedCredentials(String url, int scheme) {
+ final int at = url.indexOf('@', scheme);
+ return startOfAuthority(url, scheme) < 0 && at > scheme ? credentialsStart(url, scheme, at) : scheme;
+ }
+
+ /**
+ * Where the hosts of a url of this shape start, or -1 for a url that names no authority. The
+ * subprotocol is free to name the kind of connection in front of it - "jdbc:mysql:replication://"
+ * - so the "//" is looked for rather than expected right behind the subprotocol. An "@" in
+ * front of it belongs to an oracle url ("jdbc:oracle:thin:user/pw@//host"), whose credentials
+ * stand where an authority has no place for them.
+ */
+ private static int startOfAuthority(String url, int scheme) {
+ final int slashes = url.indexOf("//", scheme);
+ if (slashes < 0 || url.lastIndexOf('@', slashes) >= scheme) {
+ return -1;
+ }
+ // ... and so does a "/" in front of them: it is what separates the credentials of an
+ // oracle url ("thin:user/pw@..."), so a password holding a "//" of its own would start an
+ // authority inside itself. The "@" ending the credentials stands behind that point, the
+ // userinfo taken off is a piece of the password rather than the whole of it, and the "@"
+ // the guard of safeUrl() looks for is gone with it - leaving the user name and the head of
+ // the password in the message of a stall.
+ final int slash = url.indexOf('/', scheme);
+ return slash >= 0 && slash < slashes ? -1 : slashes + 2;
+ }
+
+ /**
+ * Where the hosts of an authority end: at the path behind them - a password holds a "?" more
+ * readily than a "/" - or at the first parameter of a url that has no path.
+ */
+ private static int endOfAuthority(String url, int authority, String separators) {
+ final int path = url.indexOf('/', authority);
+ if (path >= 0) {
+ return path;
+ }
+ final int parameter = indexOfAny(url, separators, authority);
+ return parameter < 0 ? url.length() : parameter;
+ }
+
+ /** The hosts of an authority, each of them without the credentials a url may give it. */
+ private static String withoutUserinfo(String authority) {
+ final StringBuilder hosts = new StringBuilder();
+ final String[] split = authority.split(",", -1);
+ for (int i = 0; i < split.length; i++) {
+ if (i > 0) { // by the position rather than by what is in hand: a first host may be empty
+ hosts.append(',');
+ }
+ final int at = split[i].lastIndexOf('@');
+ hosts.append(at < 0 ? split[i] : split[i].substring(at + 1));
+ }
+ return hosts.toString();
+ }
+
+ private static int indexOfAny(String url, String separators, int from) {
+ int found = -1;
+ for (int i = 0; i < separators.length(); i++) {
+ final int at = url.indexOf(separators.charAt(i), from);
+ if (at >= 0 && (found < 0 || at < found)) {
+ found = at;
+ }
+ }
+ return found;
+ }
+
+ private static void closeQuietly(Connection con) {
+ try {
+ con.close();
+ } catch (SQLException e) {
+ // ignore: it is on its way out anyway
}
}
@@ -153,8 +1433,21 @@
@Override
public void close() throws SQLException {
- rollback();
- cached.get(connectionString).add(this);
+ try {
+ rollback();
+ } catch (SQLException e) {
+ // A connection that cannot be rolled back must not be handed to the next borrower -
+ // and must not be dropped on the floor either: nothing else holds it any more.
+ closeQuietly(parent);
+ throw e;
+ }
+ if (!poolable) {
+ closeQuietly(parent);
+ return;
+ }
+ // Returned to the end the next borrow takes it from, so that the pool keeps reusing its
+ // hottest connections rather than cycling through every one it ever opened.
+ cached.get(connectionString).addFirst(this);
}
@Override
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
index 599a0f7..a63eb14 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
@@ -42,6 +42,7 @@
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
import static org.opends.server.backends.pluggable.spi.StorageUtils.addErrorMessage;
import static org.opends.server.util.StaticUtils.stackTraceToSingleLineString;
@@ -73,8 +74,17 @@
/** Upper bound the doubled delay is capped at, in milliseconds. */
private static final double MAX_SLEEP_ON_RETRY_MS = 1000.0;
- /** Number of {@link Throwable#getCause()} hops walked when classifying a failure, also a guard against a cycle. */
- private static final int MAX_CAUSE_HOPS = 16;
+ /**
+ * Number of links walked when classifying a failure, also a guard against a chain long enough to matter. One
+ * number for three chains at once - the causes, the next exceptions and the suppressed exceptions are walked
+ * together and counted together - so it is set well above the depth a wrapped failure of this backend reaches:
+ * mssql-jdbc chains every error of one message it received through {@code setNextException}, and a budget spent
+ * on those would never reach the cause the wrapper carries.
+ */
+ private static final int MAX_CHAIN_LINKS = 64;
+
+ /** The budget of {@link #failureScope}, which walks to the end of the chains: see the comment above it. */
+ private static final int EVERY_LINK = Integer.MAX_VALUE;
/** SQL Server error number of the transaction picked as the deadlock victim: "Rerun the transaction". */
private static final int MSSQL_DEADLOCK_VICTIM = 1205;
@@ -103,6 +113,21 @@
private static final Set<String> NON_REPLAYABLE_ROLLBACK_STATES =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList("40002", "40003")));
+ /** SQLState class 08, connection exception: the connection is gone, whatever the statement asked for. */
+ private static final String CONNECTION_FAILURE_CLASS = "08";
+
+ /**
+ * The states outside class 08 that also say the connection is gone rather than the statement wrong. PostgreSQL
+ * announces the connection it is about to drop as 57P01 (admin_shutdown - a pg_terminate_backend of an idle
+ * connection reaper, or a shutdown of the server), 57P02 (crash_shutdown) or 57P03 (cannot_connect_now), and
+ * only the next use of that connection is reported as class 08. They are the states of the list HikariCP
+ * evicts a connection on that a driver of this backend reports: of the rest, JZ0C0 and JZ0C1 belong to a Sybase
+ * driver this backend is not used with, 01002 is a disconnect none of these four drivers reports, and 0A000 is
+ * the standard "feature not supported", which says nothing about the connection at all.
+ */
+ private static final Set<String> CONNECTION_FAILURE_STATES =
+ Collections.unmodifiableSet(new HashSet<>(Arrays.asList("57P01", "57P02", "57P03")));
+
private JDBCBackendCfg config;
public JDBCStorage(JDBCBackendCfg cfg, ServerContext serverContext) {
@@ -156,11 +181,23 @@
return CachedConnection.getConnection(config.getDBDirectory());
}
+ /**
+ * Borrows a connection the pool validates whatever the alive window of
+ * {@link CachedConnection#ALIVE_BYPASS_PROPERTY} says, for the borrows this class compensates a dropped
+ * connection on in no other way: {@link #open(AccessMode)}, {@link #removeStorageFiles()} and the importer
+ * issue their statements far from the borrow, and the open issues none at all, so a connection dropped inside
+ * the window would surface out of the rollback that releases it. One round trip on a path taken once per open,
+ * per import or per removal buys back exactly what master did on every borrow.
+ */
+ Connection getValidatedConnection() throws Exception {
+ return CachedConnection.getConnection(config.getDBDirectory(), false);
+ }
+
AccessMode accessMode=AccessMode.READ_ONLY;
@Override
public void open(AccessMode accessMode) throws Exception {
- try (final Connection con=getConnection()) {
+ try (final Connection con=getValidatedConnection()) {
this.accessMode = accessMode;
storageStatus = StorageStatus.working();
}
@@ -596,42 +633,25 @@
SESSION
}
- // What a failed stamp says about trying again. Both chains of the failure are walked: a driver
- // reports the vendor error of a rejected statement as the next exception of a generic one at
- // least as often as it reports it as the cause, and reading only one of the two would classify
- // a lock timeout as a rejection, which leaves the tree unstamped for the life of the backend
- // over a moment of contention.
+ // What a failed stamp says about trying again. Every chain of the failure is walked, by the walk
+ // every other classifier of this class uses: a driver reports the vendor error of a rejected
+ // statement as the next exception of a generic one at least as often as it reports it as the
+ // cause, the statement of a try-with-resources carries what its close() saw as a suppressed
+ // exception, and reading fewer of them than the others do would classify a connection that broke
+ // as a rejection - which leaves the tree unstamped for the life of the backend. Walked to its end
+ // rather than to MAX_CHAIN_LINKS: the seen set already terminates it, and the verdict weakens
+ // under truncation rather than simply going unnoticed - a SESSION past the budget would come back
+ // as TREE. The strongest verdict wins, so it is asked for in that order.
static FailureScope failureScope(Throwable failure, Dialect dialect) {
- FailureScope scope=FailureScope.TREE;
- final Deque<Throwable> pending=new ArrayDeque<>();
- final Set<Throwable> seen=Collections.newSetFromMap(new IdentityHashMap<Throwable,Boolean>());
- if (failure!=null) {
- pending.push(failure);
+ if (firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK,
+ e -> scopeOf(e, dialect)==FailureScope.SESSION)!=null) {
+ return FailureScope.SESSION;
}
- while (!pending.isEmpty()) {
- final Throwable e=pending.pop();
- if (!seen.add(e)) { // a driver that chains an exception back to itself must not loop this walk
- continue;
- }
- if (e.getCause()!=null) {
- pending.push(e.getCause());
- }
- if (!(e instanceof SQLException)) {
- continue;
- }
- final SQLException sqlException=(SQLException) e;
- if (sqlException.getNextException()!=null) {
- pending.push(sqlException.getNextException());
- }
- final FailureScope found=scopeOf(sqlException, dialect);
- if (found==FailureScope.SESSION) { // nothing further down either chain can weaken this one
- return FailureScope.SESSION;
- }
- if (found==FailureScope.MOMENT) {
- scope=FailureScope.MOMENT;
- }
+ if (firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK,
+ e -> scopeOf(e, dialect)==FailureScope.MOMENT)!=null) {
+ return FailureScope.MOMENT;
}
- return scope;
+ return FailureScope.TREE;
}
// What one exception of the chain says on its own.
@@ -794,7 +814,7 @@
}
final Set<TreeName> trees=listTrees();
if (!trees.isEmpty()) {
- try (final Connection con = getConnection()) {
+ try (final Connection con = getValidatedConnection()) {
try {
for (final TreeName treeName : trees) {
try (final PreparedStatement statement = con.prepareStatement("drop table " + getTableName(treeName))) {
@@ -834,11 +854,43 @@
* counters in instance fields that no attempt resets, so a replay reports twice the entry count of the backend.
* Both are reachable while the server is online, since an export holds no more than a shared backend lock.
* A conflict therefore fails the read here, exactly as it did before the retry of {@link #write} was added.
+ * <p>
+ * A connection the database dropped is not replayed either, for the same reason - but it is reported to the
+ * pool, which cannot notice one on its own: a borrow inside the alive window of
+ * {@link CachedConnection#ALIVE_BYPASS_PROPERTY} asks the database nothing, so the statement that broke is the
+ * only place the drop is ever seen.
*/
@Override
public <T> T read(ReadOperation<T> readOperation) throws Exception {
- try(final Connection con=getConnection()) {
- return readOperation.run(new ReadableTransactionImpl(con));
+ //borrowed outside the try: a connect the pool could not make says nothing about the connections it
+ //holds - mysql reports a server at its connection limit as 08004, which is class 08 like a connection
+ //that broke - and distrusting the pool over it would validate every borrow under the very load the
+ //window exists for, against a server already refusing connections
+ final Connection con=getConnection();
+ boolean dropped=false;
+ try (con) {
+ try {
+ return readOperation.run(new ReadableTransactionImpl(con));
+ } catch (Exception e) {
+ //asked while this read still owns the connection: once the release below has returned it to
+ //the pool, another borrow may hold it and the driver would be answering about that one
+ dropped=isConnectionFailure(e,con);
+ if (dropped) {
+ //told before the release rather than after it: a rollback that never reaches the server -
+ //which is what pgjdbc does with a transaction it left IDLE - leaves the connection poolable,
+ //so the release puts the dropped connection back at the head of the deque, and a borrow
+ //racing the distrust would be handed it unvalidated
+ distrustPool();
+ }
+ throw e;
+ }
+ } catch (Exception e) {
+ //also the release of the connection: its rollback is the one round trip a read that found
+ //nothing makes, so it can be the only place a drop is ever seen
+ if (!dropped && isConnectionFailure(e)) {
+ distrustPool();
+ }
+ throw e;
}
}
@@ -864,6 +916,13 @@
* Only the operation itself is replayed: a failure of {@link #getConnection()} or of the implicit
* {@link Connection#close()} - which returns the connection to the pool after a rollback - leaves the loop, so
* that a completed write is never replayed because releasing its connection failed.
+ * <p>
+ * A connection the database dropped is replayed as well, on a connection the next attempt borrows of its own.
+ * That is what makes the alive window of {@link CachedConnection#ALIVE_BYPASS_PROPERTY} safe to leave on: a
+ * connection handed out unvalidated and found dead costs an attempt rather than the operation, and a write of
+ * the replication replay - which records a failed operation as applied and advances the server state past it,
+ * see #889 - never sees it. Only while nothing of the attempt may have been committed yet, though: see
+ * {@link #replayReason(Throwable, String, boolean, boolean, boolean)}.
*/
@Override
public void write(WriteOperation writeOperation) throws Exception {
@@ -871,35 +930,93 @@
for (int attempt=1;;attempt++) {
Exception failure=null;
String driver=null;
- try (final Connection con=getConnection()) {
+ boolean committing=false;
+ boolean dropped=false;
+ boolean partlyCommitted=false;
+ //borrowed outside the try, for the reason read() borrows outside it: a connect the pool could not
+ //make is not a connection of this pool that broke, and it leaves the loop as it always did
+ final Connection con=getConnection();
+ try (con) {
driver=driverNameOf(con);
final WriteableTransactionTransactionImpl txn=new WriteableTransactionTransactionImpl(con);
try {
writeOperation.run(txn);
+ committing=true;
con.commit();
return;
} catch (Exception e) {
try {
con.rollback();
- } catch (SQLException ex) {}
+ } catch (SQLException ex) {
+ //joined to the failure rather than dropped: a rollback issued on a connection the
+ //database dropped is often the first place - and on a driver that reports a killed
+ //session as a plain vendor error, the only place - the drop is stated outright, and
+ //every classifier below reads the chains of this failure
+ e.addSuppressed(ex);
+ }
+ //asked while this attempt still owns the connection: the release below returns it to the
+ //pool, and the driver would then be answering about whichever borrow holds it next
+ dropped=isConnectionFailure(e,con);
+ if (dropped) {
+ //told before the release rather than after it, for the reason read() tells it there: a
+ //rollback that never reached the server leaves the connection poolable, so the release
+ //returns the dropped connection to the head of the deque, where a borrow racing this
+ //would be handed it unvalidated
+ distrustPool();
+ }
//rethrown, so that a failure of the implicit close() is suppressed into the failure being
//replayed rather than replacing it
failure=e;
throw e;
} finally { // the comment connection lives no longer than the trees it stamped, and no longer
// than the attempt that opened it: a replay stamps on a session of its own
- txn.stampSession.close();
+ partlyCommitted=txn.partlyCommitted;
+ try {
+ txn.stampSession.close();
+ } catch (RuntimeException e) {
+ //the stamp is a diagnostic aid and must not become the outcome of the write: an unchecked
+ //throw out of a driver's close() would otherwise replace the failure being unwound (JLS
+ //14.20.2) - the very one the replay is decided on and the only one that says what went
+ //wrong - or turn a transaction that has just committed into a failure of its own
+ if (failure!=null) {
+ failure.addSuppressed(e);
+ } else {
+ logger.trace(LocalizableMessage.raw("jdbc: unable to close the comment connection: %s",
+ stackTraceToSingleLineString(e)));
+ }
+ }
}
} catch (Exception e) {
- //anything the operation did not throw comes from getConnection() or from the implicit close(),
- //which returns the connection to the pool: neither belongs to the replayed region
+ //anything the operation did not throw comes from around it - the name of the driver, the
+ //transaction, or the implicit close() that returns the connection to the pool: none of them
+ //belongs to the replayed region
if (e!=failure) {
+ //a drop reported by the release of the connection still has to reach the pool, which has no
+ //other way of hearing of it. Only the chains of the failure can be asked for it now: the
+ //connection has been released, and whether it is closed is no longer this attempt's answer
+ if (isConnectionFailure(e)) {
+ distrustPool();
+ }
throw e;
}
}
+ //a drop the release of the connection reported still has to reach the pool, which has no other way
+ //of hearing of it. It is suppressed into the failure being unwound (JLS 14.20.3.1) rather than
+ //replacing it, which is what leaves e==failure and skips the branch above - and it is the very
+ //evidence replayReason() replays the attempt on, so the pool must not be told less than the loop
+ //acts on. The drop of the operation itself was reported before the release, above
+ if (!dropped && isConnectionFailure(failure)) {
+ distrustPool();
+ }
+ //Two questions, asked apart: what the failure is - which replayReason() answers, and which is the
+ //only place that reads committing, partlyCommitted and dropped - and whether another attempt is
+ //still allowed, which is the attempt count and the window of #903. Neither subsumes the other: a
+ //dropped connection is worth replaying and carries no conflict class, while a conflict past both
+ //bounds is not replayed however plainly it is one
+ final String reason=replayReason(failure,driver,committing,partlyCommitted,dropped);
//nanoTime()-startedAt is the overflow safe form of the elapsed time
final long elapsedNanos=nanoTime()-startedAt;
- if (!replayable(attempt, elapsedNanos, failure, driver)) {
+ if (reason==null || !replayableWithin(attempt, elapsedNanos, failure, driver)) {
throw failure;
}
//logged rather than silently absorbed, so that a deployment retrying most of its writes stays observable;
@@ -910,13 +1027,13 @@
//engines report a deadlock in a few of them and whole seconds would read "0" for most of a burst; and
//the one line that replays past its own window says so, rather than reading as a bound not honoured
logger.warn(LocalizableMessage.raw(
- "jdbc: replaying the transaction after a %s conflict, attempt %d of %d, %d ms elapsed of the %d ms window%s: %s",
- conflictOf(failure, driver), attempt, MAX_RETRIES, TimeUnit.NANOSECONDS.toMillis(elapsedNanos),
+ "jdbc: replaying the transaction after %s, attempt %d of %d, %d ms elapsed of the %d ms window%s: %s",
+ reason, attempt, MAX_RETRIES, TimeUnit.NANOSECONDS.toMillis(elapsedNanos),
TimeUnit.NANOSECONDS.toMillis(RETRY_WINDOW_NANOS),
elapsedNanos>=RETRY_WINDOW_NANOS ? " (the first replay, granted past it)" : "",
- conflictSummary(failure)));
+ conflictSummary(failure, driver)));
if (logger.isTraceEnabled()) {
- logger.trace("jdbc: the conflict being replayed was %s", stackTraceToSingleLineString(failure));
+ logger.trace("jdbc: the failure being replayed was %s", stackTraceToSingleLineString(failure));
}
try {
//randomized to spread the retries of the transactions that collided, growing to outlast contention
@@ -940,6 +1057,168 @@
return System.nanoTime();
}
+ /**
+ * Why the operation of a {@link #write} is worth replaying, as the noun phrase the message reporting the replay
+ * names - or null for a failure this loop must not repeat.
+ * <p>
+ * A transaction conflict is replayable whichever phase reported it: the engine rolled the transaction back
+ * before it answered. It is read from the failure of the operation only, never from the release of the
+ * connection - see {@link #isRetryableConflict} - since the release runs after the outcome was decided and
+ * cannot make that claim for it. A connection the database dropped is replayable only while the transaction
+ * had not been committed yet. A drop reported by {@code commit()} leaves the outcome unknown - the server may
+ * have committed and died before the answer reached us - and replaying a write that in fact committed applies
+ * it twice, which is the very reason 40003 is one of {@link #NON_REPLAYABLE_ROLLBACK_STATES}.
+ * <p>
+ * Nothing is replayable once the attempt has committed part of its own work, whatever the failure says. The DDL
+ * of {@link WriteableTransactionTransactionImpl#openTree} and {@link WriteableTransactionTransactionImpl#deleteTree}
+ * commits inside {@link WriteOperation#run}, and mysql and oracle commit before a DDL statement whether asked
+ * to or not, so the attempt no longer rolls back as a whole - and {@link WriteOperation} is only idempotent in
+ * the database. {@code RootContainer.open} opens and registers every entry container of every base DN in one
+ * write: replayed after the trees of the first base DN were created and committed, it registers that base DN a
+ * second time and fails with ERR_ENTRY_CONTAINER_ALREADY_REGISTERED, which masks the failure that caused the
+ * replay and leaves the indexes of the previous attempt behind with their configuration listeners.
+ *
+ * @param committing whether the failure was reported by {@code commit()}, which leaves the outcome unknown
+ * @param partlyCommitted whether the attempt committed part of its work before it failed
+ * @param connectionClosed whether the driver closed the connection under the failure - evidence no SQLState
+ * carries on mssql-jdbc, which reports a killed session as S0001 and closes the connection behind it
+ */
+ static String replayReason(Throwable failure, String driver, boolean committing, boolean partlyCommitted,
+ boolean connectionClosed) {
+ if (partlyCommitted) {
+ return null;
+ }
+ if (isRetryableConflict(failure, driver)) {
+ return "a conflict";
+ }
+ if (!committing && (connectionClosed || isConnectionFailure(failure))) {
+ return "a connection the database dropped";
+ }
+ return null;
+ }
+
+ /**
+ * Whether a failure says the connection is gone rather than the statement rejected, asked of the failure and of
+ * the connection it was raised on. A driver is not required to say so in a SQLState: mssql-jdbc reports a
+ * session killed by {@code KILL}, by the resource governor or by an availability group transition as error 596,
+ * 3980, 10054, 18456 or 4060, and {@code generateStateCode} maps none of them - with xopenStates off, which is
+ * its default, every one of them comes out as {@code "S"+errorState}, measured as S0001. What the driver does
+ * do is close the connection for any error of severity 20 and above, before it throws.
+ * <p>
+ * Asked only while the operation that failed still owns the connection: a released one is back in the pool and
+ * may already have been handed to another borrow, whose state it would then be answering about.
+ */
+ static boolean isConnectionFailure(Throwable failure, Connection con) {
+ return isConnectionFailure(failure) || isClosed(con);
+ }
+
+ /** Whether the driver reports the connection as closed; one that cannot answer is taken as closed. */
+ private static boolean isClosed(Connection con) {
+ try {
+ return con.isClosed();
+ } catch (SQLException e) {
+ return true;
+ }
+ }
+
+ /**
+ * Whether a failure says the connection is gone rather than the statement rejected: the database dropped it,
+ * restarted, failed over, or the network did.
+ * <p>
+ * Both chains of the failure are walked, for the reason {@link #failureScope} walks both: a driver reports the
+ * error that says what happened as the next exception of a generic one at least as often as it reports it as
+ * the cause, and mssql-jdbc chains every error of a message it received that way. The suppressed exceptions are
+ * walked with them, since the rollback and the release of a connection report a drop there - a write whose
+ * operation failed for its own reasons carries the drop of its {@code close()} as a suppressed exception (JLS
+ * 14.20.3.1) rather than as a cause. The walk starts at the failure this class was handed because it reaches it
+ * wrapped in a {@link StorageRuntimeException}, and a caller such as {@code EntryContainer.addEntry} may wrap
+ * it once more.
+ */
+ static boolean isConnectionFailure(Throwable failure) {
+ return firstLinkMatching(failure, WITH_THE_RELEASE, JDBCStorage::saysTheConnectionIsGone)!=null;
+ }
+
+ /**
+ * Whether {@link #firstLinkMatching} reads the suppressed exceptions along with the causes and the next
+ * exceptions. They are where the release of the connection reports what it saw - a rollback that failed as the
+ * attempt was unwound is suppressed into the failure being unwound (JLS 14.20.3.1) - so a question about the
+ * connection is asked of them, and a question about what the engine did with the transaction is not: the
+ * release runs after the outcome was decided, and cannot speak for it.
+ */
+ private static final boolean WITH_THE_RELEASE=true;
+ private static final boolean WITHOUT_THE_RELEASE=false;
+
+ /**
+ * The first {@link SQLException} of the chains of a failure that answers the given question, or null where none
+ * does. Every classifier of this class walks the failure this way, so that none of them reads a chain the others
+ * act on: what makes a write replayable must also be what the pool is told about and what the replay logs.
+ */
+ private static SQLException firstLinkMatching(Throwable failure, boolean withTheRelease,
+ Predicate<SQLException> matches) {
+ return firstLinkMatching(failure, withTheRelease, MAX_CHAIN_LINKS, matches);
+ }
+
+ /** The walk above, with the number of links it is allowed to look at. */
+ private static SQLException firstLinkMatching(Throwable failure, boolean withTheRelease, int links,
+ Predicate<SQLException> matches) {
+ final Deque<Throwable> pending=new ArrayDeque<>();
+ final Set<Throwable> seen=Collections.newSetFromMap(new IdentityHashMap<Throwable,Boolean>());
+ if (failure!=null) {
+ pending.push(failure);
+ }
+ while (!pending.isEmpty() && seen.size()<links) {
+ final Throwable e=pending.pop();
+ if (!seen.add(e)) { // a driver that chains an exception back to itself must not loop this walk
+ continue;
+ }
+ if (e.getCause()!=null) {
+ pending.push(e.getCause());
+ }
+ if (withTheRelease) {
+ for (final Throwable suppressed : e.getSuppressed()) {
+ pending.push(suppressed);
+ }
+ }
+ if (!(e instanceof SQLException)) {
+ continue;
+ }
+ final SQLException sqlException=(SQLException) e;
+ if (sqlException.getNextException()!=null) {
+ pending.push(sqlException.getNextException());
+ }
+ if (matches.test(sqlException)) {
+ return sqlException;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * What one exception of the chain says on its own. The types are asked before the SQLState, the way
+ * {@link #scopeOf} asks them: they are what the JDBC contract gives a driver to say the connection is gone, and
+ * a driver that raises one of them has said so whatever state it filled in. Oracle reports ORA-03113, ORA-00028
+ * and ORA-01089 as {@link SQLRecoverableException} and happens to map them to 08006 as well; the type is what
+ * makes that robust rather than lucky.
+ */
+ private static boolean saysTheConnectionIsGone(SQLException e) {
+ if (e instanceof SQLRecoverableException || e instanceof SQLNonTransientConnectionException
+ || e instanceof SQLTransientConnectionException) {
+ return true;
+ }
+ final String state=String.valueOf(e.getSQLState());
+ return state.startsWith(CONNECTION_FAILURE_CLASS) || CONNECTION_FAILURE_STATES.contains(state);
+ }
+
+ /**
+ * Tells the pool of this backend that the database dropped a connection, so that the ones it still holds from
+ * before the drop are validated on their next borrow instead of being trusted for the rest of the alive window.
+ * A dropped connection is rarely alone: a restart, a failover or a network that went away takes every
+ * connection established before it, and the pool has no other way of hearing about any of them.
+ */
+ private void distrustPool() {
+ CachedConnection.distrustPool(config.getDBDirectory());
+ }
+
/** Returns the randomized delay before the given attempt is replayed, doubling with each attempt up to a cap. */
static long retryDelayMillis(int attempt) {
final double bound=Math.min(MAX_SLEEP_ON_RETRY_MS, BASE_SLEEP_ON_RETRY_MS * (1 << Math.min(attempt-1, 5)));
@@ -964,12 +1243,16 @@
* Returns the class of the conflict the given failure carries, or {@link Conflict#NONE} if it carries none,
* which is what decides whether replaying the operation can resolve it.
* <p>
- * The conflict is looked up along the whole cause chain because it reaches this class wrapped: a deadlock in
- * {@code put} arrives as {@code StorageRuntimeException(SQLException)}, and a caller such as
- * {@code EntryContainer.addEntry} may wrap it once more. The chain is walked to its end rather than stopped at
- * the first conflict found, so that the most specific class in it wins: a wrapper that carries a class 40 state
- * of its own but no vendor number would otherwise downgrade the {@link Conflict#AFTER_LOCK_WAIT} of the
- * {@link SQLException} it wraps, and hand a wait the engine already bounded a replay it does not need.
+ * The conflict is looked up along every chain of the failure, for the reason {@link #isConnectionFailure} walks
+ * them all: it reaches this class wrapped - a deadlock in {@code put} arrives as
+ * {@code StorageRuntimeException(SQLException)}, and a caller such as {@code EntryContainer.addEntry} may wrap it
+ * once more - and a driver reports the error that says what happened as the next exception of a generic one at
+ * least as often as it reports it as the cause. The suppressed links of the release are left out of it, for the
+ * reason {@link #replayReason} gives: the release runs after the outcome was decided.
+ * <p>
+ * The most specific class in those chains wins rather than the first one found: a wrapper that carries a class
+ * 40 state of its own but no vendor number would otherwise downgrade the {@link Conflict#AFTER_LOCK_WAIT} of
+ * the {@link SQLException} it wraps, and hand a wait the engine already bounded a replay it does not need.
* <p>
* The standard class 40 states carry the conflict of most engines - 40P01 for PostgreSQL, 40001 for SQL Server
* and for MySQL, whose driver replaces the server side HY000 of a deadlock and of a lock wait timeout with
@@ -984,17 +1267,13 @@
* excluded from that match.
*/
static Conflict conflictOf(Throwable t, String driver) {
- Conflict found=Conflict.NONE;
- for (int hop=0; t!=null && hop<MAX_CAUSE_HOPS; t=t.getCause(), hop++) {
- if (t instanceof SQLException) {
- final Conflict conflict=classOf((SQLException) t, driver);
- if (conflict==Conflict.AFTER_LOCK_WAIT) { // the most specific there is: no later hop refines it
- return conflict;
- }
- found=conflict!=Conflict.NONE ? conflict : found;
- }
+ // asked for the most specific class first, which is what keeps a bare class 40 state elsewhere in the
+ // chains from downgrading the numbered failure that names a wait the engine had already bounded
+ if (firstLinkMatching(t, WITHOUT_THE_RELEASE, e -> classOf(e, driver)==Conflict.AFTER_LOCK_WAIT)!=null) {
+ return Conflict.AFTER_LOCK_WAIT;
}
- return found;
+ return firstLinkMatching(t, WITHOUT_THE_RELEASE, e -> isConflict(e, driver))!=null
+ ? Conflict.PROMPT : Conflict.NONE;
}
/**
@@ -1024,18 +1303,40 @@
* transaction connection, is what would let the window govern both classes and retire this grant.
*/
static boolean replayable(int attempt, long elapsedNanos, Throwable failure, String driver) {
- final Conflict conflict=conflictOf(failure, driver);
- if (conflict==Conflict.NONE || attempt>=MAX_RETRIES) {
+ return conflictOf(failure, driver)!=Conflict.NONE
+ && replayableWithin(attempt, elapsedNanos, failure, driver);
+ }
+
+ /**
+ * The bounds half of {@link #replayable}, asked of a failure {@link #replayReason} has already found worth
+ * replaying. Split from the class half because the two answer different questions and not every replayable
+ * failure carries a conflict class: a connection the database dropped is replayed on the evidence of the drop,
+ * and would be refused by a bound that first insisted on a class 40 state.
+ */
+ static boolean replayableWithin(int attempt, long elapsedNanos, Throwable failure, String driver) {
+ if (attempt>=MAX_RETRIES) {
return false;
}
//the engine asked for the transaction to be rerun after a wait nothing here bounds: no clock denies that
//first rerun, since the window it would be measured against was spent by the wait rather than by a replay
- if (attempt==1 && conflict==Conflict.PROMPT) {
+ if (attempt==1 && conflictOf(failure, driver)==Conflict.PROMPT) {
return true;
}
return elapsedNanos<RETRY_WINDOW_NANOS;
}
+ /**
+ * Whether the failure carries a transaction conflict, which is {@link #conflictOf} asked as a yes or no. Read
+ * without the suppressed exceptions, unlike {@link #isConnectionFailure}: a conflict is replayed whichever
+ * phase reported it, on the strength of the engine having rolled the transaction back before it answered - and
+ * the release of the connection runs after the outcome was decided and cannot make that claim. A class 40
+ * raised there would otherwise replay a transaction commit() left in doubt, which is what the committing
+ * guard of replayReason() exists to prevent.
+ */
+ static boolean isRetryableConflict(Throwable t, String driver) {
+ return conflictOf(t, driver)!=Conflict.NONE;
+ }
+
private static boolean isConflict(SQLException e, String driver) {
final String state=String.valueOf(e.getSQLState());
if (state.startsWith("40") && !NON_REPLAYABLE_ROLLBACK_STATES.contains(state)) {
@@ -1051,18 +1352,30 @@
}
/**
- * Returns the SQLState and vendor error number of the first {@link SQLException} of the given cause chain, which
- * is what identifies a conflict, so that a replay can be logged without a stack trace on every attempt.
+ * Returns the SQLState and vendor error number of the exception a replay was decided on, so that a replay can be
+ * logged without a stack trace on every attempt. That line is the only record a replay leaves, so it names the
+ * link the decision was taken on rather than the first {@link SQLException} of the failure: a write whose
+ * operation failed for its own reasons and whose release then reported a drop is replayed on the class 08
+ * suppressed into it, and naming the state of the rejected statement instead would describe a replay that did
+ * not happen. Falls back to the first SQLException of the failure, and to the failure itself where it carries
+ * none.
*/
- static String conflictSummary(Throwable failure) {
- Throwable t=failure;
- for (int hop=0; t!=null && hop<MAX_CAUSE_HOPS; t=t.getCause(), hop++) {
- if (t instanceof SQLException) {
- final SQLException e=(SQLException) t;
- return "SQLState "+e.getSQLState()+", error "+e.getErrorCode()+": "+e.getMessage();
- }
+ static String conflictSummary(Throwable failure, String driver) {
+ // asked in the order replayReason() asks it, and of the same chains, so that the line names the link the
+ // decision was taken on rather than one that merely resembles it
+ SQLException named=firstLinkMatching(failure, WITHOUT_THE_RELEASE, e -> isConflict(e, driver));
+ if (named==null) {
+ named=firstLinkMatching(failure, WITH_THE_RELEASE, JDBCStorage::saysTheConnectionIsGone);
}
- return String.valueOf(failure);
+ if (named==null) {
+ // without the release, so that the line names the statement that failed rather than the rollback
+ // behind it: this is the fallback of a replay decided on isClosed(con) alone, where neither chain
+ // carries a verdict, and the walk reaches the suppressed exceptions before the cause
+ named=firstLinkMatching(failure, WITHOUT_THE_RELEASE, e -> true);
+ }
+ return named==null
+ ? String.valueOf(failure)
+ : "SQLState "+named.getSQLState()+", error "+named.getErrorCode()+": "+named.getMessage();
}
static final byte[] NULL=new byte[]{(byte)0};
@@ -1143,6 +1456,16 @@
}
}
}
+ /**
+ * A transaction able to write, unless the storage was opened read-only: then it may open an existing tree and
+ * read it, and every mutating operation throws {@link ReadOnlyStorageException} instead.
+ * <p>
+ * The mode is checked per operation rather than refused here, because {@code RootContainer.open(AccessMode)}
+ * asks for a write transaction even in read-only mode - that is where it opens the compressed schema and the
+ * entry containers - so refusing to hand one out failed the offline {@code export-ldif}, {@code verify-index}
+ * and {@code backendstat} before they read anything (#874). Both other storages of this server already have
+ * this shape: {@code PDBStorage.ReadOnlyStorageImpl} and {@code CASStorage.TransactionImpl.checkReadOnly()}.
+ */
private final class WriteableTransactionTransactionImpl extends ReadableTransactionImpl implements WriteableTransaction {
// Shared by every table this transaction stamps: opening a backend opens all its trees,
@@ -1150,12 +1473,61 @@
// write() (and by ImporterImpl.close()) when the transaction is done with.
final StampSession stampSession=new StampSession();
+ /**
+ * Whether this transaction has committed part of its own work, which takes the attempt out of the
+ * replay of {@link JDBCStorage#write}: what it did no longer rolls back as a whole, and a
+ * {@link WriteOperation} is only idempotent in the database.
+ * <p>
+ * Raised by {@link #commitStatement} alone, which is what every statement of this transaction that
+ * commits goes through - never once for a method that may issue one: a catalog read deciding that the
+ * statement is not needed commits nothing, and a transaction the engine rolled back whole is still worth
+ * replaying. Which side of the statement the flag goes up on is the engine's answer, see there.
+ */
+ boolean partlyCommitted;
+
public WriteableTransactionTransactionImpl(Connection con) {
super(con);
- if (!accessMode.isWriteable()) {
+ //captured once rather than read per operation: the access mode of the storage is mutable state -
+ //ImporterImpl reopens the storage READ_WRITE under its caller - and a transaction has to keep the mode
+ //it was created with. It also drives isReadOnly, so that a cursor this transaction opens refuses
+ //delete() as well.
+ isReadOnly = !accessMode.isWriteable();
+ }
+
+ void checkReadOnly() {
+ if (isReadOnly) {
throw new ReadOnlyStorageException();
}
- isReadOnly = false;
+ }
+
+ /**
+ * Issues a statement that ends in a commit, raising {@link #partlyCommitted} at the moment the attempt
+ * stops rolling back as a whole.
+ * <p>
+ * mysql and oracle commit before a DDL statement whether asked to or not, so there the work behind it is
+ * committed by the statement itself and the flag has to be up before it is issued: the statement that
+ * fails has committed everything before it just as surely as the one that succeeds. postgresql and sql
+ * server run DDL inside the transaction, and a DML statement commits of its own accord nowhere - one that
+ * fails there has committed nothing, {@link JDBCStorage#write} rolls the attempt back whole, and a flag
+ * raised in front of it would take a conflict the engine itself undid out of the replay. On those the
+ * flag goes up in front of the commit instead, which is the call that leaves the outcome of the
+ * transaction unknown when it fails.
+ *
+ * @param ddl whether the statement is a DDL one, which two of the four engines commit before
+ */
+ private void commitStatement(String sql, boolean ddl) throws SQLException {
+ partlyCommitted|=ddl && commitsBeforeDdl();
+ try (final PreparedStatement statement=con.prepareStatement(sql)) {
+ execute(statement);
+ partlyCommitted=true; // a commit that fails leaves the outcome unknown, which is no more replayable
+ con.commit();
+ }
+ }
+
+ /** Whether this engine commits the transaction before a DDL statement whether asked to or not. */
+ private boolean commitsBeforeDdl() {
+ final String driverName=driverNameOf(con);
+ return driverName.contains("mysql") || driverName.contains("oracle");
}
boolean isExistsTable(TreeName treeName) {
@@ -1196,10 +1568,17 @@
@Override
public void openTree(TreeName treeName, boolean createOnDemand) {
if (createOnDemand) {
+ checkReadOnly();
+ // Every statement below is a DDL that commits, and each raises partlyCommitted through
+ // commitStatement() rather than once for the method: every one of them is guarded by a
+ // catalog read, so on an existing backend this method issues nothing at all. Raising the
+ // flag for a catalog read that commits nothing would make the whole attempt unreplayable -
+ // the conflict replay of #867 as much as the drop replay, since replayReason() reads the
+ // flag before it asks anything else - and RootContainer.open() opens every tree of every
+ // base DN in a single write, whose first act is one of these.
if (!isExistsTable(treeName)) {
- try (final PreparedStatement statement=con.prepareStatement("create table "+getTableName(treeName)+" ("+getTableDialect()+")")){
- execute(statement);
- con.commit();
+ try {
+ commitStatement("create table "+getTableName(treeName)+" ("+getTableDialect()+")", true);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1208,19 +1587,21 @@
final String driverName=driverNameOf(con);
final String tableName=getTableName(treeName);
if (driverName.contains("postgres")) {
- try (final PreparedStatement statement=con.prepareStatement("create index if not exists k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)")){
- execute(statement);
- con.commit();
+ try {
+ // asked although postgresql has "create index if not exists": that statement commits
+ // whether it creates anything or not, and this is the engine of every default
+ // deployment - unguarded, it would take every write that opens a tree out of the
+ // conflict replay, RootContainer.open() and its ~25 trees per suffix included
+ if (!isExistsIndex(tableName,"k_"+tableName.substring("opendj_".length()))) {
+ commitStatement("create index if not exists k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)", true);
+ }
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
}else if (driverName.contains("mysql")) {
try {
if (!isExistsIndex(tableName,"k_"+tableName.substring("opendj_".length()))) { // mysql has no "create index if not exists"
- try (final PreparedStatement statement=con.prepareStatement("create index k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)")){
- execute(statement);
- con.commit();
- }
+ commitStatement("create index k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)", true);
}
}catch (SQLException e) {
throw new StorageRuntimeException(e);
@@ -1229,10 +1610,7 @@
try {
// oracle has no "create index if not exists"; unquoted identifiers are stored in uppercase
if (!isExistsIndex(tableName.toUpperCase(),"k_"+tableName.substring("opendj_".length()))) {
- try (final PreparedStatement statement=con.prepareStatement("create index k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)")){
- execute(statement);
- con.commit();
- }
+ commitStatement("create index k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)", true);
}
}catch (SQLException e) {
throw new StorageRuntimeException(e);
@@ -1258,9 +1636,9 @@
}
public void clearTree(TreeName treeName) {
- try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName))){
- execute(statement);
- con.commit();
+ checkReadOnly();
+ try { // the commit takes the attempt out of the replay: it commits the delete, and everything before it
+ commitStatement("delete from "+getTableName(treeName), false);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1268,10 +1646,10 @@
@Override
public void deleteTree(TreeName treeName) {
+ checkReadOnly();
if (isExistsTable(treeName)) {
- try (final PreparedStatement statement = con.prepareStatement("drop table " + getTableName(treeName))) {
- execute(statement);
- con.commit();
+ try {
+ commitStatement("drop table " + getTableName(treeName), true);
} catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1283,6 +1661,7 @@
@Override
public void put(TreeName treeName, ByteSequence key, ByteSequence value) {
+ checkReadOnly();
try {
upsert(treeName, key, value);
} catch (SQLException e) {
@@ -1350,6 +1729,9 @@
@Override
public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f) {
+ //checked before the read, so that a read-only transaction reports the mode rather than the value it
+ //computed being equal to the stored one
+ checkReadOnly();
final ByteString oldValue=read(treeName,key);
final ByteSequence newValue=f.computeNewValue(oldValue);
if (Objects.equals(newValue, oldValue))
@@ -1366,6 +1748,7 @@
@Override
public boolean delete(TreeName treeName, ByteSequence key) {
+ checkReadOnly();
try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName)+" where h="+hashParam(con)+" and k=?")){
statement.setString(1,key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2,real2db(key.toByteArray()));
@@ -1609,7 +1992,7 @@
}
}
try {
- con = getConnection();
+ con = getValidatedConnection();
}catch (Exception e){
throw new StorageRuntimeException(e);
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
index 757aa99..ca84641 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
@@ -105,6 +105,7 @@
/** The storage associated with this index. */
private final Storage storage;
private final State state;
+ private final EntryContainer entryContainer;
/**
* A flag to indicate if this vlvIndex should be trusted to be consistent with the entries tree.
@@ -131,15 +132,8 @@
}
this.state = state;
+ this.entryContainer = entryContainer;
this.trusted = state.getIndexFlags(txn, getName()).contains(IndexFlag.TRUSTED);
- if (!trusted && entryContainer.getHighestEntryID(txn).longValue() == 0)
- {
- /*
- * If there are no entries in the entry container then there is no reason why this vlvIndex
- * can't be upgraded to trusted.
- */
- setTrusted(txn, true);
- }
this.config.addChangeListener(this);
}
@@ -163,6 +157,19 @@
void afterOpen(final WriteableTransaction txn, boolean createOnDemand) throws StorageRuntimeException
{
counter.open(txn, createOnDemand);
+ if (createOnDemand && !trusted && entryContainer.isEmpty(txn))
+ {
+ /*
+ * If there are no entries in the entry container then there is no reason why this vlvIndex
+ * can't be upgraded to trusted.
+ *
+ * Guarded by createOnDemand - which is accessMode.isWriteable() - and done here rather than in the
+ * constructor, as DefaultIndex.afterOpen() does: the transaction a read-only container opens is not
+ * allowed to write, so upgrading an untrusted index of an empty backend used to fail the offline tools
+ * on it instead of leaving the flag alone (#874).
+ */
+ setTrusted(txn, true);
+ }
}
@Override
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
new file mode 100644
index 0000000..c98f280
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java
@@ -0,0 +1,1870 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions Copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package org.opends.server.backends.jdbc;
+
+import org.opends.server.DirectoryServerTestCase;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.Field;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.DriverPropertyInfo;
+import java.sql.SQLException;
+import java.sql.SQLTimeoutException;
+import java.util.ArrayDeque;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.IdentityHashMap;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Logger;
+
+import org.mockito.InOrder;
+
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyInt;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNotSame;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertSame;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+/**
+ * The pool every operation of the JDBC backend borrows from must bound both of its phases and
+ * report a connect it cannot make, rather than retrying it out of sight of the caller (#872).
+ * Needs no database: the dialects are exercised against a socket that never answers and against a
+ * driver of this test, so a regression fails the build wherever it runs.
+ */
+@SuppressWarnings("javadoc")
+@Test(groups = { "precommit", "jdbc" }, sequential = true)
+public class CachedConnectionTestCase extends DirectoryServerTestCase {
+
+ /** A connect attempt of a bounded dialect must give up in about this long, plus room for a slow machine. */
+ private static final long BOUND_SECONDS = 2;
+ /**
+ * Room for a slow machine on top of a bound, and no more than that. A minute of it turned
+ * every assertion below into "it does not run forever": a connect that has to be reported at
+ * once and a borrow that has to give up at its two second deadline both passed at 59 s.
+ */
+ private static final long BOUND_MARGIN_MS = 10000;
+
+ private final StubDriver stub = new StubDriver();
+
+ /** The window as this JVM was started with it, put back after every test that varies it. */
+ private static final long CONFIGURED_ALIVE_BYPASS_NANOS = CachedConnection.aliveBypassNanos;
+
+ @BeforeClass
+ public void registerStubDriver() throws Exception {
+ DriverManager.registerDriver(stub);
+ }
+
+ @AfterClass
+ public void deregisterStubDriver() throws Exception {
+ DriverManager.deregisterDriver(stub);
+ }
+
+ /**
+ * Most of the tests below seed the pool by hand, and a connection built a moment ago is inside
+ * the alive window - they are about what the validation of a borrow does, so the window is off
+ * unless the test at hand is one of the window's own.
+ */
+ @BeforeMethod
+ public void validateEveryBorrow() {
+ CachedConnection.aliveBypassNanos = 0;
+ }
+
+ @AfterMethod
+ public void clearProperties() {
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.TTL_PROPERTY);
+ System.clearProperty(CachedConnection.ALIVE_BYPASS_PROPERTY);
+ // what has been reported once is remembered for the life of the jvm: left standing, the key
+ // of one test is what the next one finds when it asserts that it reported something itself
+ CachedConnection.warnedOnce.clear();
+ CachedConnection.aliveBypassNanos = CONFIGURED_ALIVE_BYPASS_NANOS;
+ }
+
+ /**
+ * A driver that is not on the classpath - the JDBC backend needs one dropped into
+ * lib/extensions by hand - is a configuration error the caller has to see. Retried, it is
+ * indistinguishable from a database that hangs.
+ */
+ @Test(timeOut = 120000)
+ public void testMissingDriverIsReportedAtOnce() throws Exception {
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection("jdbc:nosuchengine://127.0.0.1:5432/opendj");
+ fail("a connection string no registered driver accepts must be reported");
+ } catch (SQLException expected) {
+ assertTrue(expected.getMessage().contains("No suitable driver"), expected.getMessage());
+ }
+ assertElapsedWithinBound(startedAt, 0);
+ }
+
+ /**
+ * ... and the report of it carries no password. This is the path the credentials leave by: the
+ * jdk itself builds "No suitable driver found for " + url, a missing driver jar is the ordinary
+ * oracle misconfiguration, and what this class throws reaches the server error log in full -
+ * JDBCStorage.open() hands it to RootContainer, which makes the message of the cause the
+ * message of what it throws, and BackendConfigManager logs that at ERROR and answers a config
+ * change with it. Every link of the chain is asserted, not only the message on top: everything
+ * that prints a failure prints its causes along with it.
+ */
+ @Test(timeOut = 120000)
+ public void testAReportedConnectCarriesNoCredentials() throws Exception {
+ final String url = "jdbc:nosuchengine://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
+ try {
+ CachedConnection.getConnection(url);
+ fail("a connection string no registered driver accepts must be reported");
+ } catch (SQLException expected) {
+ assertNoCredentials(expected);
+ assertTrue(expected.getMessage().contains("No suitable driver"),
+ "the failure has to stay recognizable: " + expected.getMessage());
+ assertTrue(expected.getMessage().contains("127.0.0.1:5432"),
+ "the host is what a report of a connect is read for: " + expected.getMessage());
+ }
+ }
+
+ /**
+ * A url this class knows no timeout properties for is reported once. The properties bounding a
+ * connect are the ones of a driver, so a driver outside the four - an admin-added mariadb, an
+ * h2 - leaves every attempt unbounded, and the deadline of the borrow cannot reach into a
+ * connect already under way: the driver is the only thing holding the socket. Silently, that
+ * is #872 again, for a backend nobody thinks of as unbounded.
+ */
+ @Test(timeOut = 120000)
+ public void testAUrlThisBackendCannotBoundIsReportedOnce() throws Exception {
+ final String url = "jdbc:nosuchengine-unbounded://127.0.0.1:5432/opendj";
+ final String key = CachedConnection.safeUrl(url) + "|unknown-dialect";
+ // the set is the gate warnOnce() logs behind, so it has to start empty for this to be a
+ // test of what this borrow reported rather than of what some earlier one left behind
+ CachedConnection.warnedOnce.clear();
+ borrowExpectingFailure(url);
+ assertEquals(CachedConnection.warnedOnce, Collections.singleton(key),
+ "a connection string no bound of this class can reach was not reported");
+
+ // ... and once: every operation of the backend borrows through here, so a report per borrow
+ // is one nobody reads. A second borrow that would log again is one that adds a key again.
+ CachedConnection.warnedOnce.remove(key);
+ borrowExpectingFailure(url);
+ assertEquals(CachedConnection.warnedOnce, Collections.singleton(key),
+ "the report is not the one warnOnce() gates: " + CachedConnection.warnedOnce);
+ borrowExpectingFailure(url);
+ assertEquals(CachedConnection.warnedOnce, Collections.singleton(key),
+ "the same url was reported a second time");
+ }
+
+ /** A borrow that has to fail: what the test is after is what was logged on the way. */
+ private static void borrowExpectingFailure(String url) throws Exception {
+ try {
+ CachedConnection.getConnection(url);
+ fail("a connection string no registered driver accepts must be reported");
+ } catch (SQLException expected) {
+ // the point of the test is what was logged on the way, not what came back
+ }
+ }
+
+ /**
+ * ... and so is a postgresql url that turns the read bound off: a parameter of one outranks the
+ * property this class supplies, so a "socketTimeout=0" there cannot be replaced. Nothing is
+ * left to end a borrow that reaches a database accepting the connection and answering nothing,
+ * and an administrator who wrote that zero has to be able to find it in the log.
+ */
+ @Test
+ public void testAReadBoundTurnedOffInAPostgresUrlIsReportedOnce() throws Exception {
+ final String url = "jdbc:postgresql://reported:5432/db?socketTimeout=0";
+ final String key = CachedConnection.safeUrl(url) + "|unbounded|socketTimeout";
+ CachedConnection.warnedOnce.clear();
+ assertFalse(CachedConnection.ConnectDialect.POSTGRES.bound(url, new Properties(), 7));
+ assertEquals(CachedConnection.warnedOnce, Collections.singleton(key),
+ "a url leaving the reads of its login unbounded was not reported");
+
+ assertFalse(CachedConnection.ConnectDialect.POSTGRES.bound(url, new Properties(), 7));
+ assertEquals(CachedConnection.warnedOnce, Collections.singleton(key),
+ "the same url was reported a second time");
+ }
+
+ /**
+ * ... and every parameter of it that is turned off, not only the first one. A url is free to
+ * turn off the read bound and the login bound both, and the login bound is the per-host budget
+ * of a failover url - safeUrl() keeps neither parameter, so a key of the url alone would name
+ * the first offender, remember the url as reported, and leave the rest of them unmentionable.
+ */
+ @Test
+ public void testEveryBoundTurnedOffInAPostgresUrlIsReported() throws Exception {
+ final String url = "jdbc:postgresql://reported-twice:5432/db?socketTimeout=0&loginTimeout=0";
+ final String safe = CachedConnection.safeUrl(url);
+ CachedConnection.warnedOnce.clear();
+
+ CachedConnection.ConnectDialect.POSTGRES.bound(url, new Properties(), 7);
+
+ assertTrue(CachedConnection.warnedOnce.contains(safe + "|unbounded|socketTimeout"),
+ "the read bound left at 0 was not reported: " + CachedConnection.warnedOnce);
+ assertTrue(CachedConnection.warnedOnce.contains(safe + "|unbounded|loginTimeout"),
+ "the login bound left at 0 was not reported: " + CachedConnection.warnedOnce);
+ }
+
+ /**
+ * Nothing in the chain of a failure names the password of the backend, however deep it stands.
+ * Walked by identity rather than link by link: a driver is free to make the cause and the next
+ * exception of a link the same failure, which is the very shape the sibling test builds, and a
+ * helper looping on it would hang the run it is checking for exactly that.
+ */
+ private static void assertNoCredentials(Throwable failure) {
+ final Set<Throwable> seen = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
+ final Deque<Throwable> pending = new ArrayDeque<>();
+ enqueue(pending, seen, failure);
+ while (!pending.isEmpty()) {
+ final Throwable t = pending.poll();
+ assertFalse(String.valueOf(t.getMessage()).contains("S3cretOfTheBackend"),
+ "the password of the backend reached a message: " + t);
+ if (t instanceof SQLException) {
+ enqueue(pending, seen, ((SQLException) t).getNextException());
+ }
+ enqueue(pending, seen, t.getCause());
+ }
+ }
+
+ private static void enqueue(Deque<Throwable> pending, Set<Throwable> seen, Throwable t) {
+ if (t != null && seen.add(t)) {
+ pending.add(t);
+ }
+ }
+
+ /**
+ * A link of a chain carries a cause and a next exception both, and a driver is free to make the
+ * two the same failure. Rebuilt under a bound on the depth alone, a chain of those is copied
+ * twice over at every step - 2^32 links for one reaching the bound, which is a report of a
+ * failed connect that never comes back. What bounds the redaction is the number of links it
+ * rebuilds, the way the walk looking for credentials is bounded by the ones it visits.
+ */
+ @Test(timeOut = 60000)
+ public void testAFailureWhoseCauseIsItsNextExceptionIsRedactedInBoundedTime() throws Exception {
+ final String url = "jdbc:postgresql://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
+ SQLException chain = new SQLException("connect to " + url + " failed", "08006", 1);
+ for (int i = 0; i < 64; i++) {
+ final SQLException link = new SQLException("link " + i + " of " + url, "08006", i);
+ link.setNextException(chain);
+ link.initCause(chain);
+ chain = link;
+ }
+ final SQLException reported = CachedConnection.reported(chain, url);
+ assertNoCredentials(reported);
+ assertEquals(reported.getSQLState(), "08006", "the SQLState of a link has to survive its redaction");
+ }
+
+ /**
+ * ... and the chain of one is walked link by link rather than copy by copy. Enqueued twice, a
+ * failure whose cause is its own next exception fans out into a level twice the size of the one
+ * above it, so five levels of it are enough to spend the whole budget of the walk: the link
+ * that names the url stands at level six of seven and was never reached - and a walk that ends
+ * without finding credentials is one that reports the failure as it stands, password and all.
+ */
+ @Test(timeOut = 60000)
+ public void testACredentialBehindADuplicatedChainIsStillRedacted() throws Exception {
+ final String url = "jdbc:postgresql://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
+ SQLException chain = new SQLException("connect to " + url + " failed", "08006", 1);
+ for (int i = 0; i < 6; i++) {
+ final SQLException link = new SQLException("wrapper " + i, "08006", i);
+ link.setNextException(chain);
+ link.initCause(chain);
+ chain = link;
+ }
+
+ assertNoCredentials(CachedConnection.reported(chain, url));
+ }
+
+ /**
+ * The tail a rebuild has no budget left for is named rather than dropped. Without it the same
+ * failure keeps its root cause where the url of the backend carries no password and loses it
+ * without a word where it does - and the root cause is what a report of a failed connect is
+ * read for.
+ */
+ @Test(timeOut = 60000)
+ public void testTheTailOfALongChainIsNamedRatherThanDropped() throws Exception {
+ final String url = "jdbc:postgresql://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
+ SQLException chain = new SQLException("Connection to 127.0.0.1:5432 refused", "08006", 1);
+ for (int i = 0; i < 40; i++) {
+ final SQLException link = new SQLException("wrapper " + i + " of " + url, "08006", i);
+ link.initCause(chain);
+ chain = link;
+ }
+
+ final SQLException reported = CachedConnection.reported(chain, url);
+
+ assertNoCredentials(reported);
+ Throwable last = reported;
+ while (last.getCause() != null) {
+ last = last.getCause();
+ }
+ assertTrue(String.valueOf(last.getMessage()).contains("left out"),
+ "the tail a rebuild had no budget for has to say so: " + last.getMessage());
+ }
+
+ /**
+ * A database that is not listening at all: every dialect reports it instead of retrying the
+ * refused connect until the caller gives up on the operation.
+ */
+ @Test(timeOut = 120000)
+ public void testRefusedConnectIsReportedAtOnce() throws Exception {
+ final int closedPort = closedPort();
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, Long.toString(BOUND_SECONDS));
+ for (final String url : urlsOf(closedPort)) {
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection(url);
+ fail("a refused connect must be reported: " + CachedConnection.safeUrl(url));
+ } catch (SQLException expected) {
+ // the failure of the moment, reported rather than retried
+ }
+ assertElapsedWithinBound(startedAt, BOUND_SECONDS * 1000);
+ }
+ }
+
+ /**
+ * The failure this bound exists for: a database that completes the TCP connection and then
+ * says nothing - a moved VIP, a proxy at its connection limit, a host that lost its answer -
+ * leaving the login of the driver, and with it the operation, without an end. The connection
+ * of the accept queue is never answered here, so every dialect has to give up on its own.
+ */
+ @Test(timeOut = 300000)
+ public void testLoginIsBoundedWhenTheDatabaseNeverAnswers() throws Exception {
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, Long.toString(BOUND_SECONDS));
+ // a socket that is bound and never accepted: the kernel completes the handshake, so the
+ // connect of the driver succeeds and every read of the login that follows hangs
+ try (final ServerSocket blackhole = new ServerSocket(0, 50, InetAddress.getLoopbackAddress())) {
+ for (final String url : urlsOf(blackhole.getLocalPort())) {
+ // borrowed on a thread of its own: a bound that a driver does not honour has to
+ // fail this test at once, and not by hanging the run it is part of
+ final FutureTask<Connection> borrow = new FutureTask<>(() -> CachedConnection.getConnection(url));
+ final Thread thread = new Thread(borrow, "borrow-" + CachedConnection.safeUrl(url));
+ thread.setDaemon(true);
+ thread.start();
+ try {
+ final Connection con = borrow.get(BOUND_SECONDS * 1000 + BOUND_MARGIN_MS, TimeUnit.MILLISECONDS);
+ fail("a database that never answers must not hand out a connection: " + con);
+ } catch (TimeoutException e) {
+ fail("the login of " + CachedConnection.safeUrl(url) + " is not bounded: it never gave up");
+ } catch (ExecutionException expected) {
+ assertTrue(expected.getCause() instanceof SQLException, String.valueOf(expected.getCause()));
+ }
+ }
+ }
+ }
+
+ /**
+ * The deadline of the borrow bounds the attempt inside it as well: the pool timeout stands for
+ * the whole borrow, and an attempt left to run out its own bound would overrun it by that bound.
+ */
+ @Test(timeOut = 120000)
+ public void testTheAttemptIsBoundedByTheDeadlineOfTheBorrow() throws Exception {
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "600");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "2");
+ try (final ServerSocket blackhole = new ServerSocket(0, 50, InetAddress.getLoopbackAddress())) {
+ final String url = "jdbc:postgresql://127.0.0.1:" + blackhole.getLocalPort() + "/opendj?user=opendj&password=opendj";
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection(url);
+ fail("a database that never answers must not hand out a connection");
+ } catch (SQLException expected) {
+ // reported, and within the borrow it was given rather than the 600 s of the attempt
+ }
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ assertTrue(elapsed < 2000 + BOUND_MARGIN_MS,
+ "the attempt outlived the deadline of the borrow: " + elapsed + " ms");
+ }
+ }
+
+ /** Pool exhaustion stays a retry - one of our own connections is on its way back to the pool. */
+ @Test(timeOut = 120000)
+ public void testConnectionLimitIsRetried() throws Exception {
+ final String url = StubDriver.PREFIX + "retried";
+ stub.failWith(tooManyConnections(), 2);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30");
+
+ final Connection con = CachedConnection.getConnection(url);
+
+ assertNotNull(con);
+ assertEquals(stub.attempts.get(), 3, "the connect must be retried while the database is at its limit");
+ }
+
+ /** ... but under a deadline: the retry used to double its wait from 1 ms with no end to it. */
+ @Test(timeOut = 120000)
+ public void testConnectionLimitGivesUpAtTheDeadline() throws Exception {
+ final String url = StubDriver.PREFIX + "deadline";
+ stub.failWith(tooManyConnections(), StubDriver.ALWAYS);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "2");
+
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection(url);
+ fail("a database that stays at its connection limit must be reported, not waited out forever");
+ } catch (SQLTimeoutException expected) {
+ assertTrue(expected.getMessage().contains("2s"), expected.getMessage());
+ assertEquals(((SQLException) expected.getCause()).getSQLState(), "53300");
+ // the state of a connect that did not happen, rather than none at all: this is the
+ // failure of a borrow, and monitoring reading the state off what it caught would
+ // otherwise see null where the driver's own exception carried one
+ assertEquals(expected.getSQLState(), "08001");
+ }
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ assertTrue(elapsed >= 2000, "gave up after " + elapsed + " ms, before the deadline it was given");
+ assertElapsedWithinBound(startedAt, 2000);
+ assertTrue(stub.attempts.get() > 1, "the connect must be retried while the deadline lasts");
+ }
+
+ /**
+ * A database on its way up - starting, recovering, shutting down - says so, and says it for
+ * seconds: the backend it belongs to would otherwise stay locked down until the next restart
+ * of the server, since nothing above JDBCStorage.open() attempts it a second time.
+ */
+ @Test(timeOut = 120000)
+ public void testDatabaseOnItsWayUpIsRetried() throws Exception {
+ final String url = StubDriver.PREFIX + "starting-up";
+ stub.failWith(new SQLException("the database system is starting up", "57P03"), 2);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30");
+
+ final Connection con = CachedConnection.getConnection(url);
+
+ assertNotNull(con);
+ assertEquals(stub.attempts.get(), 3, "a database that is starting up must be waited out");
+ }
+
+ /** ... and it is recognized however the driver wrapped it: a SQLException carries two chains. */
+ @Test(timeOut = 120000)
+ public void testTheWholeChainOfTheFailureIsLookedAt() throws Exception {
+ final String url = StubDriver.PREFIX + "wrapped";
+ final SQLException wrapped = new SQLException("could not connect to the server", "08006");
+ wrapped.setNextException(tooManyConnections());
+ stub.failWith(wrapped, 1);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30");
+
+ assertNotNull(CachedConnection.getConnection(url));
+ assertEquals(stub.attempts.get(), 2, "the failure behind the one reported 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.
+ */
+ @Test(timeOut = 120000)
+ public void testDiskFullIsNotRetried() throws Exception {
+ final String url = StubDriver.PREFIX + "disk-full";
+ stub.failWith(new SQLException("could not extend file: No space left on device", "53100"), StubDriver.ALWAYS);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30");
+
+ try {
+ CachedConnection.getConnection(url);
+ fail("a database out of disk must be reported to the caller");
+ } catch (SQLException expected) {
+ assertEquals(expected.getSQLState(), "53100");
+ }
+ assertEquals(stub.attempts.get(), 1, "a failure that waiting cannot clear must be attempted once");
+ }
+
+ /**
+ * Every other failure is the caller's to report. A password the database does not accept is
+ * never going to be accepted by waiting, and the retry that swallowed it left the operation
+ * hanging with nothing in the log.
+ */
+ @Test(timeOut = 120000)
+ public void testRejectedLoginIsNotRetried() throws Exception {
+ final String url = StubDriver.PREFIX + "rejected";
+ stub.failWith(new SQLException("password authentication failed", "28P01"), StubDriver.ALWAYS);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "30");
+
+ try {
+ CachedConnection.getConnection(url);
+ fail("a rejected login must be reported to the caller");
+ } catch (SQLException expected) {
+ assertEquals(expected.getSQLState(), "28P01");
+ }
+ assertEquals(stub.attempts.get(), 1, "a rejected login must be attempted once");
+ }
+
+ /** A connection the setup of which failed belongs to nobody: it has to be closed, not leaked. */
+ @Test(timeOut = 120000)
+ public void testConnectionIsClosedWhenItsSetupFails() throws Exception {
+ final String url = StubDriver.PREFIX + "setup-failure";
+ final Connection broken = mock(Connection.class);
+ doThrow(new SQLException("read only")).when(broken).setAutoCommit(false);
+ stub.answerWith(broken);
+
+ try {
+ CachedConnection.getConnection(url);
+ fail("a connection that cannot be set up must be reported");
+ } catch (SQLException expected) {
+ assertEquals(expected.getMessage(), "read only");
+ }
+ verify(broken).close();
+ }
+
+ /** The same, for a driver whose failure in the setup is not a SQLException but an unchecked one. */
+ @Test(timeOut = 120000)
+ public void testConnectionIsClosedWhenItsSetupFailsWithAnUncheckedError() throws Exception {
+ final String url = StubDriver.PREFIX + "setup-unchecked";
+ final Connection broken = mock(Connection.class);
+ doThrow(new IllegalStateException("driver internal")).when(broken).setTransactionIsolation(anyInt());
+ stub.answerWith(broken);
+
+ try {
+ CachedConnection.getConnection(url);
+ fail("a connection that cannot be set up must be reported");
+ } catch (IllegalStateException expected) {
+ assertEquals(expected.getMessage(), "driver internal");
+ }
+ verify(broken).close();
+ }
+
+ /**
+ * isValid(n) is not a bound at the socket on every driver - the SQL Server driver turns it
+ * into a query timeout, which needs an answer from the server to fire - and the read bound of
+ * the login was lifted the moment the connection was established, so the socket carries the
+ * bound of the validation, for the length of the validation only.
+ */
+ @Test(timeOut = 120000)
+ public void testValidationOfAPooledConnectionIsBoundedAtTheSocket() throws Exception {
+ final String url = StubDriver.PREFIX + "validation-bound";
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ when(pooled.getNetworkTimeout()).thenReturn(0);
+ seedPool(url, pooled);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, pooled);
+ final InOrder inOrder = inOrder(pooled);
+ inOrder.verify(pooled).setNetworkTimeout(any(Executor.class), eq(CachedConnection.VALIDATION_TIMEOUT_SECONDS * 1000));
+ inOrder.verify(pooled).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ inOrder.verify(pooled).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * A driver that takes the call bounding the validation and then fails inside it is told apart
+ * from one that never takes a network timeout at all: it is free to have applied the bound
+ * before failing, so the connection is discarded rather than validated unbounded and handed
+ * out - pooled, it would carry five seconds of ours into every statement for the rest of its
+ * life, and the import batch of a backend open is the first thing to die of that.
+ */
+ @Test(timeOut = 120000)
+ public void testAPooledConnectionThatCannotBeBoundedForItsValidationIsDiscarded() throws Exception {
+ final String url = StubDriver.PREFIX + "validation-bound-fails";
+ final Connection pooled = mock(Connection.class);
+ when(pooled.getNetworkTimeout()).thenReturn(0);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ doThrow(new SQLException("the driver took the bound and then failed"))
+ .when(pooled).setNetworkTimeout(any(Executor.class), anyInt());
+ seedPool(url, pooled);
+ final Connection fresh = mock(Connection.class);
+ stub.answerWith(fresh);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh, "a connection this could not bound was handed out");
+ verify(pooled, never()).isValid(anyInt());
+ verify(pooled).close();
+ }
+
+ /**
+ * A driver answering a negative network timeout is outside the contract of the call - 0 is no
+ * limit and nothing below it stands for anything - and taken back as it is, it is one of the
+ * two sentinels this class tells its own outcomes apart by: the bound of the validation would
+ * be read as a bound that was never set, and the connection would go back into the pool still
+ * carrying five seconds of ours into every statement of the next borrower.
+ */
+ @Test(timeOut = 120000)
+ public void testAPooledConnectionWhoseDriverAnswersANegativeBoundIsPutBackUnbounded() throws Exception {
+ final String url = StubDriver.PREFIX + "validation-negative-bound";
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ when(pooled.getNetworkTimeout()).thenReturn(-1);
+ CachedConnection.cached.get(url).add(new CachedConnection(url, pooled));
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, pooled);
+ final InOrder inOrder = inOrder(pooled);
+ inOrder.verify(pooled)
+ .setNetworkTimeout(any(Executor.class), eq(CachedConnection.VALIDATION_TIMEOUT_SECONDS * 1000));
+ inOrder.verify(pooled).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ inOrder.verify(pooled).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /** A read bound of the connection string is tighter than ours and stays untouched. */
+ @Test(timeOut = 120000)
+ public void testValidationLeavesTheBoundOfTheConnectionStringAlone() throws Exception {
+ final String url = StubDriver.PREFIX + "validation-tighter";
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ when(pooled.getNetworkTimeout()).thenReturn(2000);
+ seedPool(url, pooled);
+
+ assertNotNull(CachedConnection.getConnection(url));
+
+ verify(pooled, never()).setNetworkTimeout(any(Executor.class), anyInt());
+ }
+
+ /**
+ * The pool has no upper bound on the number of connections it holds, and a validation is a
+ * round trip: after a failover that left them half-open, draining the pool must not outlive
+ * the deadline of the borrow - establishing a connection is the faster answer past it.
+ */
+ @Test(timeOut = 120000)
+ public void testDrainOfThePoolStopsAtTheDeadline() throws Exception {
+ final String url = StubDriver.PREFIX + "drain-deadline";
+ final int pooled = 8;
+ final AtomicInteger validated = new AtomicInteger();
+ for (int i = 0; i < pooled; i++) {
+ final Connection stale = mock(Connection.class);
+ when(stale.isValid(anyInt())).thenAnswer(invocation -> {
+ validated.incrementAndGet();
+ Thread.sleep(500); // a database that no longer answers: every validation waits out its bound
+ return false;
+ });
+ seedPool(url, stale);
+ }
+ final Connection fresh = mock(Connection.class);
+ stub.answerWith(fresh);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "1");
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh);
+ assertTrue(validated.get() > 0 && validated.get() < pooled,
+ "the drain has to start and to stop at the deadline: " + validated.get() + " of " + pooled);
+ }
+
+ /** A pooled connection that no longer validates is closed and replaced, not handed out. */
+ @Test(timeOut = 120000)
+ public void testBrokenPooledConnectionIsDiscarded() throws Exception {
+ final String url = StubDriver.PREFIX + "broken-pooled";
+ final Connection stale = mock(Connection.class);
+ when(stale.isValid(anyInt())).thenReturn(false);
+ seedPool(url, stale);
+ final Connection fresh = mock(Connection.class);
+ when(fresh.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(fresh);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh);
+ verify(stale).close();
+ // the validation of a pooled connection needs a bound of its own as well
+ verify(stale, never()).isValid(0);
+ verify(stale).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+
+ /**
+ * The same for a driver whose answer to the validation is an unchecked failure: it unwinds
+ * through poll(), which stands outside every try of the borrow, so the connection it was
+ * raised over is already out of the pool and would be held by nobody.
+ */
+ @Test(timeOut = 120000)
+ public void testAPooledConnectionWhoseValidationThrowsIsDiscarded() throws Exception {
+ final String url = StubDriver.PREFIX + "validation-unchecked";
+ final Connection broken = mock(Connection.class);
+ when(broken.isValid(anyInt())).thenThrow(new IllegalStateException("driver internal"));
+ seedPool(url, broken);
+ final Connection fresh = mock(Connection.class);
+ stub.answerWith(fresh);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh);
+ verify(broken).close();
+ }
+
+ /** A connection that cannot be rolled back must not go back into the pool - nor be dropped. */
+ @Test(timeOut = 120000)
+ public void testConnectionThatCannotBeRolledBackIsClosed() throws Exception {
+ final String url = StubDriver.PREFIX + "rollback-failure";
+ final Connection parent = mock(Connection.class);
+ doThrow(new SQLException("connection is closed")).when(parent).rollback();
+
+ try {
+ new CachedConnection(url, parent).close();
+ fail("a failed rollback must be reported");
+ } catch (SQLException expected) {
+ assertEquals(expected.getMessage(), "connection is closed");
+ }
+ verify(parent).close();
+ assertTrue(CachedConnection.cached.get(url).isEmpty(), "a connection that cannot be rolled back was pooled");
+ }
+
+ @Test
+ public void testDialectIsRecognizedByTheConnectionString() throws Exception {
+ assertEquals(CachedConnection.ConnectDialect.of("jdbc:postgresql://h:5432/db"), CachedConnection.ConnectDialect.POSTGRES);
+ assertEquals(CachedConnection.ConnectDialect.of("jdbc:mysql://h:3306/db"), CachedConnection.ConnectDialect.MYSQL);
+ assertEquals(CachedConnection.ConnectDialect.of("jdbc:oracle:thin:@//h:1521/svc"), CachedConnection.ConnectDialect.ORACLE);
+ assertEquals(CachedConnection.ConnectDialect.of("jdbc:sqlserver://h:1433;databaseName=db"), CachedConnection.ConnectDialect.MICROSOFT);
+ assertNull(CachedConnection.ConnectDialect.of("jdbc:h2:mem:db"), "an unknown engine must not be fed the properties of another");
+ }
+
+ /** Both phases are bounded, in the units of the driver: the connect alone leaves the login open. */
+ @Test
+ public void testBothPhasesOfTheLoginAreBounded() throws Exception {
+ // pgjdbc puts an SO_TIMEOUT on the login socket only where socketTimeout is set: without it
+ // loginTimeout bounds the caller alone, and the thread the driver runs the login on stays
+ // parked in the read it abandoned
+ final Properties postgres = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.POSTGRES.bound("jdbc:postgresql://h:5432/db", postgres, 7),
+ "the read bound of postgresql outlives the login and has to be lifted");
+ assertEquals(postgres.getProperty("connectTimeout"), "7");
+ assertEquals(postgres.getProperty("socketTimeout"), "7");
+ assertEquals(postgres.getProperty("loginTimeout"), "7", "the bound of a url naming more than one host");
+
+ final Properties mysql = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.MYSQL.bound("jdbc:mysql://h:3306/db", mysql, 7),
+ "the read bound of mysql outlives the login and has to be lifted");
+ assertEquals(mysql.getProperty("connectTimeout"), "7000");
+ assertEquals(mysql.getProperty("socketTimeout"), "7000");
+
+ final Properties oracle = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.ORACLE.bound("jdbc:oracle:thin:@//h:1521/svc", oracle, 7));
+ assertEquals(oracle.getProperty("oracle.net.CONNECT_TIMEOUT"), "7000");
+ assertEquals(oracle.getProperty("oracle.jdbc.ReadTimeout"), "7000");
+
+ // the loginTimeout of the sql server driver leaves the read of the prelogin answer open
+ final Properties microsoft = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.MICROSOFT.bound("jdbc:sqlserver://h:1433;databaseName=db", microsoft, 7));
+ assertEquals(microsoft.getProperty("loginTimeout"), "7");
+ assertEquals(microsoft.getProperty("socketTimeout"), "7000");
+ }
+
+ /**
+ * A driver with a range of its own for its connect property is never handed a value beyond it:
+ * SQLServerDriverIntProperty.LOGIN_TIMEOUT is validated against [0, 65535], so a bound past
+ * that would not widen the connect, it would fail every one of them.
+ */
+ @Test
+ public void testConnectBoundStaysInTheRangeTheDriverTakes() throws Exception {
+ final Properties microsoft = new Properties();
+ CachedConnection.ConnectDialect.MICROSOFT.bound("jdbc:sqlserver://h:1433;databaseName=db", microsoft, 100000);
+ assertEquals(microsoft.getProperty("loginTimeout"), "65535");
+ assertEquals(microsoft.getProperty("socketTimeout"), "100000000", "the read bound takes any value");
+ }
+
+ /**
+ * A bound the administrator put into the connection string by hand - the only workaround this
+ * backend had - keeps precedence, property by property.
+ */
+ @Test
+ public void testConnectionStringKeepsPrecedence() throws Exception {
+ final Properties postgres = new Properties();
+ CachedConnection.ConnectDialect.POSTGRES.bound(
+ "jdbc:postgresql://h:5432/db?user=u&password=p&loginTimeout=30&socketTimeout=300", postgres, 7);
+ assertNull(postgres.getProperty("loginTimeout"), "the setting of the connection string was overridden");
+ assertNull(postgres.getProperty("socketTimeout"), "the setting of the connection string was overridden");
+ assertNull(postgres.getProperty("connectTimeout"),
+ "the connect side is one budget: a bound of the administrator under either of its names is theirs");
+
+ // the sql server driver gives a supplied property precedence over the one of the url
+ final Properties microsoft = new Properties();
+ CachedConnection.ConnectDialect.MICROSOFT.bound("jdbc:sqlserver://h:1433;loginTimeout=45;databaseName=db", microsoft, 7);
+ assertNull(microsoft.getProperty("loginTimeout"), "the setting of the connection string was overridden");
+ assertEquals(microsoft.getProperty("socketTimeout"), "7000");
+
+ // inside the descriptor of an oracle tns url the property goes by the last segment of its name
+ final Properties oracle = new Properties();
+ CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(DESCRIPTION=(CONNECT_TIMEOUT=3)(ADDRESS=(HOST=h)(PORT=1521)))", oracle, 7);
+ assertNull(oracle.getProperty("oracle.net.CONNECT_TIMEOUT"));
+ assertEquals(oracle.getProperty("oracle.jdbc.ReadTimeout"), "7000");
+
+ // inside the descriptor the read bound goes by READ_TIMEOUT, and one of the administrator
+ // is never lifted after the login, because ours is not set on top of it
+ final Properties read = new Properties();
+ assertFalse(CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(DESCRIPTION=(READ_TIMEOUT=30)(ADDRESS=(HOST=h)(PORT=1521)))", read, 7),
+ "a read bound of the connection string must not be lifted once the login is through");
+ assertNull(read.getProperty("oracle.jdbc.ReadTimeout"));
+
+ // ... while RECV_TIMEOUT is a parameter of sqlnet.ora and of the listener that ojdbc8 does
+ // not read - the name appears nowhere in the driver - so a descriptor carrying one is not
+ // a read bound of the connection and must not take ours off it
+ final Properties recv = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(DESCRIPTION=(RECV_TIMEOUT=30)(ADDRESS=(HOST=h)(PORT=1521)))", recv, 7),
+ "a parameter no driver reads left the login of this connection unbounded");
+ assertEquals(recv.getProperty("oracle.jdbc.ReadTimeout"), "7000");
+
+ // a name that only appears as the tail of another parameter is not a setting of its own
+ final Properties mysql = new Properties();
+ CachedConnection.ConnectDialect.MYSQL.bound("jdbc:mysql://h:3306/db?xconnectTimeout=1&socketTimeoutX=2", mysql, 7);
+ assertEquals(mysql.getProperty("connectTimeout"), "7000");
+ assertEquals(mysql.getProperty("socketTimeout"), "7000");
+ }
+
+ /**
+ * A parameter is recognized the way the driver of its dialect recognizes it: pgjdbc and
+ * Connector/J look their properties up by their exact name, so a name of another case is a
+ * parameter of nobody - neither side bounds anything by it - and must not pass for a bound the
+ * administrator set, while the other two match either way.
+ */
+ @Test
+ public void testTheCaseOfAParameterIsTheOneOfItsDriver() throws Exception {
+ final Properties postgres = new Properties();
+ CachedConnection.ConnectDialect.POSTGRES.bound("jdbc:postgresql://h:5432/db?ConnectTimeout=5", postgres, 7);
+ assertEquals(postgres.getProperty("connectTimeout"), "7", "pgjdbc ignores a parameter of another case");
+
+ // PropertyKey.fromValue("SocketTimeout") answers null, and Connector/J then reads no bound
+ // out of the url either: a mis-cased parameter left the borrower parked on a host that
+ // completes the handshake and says nothing
+ final Properties mysql = new Properties();
+ CachedConnection.ConnectDialect.MYSQL.bound("jdbc:mysql://h:3306/db?SocketTimeout=1", mysql, 7);
+ assertEquals(mysql.getProperty("socketTimeout"), "7000", "Connector/J ignores a parameter of another case");
+
+ final Properties microsoft = new Properties();
+ CachedConnection.ConnectDialect.MICROSOFT.bound("jdbc:sqlserver://h:1433;LoginTimeout=45", microsoft, 7);
+ assertNull(microsoft.getProperty("loginTimeout"), "the sql server driver normalizes the name of a property");
+
+ // the keywords of an oracle descriptor are matched without regard to case as well
+ final Properties oracle = new Properties();
+ CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(description=(connect_timeout=3)(address=(host=h)(port=1521)))", oracle, 7);
+ assertNull(oracle.getProperty("oracle.net.CONNECT_TIMEOUT"), "an oracle descriptor is read without case");
+ }
+
+ /**
+ * The connection string is not the only channel of the administrator: the oracle driver reads
+ * some of its properties out of the system properties as well, which is how a whole jvm is
+ * bounded with -Doracle.jdbc.ReadTimeout. A property supplied to the driver outranks that one
+ * without a word, and this class would then lift it once the login is through as if it were
+ * its own - leaving a connection with no read bound at all where the administrator set one.
+ */
+ @Test
+ public void testASystemPropertyOfTheAdministratorKeepsPrecedence() throws Exception {
+ System.setProperty("oracle.jdbc.ReadTimeout", "30000");
+ try {
+ final Properties oracle = new Properties();
+ assertFalse(CachedConnection.ConnectDialect.ORACLE.bound("jdbc:oracle:thin:@//h:1521/svc", oracle, 7),
+ "a read bound of the administrator must not be lifted once the login is through");
+ assertNull(oracle.getProperty("oracle.jdbc.ReadTimeout"), "the setting of the administrator was overridden");
+ assertEquals(oracle.getProperty("oracle.net.CONNECT_TIMEOUT"), "7000",
+ "the property it leaves open must still be bounded");
+ } finally {
+ System.clearProperty("oracle.jdbc.ReadTimeout");
+ }
+
+ // the connect property of the same driver, over the same channel
+ System.setProperty("oracle.net.CONNECT_TIMEOUT", "30000");
+ try {
+ final Properties oracle = new Properties();
+ CachedConnection.ConnectDialect.ORACLE.bound("jdbc:oracle:thin:@//h:1521/svc", oracle, 7);
+ assertNull(oracle.getProperty("oracle.net.CONNECT_TIMEOUT"), "the setting of the administrator was overridden");
+ } finally {
+ System.clearProperty("oracle.net.CONNECT_TIMEOUT");
+ }
+
+ // a plain name is common enough to be somebody else's system property: only a name a
+ // driver of these actually reads out of them is one of the administrator's
+ System.setProperty("socketTimeout", "30000");
+ try {
+ final Properties mysql = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.MYSQL.bound("jdbc:mysql://h:3306/db", mysql, 7));
+ assertEquals(mysql.getProperty("socketTimeout"), "7000");
+ } finally {
+ System.clearProperty("socketTimeout");
+ }
+ }
+
+ /**
+ * ... and a dotted name is not a name a driver reads out of the system properties by the shape
+ * of it. ojdbc8 resolves oracle.jdbc.ReadTimeout and oracle.net.CONNECT_TIMEOUT in three tiers
+ * (the properties it was supplied, then System.getProperty, then the properties of the data
+ * source), while oracle.net.READ_TIMEOUT - a dotted name of the same driver, and the name the
+ * socket option is finally read under - reaches the socket from the connection properties
+ * alone: the classes carrying the literal hand it to Properties.get, none of them to
+ * System.getProperty. Taken for a bound of the administrator, a -D of it leaves the login with
+ * no read bound whatever: theirs is not read and ours is not set.
+ */
+ @Test
+ public void testASystemPropertyNoDriverReadsIsNoBound() throws Exception {
+ System.setProperty("oracle.net.READ_TIMEOUT", "30000");
+ try {
+ final Properties oracle = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.ORACLE.bound("jdbc:oracle:thin:@//h:1521/svc", oracle, 7),
+ "a -D the driver never reads left this login with no read bound at all");
+ assertEquals(oracle.getProperty("oracle.jdbc.ReadTimeout"), "7000");
+ } finally {
+ System.clearProperty("oracle.net.READ_TIMEOUT");
+ }
+
+ // ... while the same name written into the connection string is one the driver does read
+ final Properties declared = new Properties();
+ assertFalse(CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(DESCRIPTION=(READ_TIMEOUT=30)(ADDRESS=(HOST=h)(PORT=1521)))", declared, 7));
+ assertNull(declared.getProperty("oracle.jdbc.ReadTimeout"));
+ }
+
+ /**
+ * A property the administrator set to 0 is not a bound of theirs: every one of these drivers
+ * reads 0 as "wait as long as it takes", which is the default this class exists to replace -
+ * and on the three whose driver lets a supplied property win, ours is set on top of it.
+ * Postgresql is the one where it cannot be, and is covered on its own below.
+ */
+ @Test
+ public void testAZeroIsNotABoundOfTheAdministrator() throws Exception {
+ final Properties mysql = new Properties();
+ CachedConnection.ConnectDialect.MYSQL.bound("jdbc:mysql://h:3306/db?connectTimeout=0&socketTimeout=0", mysql, 7);
+ assertEquals(mysql.getProperty("connectTimeout"), "7000");
+ assertEquals(mysql.getProperty("socketTimeout"), "7000");
+
+ // ... and neither is a property left without a value
+ final Properties microsoft = new Properties();
+ CachedConnection.ConnectDialect.MICROSOFT.bound("jdbc:sqlserver://h:1433;socketTimeout=;databaseName=db", microsoft, 7);
+ assertEquals(microsoft.getProperty("socketTimeout"), "7000");
+
+ // the same of a system property, and of the descriptor of an oracle url
+ System.setProperty("oracle.jdbc.ReadTimeout", "0");
+ try {
+ final Properties oracle = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.ORACLE.bound(
+ "jdbc:oracle:thin:@(DESCRIPTION=(READ_TIMEOUT=0)(ADDRESS=(HOST=h)(PORT=1521)))", oracle, 7));
+ assertEquals(oracle.getProperty("oracle.jdbc.ReadTimeout"), "7000");
+ } finally {
+ System.clearProperty("oracle.jdbc.ReadTimeout");
+ }
+
+ // a value that is no number is left to the driver it belongs to: it is not this class's to read
+ final Properties unreadable = new Properties();
+ assertFalse(CachedConnection.ConnectDialect.MYSQL.bound(
+ "jdbc:mysql://h:3306/db?socketTimeout=PT30S", unreadable, 7));
+ assertNull(unreadable.getProperty("socketTimeout"));
+ }
+
+ /**
+ * On postgresql a parameter of the url outranks the property this class supplies: Driver
+ * .connect copies what it was handed into a flat map and parseURL then writes the parameters of
+ * the url on top of it. So a "socketTimeout=0" there cannot be replaced, and setting ours
+ * regardless would leave this class believing it bounded a login that carries no bound - and
+ * lifting a read bound after it that was never in force. The effective values are read back
+ * through the parser of the driver itself, since asserting on the map handed to it is
+ * asserting on the half of the story this bug lived in.
+ */
+ @Test
+ public void testAParameterOfAPostgresUrlOutranksTheBoundOfThisClass() throws Exception {
+ final String url = "jdbc:postgresql://h:5432/db?connectTimeout=0&socketTimeout=0&loginTimeout=0";
+ final Properties supplied = new Properties();
+ assertFalse(CachedConnection.ConnectDialect.POSTGRES.bound(url, supplied, 7),
+ "a read bound that never reaches the driver must not be reported as one to lift");
+ assertNull(supplied.getProperty("socketTimeout"));
+ assertNull(supplied.getProperty("connectTimeout"));
+ assertNull(supplied.getProperty("loginTimeout"));
+
+ final Properties effective = org.postgresql.Driver.parseURL(url, supplied);
+ assertEquals(effective.getProperty("socketTimeout"), "0", "the url is what the driver ends up reading");
+ assertEquals(effective.getProperty("connectTimeout"), "0");
+ assertEquals(effective.getProperty("loginTimeout"), "0");
+ }
+
+ /**
+ * The connect side of a dialect is one budget rather than a set of independent knobs, so a
+ * bound of the administrator under any of its names leaves all of them alone. On postgresql
+ * connectTimeout bounds the socket connect and loginTimeout the login behind it: filling in the
+ * one they left out caps the one they set, since Driver.connect hands the login to a thread of
+ * its own as soon as loginTimeout is anything but 0 and gives up on it there.
+ */
+ @Test
+ public void testAConnectBudgetOfTheAdministratorIsNotCappedByThisClass() throws Exception {
+ final String url = "jdbc:postgresql://h:5432/db?connectTimeout=300";
+ final Properties supplied = new Properties();
+ assertTrue(CachedConnection.ConnectDialect.POSTGRES.bound(url, supplied, 30),
+ "the read bound is a budget of its own and is still set");
+ assertNull(supplied.getProperty("loginTimeout"),
+ "a loginTimeout of ours caps the connectTimeout the administrator set");
+ assertNull(supplied.getProperty("connectTimeout"));
+ assertEquals(supplied.getProperty("socketTimeout"), "30");
+
+ final Properties effective = org.postgresql.Driver.parseURL(url, supplied);
+ assertEquals(effective.getProperty("connectTimeout"), "300", "the budget of the administrator, in full");
+ assertNull(effective.getProperty("loginTimeout"), "nothing of ours hands this login to a thread to abandon");
+ }
+
+ /**
+ * The bound handed to a driver stays inside the range an int of milliseconds takes. Where the
+ * per-attempt property is off, the attempt takes what is left of the deadline of the borrow,
+ * and the pool timeout has no upper bound of its own - while the SQL Server driver rejects a
+ * socketTimeout past Integer.MAX_VALUE outright, failing every connect of that backend with
+ * the name of a property nobody typed.
+ */
+ @Test(timeOut = 120000)
+ public void testTheBoundHandedToADriverStaysInTheRangeAnIntTakes() throws Exception {
+ final long deadline = System.currentTimeMillis() + 3000000L * 1000; // 34 days: past 2^31 ms
+ assertEquals(CachedConnection.attemptSeconds(0, deadline), Integer.MAX_VALUE / 1000);
+
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "0");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "3000000");
+ // a socket that answers the connect and closes it, rather than a port bound and released:
+ // with every bound of this borrow turned off, anything taking that port in between would
+ // leave the test hanging on the timeOut instead of failing
+ try (final ServerSocket rejecting = rejectingSocket()) {
+ final String url = "jdbc:sqlserver://127.0.0.1:" + rejecting.getLocalPort()
+ + ";databaseName=opendj;user=opendj;password=opendj;encrypt=false";
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection(url);
+ fail("a connect the database closes must be reported");
+ } catch (SQLException expected) {
+ assertFalse(expected.getMessage().contains("socketTimeout"),
+ "the driver was handed a bound it does not take: " + expected.getMessage());
+ }
+ assertElapsedWithinBound(startedAt, 0);
+ }
+ }
+
+ /** The clamp of the range holds where the borrow has no deadline to take it from either. */
+ @Test
+ public void testTheBoundOfAnAttemptStaysInRangeWithoutADeadline() throws Exception {
+ assertEquals(CachedConnection.attemptSeconds(Long.MAX_VALUE, Long.MAX_VALUE), Integer.MAX_VALUE / 1000);
+ assertEquals(CachedConnection.attemptSeconds(0, Long.MAX_VALUE), 0, "0 stands for an attempt with no bound");
+ assertEquals(CachedConnection.attemptSeconds(30, Long.MAX_VALUE), 30);
+ }
+
+ /** The connection string holds the credentials of the backend: a stall report must not carry them. */
+ @Test
+ public void testLoggedConnectionStringCarriesNoCredentials() throws Exception {
+ assertEquals(CachedConnection.safeUrl("jdbc:postgresql://h:5432/db?user=u&password=secret"), "jdbc:postgresql://h:5432/db");
+ // the parameter naming the database stays: two backends of one sql server host answer to
+ // the same url up to it, and a stall report that cannot tell them apart is one of neither
+ assertEquals(CachedConnection.safeUrl("jdbc:sqlserver://h:1433;databaseName=db;password=secret"),
+ "jdbc:sqlserver://h:1433;databaseName=db");
+ // ... and the token naming the kind of oracle driver stays as well: thin against oci is a
+ // first question of an oracle connect, and it stands in front of the credentials
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:scott/secret@//h:1521/svc"), "jdbc:oracle:thin:@//h:1521/svc");
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://u:secret@h:3306/db"), "jdbc:mysql://h:3306/db");
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:@//h:1521/svc"), "jdbc:oracle:thin:@//h:1521/svc");
+ // a password holding the parameter separator of another dialect: ";" separates nothing on
+ // an oracle url, so the credentials are cut in front of the "@" rather than inside them
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:scott/pa;ss@//h:1521/svc"), "jdbc:oracle:thin:@//h:1521/svc");
+ // ... and one holding a ":" is cut in front of it too: the token of the driver is the one
+ // behind the subprotocol, not the last one standing in front of the "@"
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:scott/pa:ss@//h:1521/svc"), "jdbc:oracle:thin:@//h:1521/svc");
+ // ... and an "@" that stands inside a parameter is not the end of credentials: the host survives
+ assertEquals(CachedConnection.safeUrl("jdbc:postgresql://h:5432/db?user=u@example.com&password=secret"),
+ "jdbc:postgresql://h:5432/db");
+ // a password holding the parameter separator of its own dialect: on an oracle url the
+ // parameters stand behind the descriptor, so a "?" in front of the "@" is part of the
+ // password and cutting there would leave the start of it in the log
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:scott/pa?ss@//h:1521/svc"), "jdbc:oracle:thin:@//h:1521/svc");
+ // the same inside an authority, where the credentials end at the path rather than at a "?"
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://u:sec?ret@h:3306/db"), "jdbc:mysql://h:3306/db");
+ // a descriptor of an oracle url carries no credentials and survives whole
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(HOST=h)(PORT=1521)))"),
+ "jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(HOST=h)(PORT=1521)))");
+ // a first host with nothing in front of the comma keeps the comma: what is left is the
+ // hosts of a url, not one host of it
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://,h2:3306/db"), "jdbc:mysql://,h2:3306/db");
+ // a password under a name of its own, and one numbered by the factor it belongs to
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://(host=h,user=u,password2=secret)/db"),
+ "jdbc:mysql://(host=h,user=u,password2=***)/db");
+
+ // a url of Connector/J gives every host of it credentials of its own, and every one of
+ // them goes: the second used to stay in the message with the password of the failover host
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://u:p@h1:3306,u2:p2@h2:3306/db"),
+ "jdbc:mysql://h1:3306,h2:3306/db");
+ // ... including a url whose subprotocol names the kind of connection in front of the hosts
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql:replication://master:p1@h1:3306,slave:p2@h2:3306/db"),
+ "jdbc:mysql:replication://h1:3306,h2:3306/db");
+ // the key-value host syntax of Connector/J puts the credentials inside the authority, where
+ // neither the userinfo nor the parameters of a url stand
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://address=(host=h)(port=3306)(user=u)(password=secret)/db"),
+ "jdbc:mysql://address=(host=h)(port=3306)(user=u)(password=***)/db");
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://(host=h,port=3306,user=u,password=secret)/db"),
+ "jdbc:mysql://(host=h,port=3306,user=u,password=***)/db");
+ assertEquals(CachedConnection.safeUrl("jdbc:sqlserver://h:1433;databaseName=db;PWD=secret"),
+ "jdbc:sqlserver://h:1433;databaseName=db");
+
+ // a shape none of this took apart is not logged past its subprotocol: a password holding a
+ // "/" ends the authority in front of the "@" that would have given the credentials away
+ assertEquals(CachedConnection.safeUrl("jdbc:mysql://u:pa/ss@h:3306/db"),
+ "jdbc:mysql:" + CachedConnection.CREDENTIALS_HIDDEN);
+ // ... and so does a password that holds an "@" and was quoted for the driver
+ assertEquals(CachedConnection.safeUrl("jdbc:oracle:thin:scott/\"pa@ss\"@//h:1521/svc"),
+ "jdbc:oracle:" + CachedConnection.CREDENTIALS_HIDDEN);
+ // a string that is no connection string of any driver carries nothing to the log either
+ assertEquals(CachedConnection.safeUrl("h:5432/db?password=secret"), CachedConnection.CREDENTIALS_HIDDEN);
+ }
+
+ /**
+ * A driver is free to quote the connection string it was handed back into the message of its
+ * failure, and that message travels: RootContainer makes it the message of what it throws,
+ * BackendConfigManager logs it at ERROR and answers a config change with it. So the message is
+ * redacted rather than the two call sites that happen to log a url.
+ */
+ @Test
+ public void testAMessageOfADriverCarriesNoCredentials() throws Exception {
+ final String url = "jdbc:postgresql://opendj:S3cret@h:5432/db";
+ assertEquals(CachedConnection.redact("No suitable driver found for " + url, url),
+ "No suitable driver found for jdbc:postgresql://h:5432/db");
+ // a driver naming the credentials alone, without the url around them
+ assertEquals(CachedConnection.redact("authentication of opendj:S3cret failed", url),
+ "authentication of " + CachedConnection.CREDENTIALS_HIDDEN + " failed");
+ // ... and naming the password alone
+ assertEquals(CachedConnection.redact("the password S3cret was not accepted", url),
+ "the password " + CachedConnection.CREDENTIALS_HIDDEN + " was not accepted");
+ // the credentials of an oracle url stand in front of its descriptor
+ final String oracle = "jdbc:oracle:thin:scott/S3cret@//h:1521/svc";
+ assertEquals(CachedConnection.redact("IO Error connecting to " + oracle, oracle),
+ "IO Error connecting to jdbc:oracle:thin:@//h:1521/svc");
+ // a password of a parameter is blanked wherever the message carries it
+ assertEquals(CachedConnection.redact("bad url jdbc:sqlserver://h:1433;password=S3cret",
+ "jdbc:sqlserver://h:1433;password=S3cret"), "bad url jdbc:sqlserver://h:1433");
+ // a message naming nothing of the connection string is left as it stands
+ assertEquals(CachedConnection.redact("Connection to h:5432 refused", url), "Connection to h:5432 refused");
+ assertNull(CachedConnection.redact(null, url));
+
+ // the stall report is the other way a driver's message reaches the log, and it carries the
+ // url of the backend alongside it
+ final String stall = CachedConnection.stallMessage(url, 3, 4000,
+ new SQLException("FATAL: too many connections for " + url));
+ assertFalse(stall.contains("S3cret"), stall);
+ assertTrue(stall.contains("jdbc:postgresql://h:5432/db"), stall);
+ assertTrue(stall.contains("4000 ms") && stall.contains("(3 attempts)"), stall);
+ }
+
+ /**
+ * A password is free to be one character long, and a bare one of those stands inside half the
+ * lines a driver writes. Replaced wherever it is found, it takes the diagnostic apart along
+ * with the credential - and, since the walk looking for credentials asks the redaction whether
+ * it changed anything, it makes every failure of that backend one whose chain is rebuilt.
+ */
+ @Test
+ public void testAShortPasswordDoesNotRewriteTheMessageOfADriver() throws Exception {
+ final String url = "jdbc:oracle:thin:opendj/1@//h:1521/svc";
+ // the "1" of an ORA number is part of a number, not a credential of anybody
+ assertEquals(CachedConnection.redact("ORA-12541: TNS:no listener", url), "ORA-12541: TNS:no listener");
+ // ... while the same password quoted back on its own is still taken out
+ assertEquals(CachedConnection.redact("the password 1 was not accepted", url),
+ "the password " + CachedConnection.CREDENTIALS_HIDDEN + " was not accepted");
+ assertEquals(CachedConnection.redact("IO Error connecting to " + url, url),
+ "IO Error connecting to jdbc:oracle:thin:@//h:1521/svc");
+ }
+
+ /**
+ * A driver is free to name a parameter in the middle of a sentence. The value of one ends at
+ * the first space: reaching to the end of the string, it would take the host, the port and the
+ * cause of the failure into the blank along with the password - the very information the
+ * redaction of a connection string goes out of its way to keep.
+ */
+ @Test
+ public void testAPasswordParameterDoesNotSwallowTheRestOfTheMessage() throws Exception {
+ final String url = "jdbc:postgresql://h:5432/db?user=u&password=hunter2";
+ assertEquals(CachedConnection.redact("Connection refused: password=hunter2 for user u at h:5432", url),
+ "Connection refused: password=*** for user u at h:5432");
+ }
+
+ /**
+ * The credentials of an oracle url are separated by a "/", so a password holding a "//" of
+ * its own used to start an authority inside itself: what was taken off as a userinfo was the
+ * tail of the password, the "@" the last guard of safeUrl() looks for went with it, and the
+ * user name and the head of the password stayed in the message of a stall.
+ */
+ @Test
+ public void testAPasswordHoldingASlashPairIsNotLeftInTheLog() throws Exception {
+ final String easyConnect = "jdbc:oracle:thin:opendj/pa//ss@//h:1521/svc";
+ assertEquals(CachedConnection.safeUrl(easyConnect), "jdbc:oracle:thin:@//h:1521/svc");
+ // ... and the same password in front of a host that names no "//" of its own
+ final String sid = "jdbc:oracle:thin:opendj/pa//ss@h:1521:svc";
+ assertEquals(CachedConnection.safeUrl(sid), "jdbc:oracle:thin:@h:1521:svc");
+ // the message of a driver quoting the url back carries no more of it than the log does
+ assertEquals(CachedConnection.redact("IO Error connecting to " + easyConnect, easyConnect),
+ "IO Error connecting to jdbc:oracle:thin:@//h:1521/svc");
+ }
+
+ /**
+ * pgjdbc enforces loginTimeout out of process: Driver.connect hands the login to a daemon
+ * thread of its own and gives up on the thread rather than on the login. Against the database
+ * this bound exists for - one that completes the handshake and then says nothing - an
+ * unbounded read there leaves that thread, and the socket it holds, behind on every borrow;
+ * a few operations a second are enough to run the server out of threads and file descriptors.
+ */
+ @Test(timeOut = 300000)
+ public void testTheLoginThreadOfPostgresDoesNotOutliveTheBorrow() throws Exception {
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, Long.toString(BOUND_SECONDS));
+ // counted as a delta of this borrow rather than as a count of the jvm: three tests of this
+ // file open a pgjdbc login against a socket that never answers, and the order they run in
+ // is not contractual - a thread left by any of them would be reported here
+ final int before = loginThreadsOfPostgres();
+ try (final ServerSocket blackhole = new ServerSocket(0, 50, InetAddress.getLoopbackAddress())) {
+ final String url = "jdbc:postgresql://127.0.0.1:" + blackhole.getLocalPort() + "/opendj?user=opendj&password=opendj";
+ try {
+ CachedConnection.getConnection(url);
+ fail("a database that never answers must not hand out a connection");
+ } catch (SQLException expected) {
+ // reported to the caller, as the bound of the attempt promises
+ }
+ final long giveUpAt = System.currentTimeMillis() + BOUND_SECONDS * 1000 + BOUND_MARGIN_MS;
+ while (loginThreadsOfPostgres() > before && System.currentTimeMillis() < giveUpAt) {
+ Thread.sleep(100);
+ }
+ assertTrue(loginThreadsOfPostgres() <= before,
+ "the login thread pgjdbc abandoned outlived the borrow: the read of the login is not bounded");
+ }
+ }
+
+ private static int loginThreadsOfPostgres() {
+ int alive = 0;
+ for (final Thread thread : Thread.getAllStackTraces().keySet()) {
+ if (thread.isAlive() && thread.getName().startsWith("PostgreSQL JDBC driver connection thread")) {
+ alive++;
+ }
+ }
+ return alive;
+ }
+
+ /**
+ * The deadline of the borrow stands for the whole borrow, so it bounds the attempt inside it
+ * even where the per-attempt property gives it no bound of its own: turning that property off
+ * must not turn the bound of the borrow off with it.
+ */
+ @Test(timeOut = 300000)
+ public void testTheDeadlineBoundsAnAttemptTheConnectPropertyDoesNot() throws Exception {
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "0");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "2");
+ try (final ServerSocket blackhole = new ServerSocket(0, 50, InetAddress.getLoopbackAddress())) {
+ final String url = "jdbc:postgresql://127.0.0.1:" + blackhole.getLocalPort() + "/opendj?user=opendj&password=opendj";
+ final long startedAt = System.currentTimeMillis();
+ try {
+ CachedConnection.getConnection(url);
+ fail("a database that never answers must not hand out a connection");
+ } catch (SQLException expected) {
+ // bounded by what is left of the deadline of the borrow
+ }
+ // the wait is the point of this one as much as its end is: a borrow against a socket that
+ // never answers cannot be over before the deadline unless something else ended it
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ assertTrue(elapsed >= 1000, "the borrow was over after " + elapsed + " ms, before its deadline");
+ assertElapsedWithinBound(startedAt, 2000);
+ }
+ }
+
+ /**
+ * The deadline stops the drain of the pool; it does not throw away the connection in hand. A
+ * database at its connection limit has no other source of connections than the ones coming
+ * back to the pool, and closing one unvalidated takes it out of that source for good - while
+ * the borrow that closed it fails with a timeout anyway.
+ */
+ @Test(timeOut = 120000)
+ public void testAPooledConnectionIsNotDiscardedUnvalidatedAtTheDeadline() throws Exception {
+ final String url = StubDriver.PREFIX + "unvalidated-at-deadline";
+ final Connection stale = mock(Connection.class);
+ when(stale.isValid(anyInt())).thenAnswer(invocation -> {
+ Thread.sleep(1500); // a database that no longer answers: the validation waits out its bound
+ return false;
+ });
+ final Connection good = mock(Connection.class);
+ when(good.isValid(anyInt())).thenReturn(true);
+ seedPool(url, stale, good);
+ final Connection fresh = mock(Connection.class);
+ stub.answerWith(fresh);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "1");
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh, "the drain must stop at the deadline");
+ verify(stale).close();
+ verify(good, never()).close();
+ assertFalse(CachedConnection.cached.get(url).isEmpty(), "a connection the deadline was reached in front of was lost");
+ }
+
+ /**
+ * A connection the validation of which failed is on its way out, and its driver knows it:
+ * Connector/J answers a failed validation by aborting the connection and the SQL Server driver
+ * by terminating it. Putting the previous bound back on it fails, and warns about statements
+ * of a connection that is being closed - over an idle connection the server reaped, which is
+ * nobody's problem.
+ */
+ @Test(timeOut = 120000)
+ public void testAConnectionOnItsWayOutIsNotGivenItsBoundBack() throws Exception {
+ final String url = StubDriver.PREFIX + "reaped-idle";
+ final Connection reaped = mock(Connection.class);
+ when(reaped.getNetworkTimeout()).thenReturn(0);
+ when(reaped.isValid(anyInt())).thenReturn(false);
+ seedPool(url, reaped);
+ final Connection fresh = mock(Connection.class);
+ stub.answerWith(fresh);
+
+ assertSame(((CachedConnection) CachedConnection.getConnection(url)).parent, fresh);
+
+ verify(reaped).setNetworkTimeout(any(Executor.class), eq(CachedConnection.VALIDATION_TIMEOUT_SECONDS * 1000));
+ verify(reaped, never()).setNetworkTimeout(any(Executor.class), eq(0));
+ verify(reaped).close();
+ }
+
+ /**
+ * The read bound of the login is lifted once the login is through, because left in place it
+ * fails every statement slower than it. A driver that will not take it back leaves a
+ * connection that must not be pooled: it would carry that bound into every borrow the pool
+ * hands it to, an import batch among them.
+ */
+ @Test(timeOut = 120000)
+ public void testAConnectionStillCarryingTheBoundOfItsLoginIsNotPooled() throws Exception {
+ final String url = StubDriver.PREFIX + "unliftable-bound";
+ final Connection parent = mock(Connection.class);
+ doThrow(new SQLException("setNetworkTimeout is not supported"))
+ .when(parent).setNetworkTimeout(any(Executor.class), eq(0));
+ stub.answerWith(parent);
+
+ final CachedConnection borrowed = CachedConnection.connect(url, CachedConnection.ConnectDialect.MYSQL, 30);
+ borrowed.close();
+
+ verify(parent).close();
+ assertTrue(CachedConnection.cached.get(url).isEmpty(),
+ "a connection still carrying the read bound of its login went back into the pool");
+ }
+
+ /**
+ * The case the window exists for: the connection this borrow takes out answered the database a
+ * moment ago, and asking it again costs the round trip the operation came to make.
+ */
+ @Test(timeOut = 120000)
+ public void testAConnectionProvenAliveIsNotValidatedAgainWithinTheWindow() throws Exception {
+ final String url = StubDriver.PREFIX + "within-the-window";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+
+ final Connection first = CachedConnection.getConnection(url); // established: it has just answered
+ first.close();
+ final Connection second = CachedConnection.getConnection(url);
+
+ assertSame(second, first, "the pooled connection was not the one handed back");
+ assertEquals(stub.attempts.get(), 1, "the pool established a second connection");
+ verify(parent, never()).isValid(anyInt());
+ }
+
+ /** Past the window it is the connection the database or a firewall may have dropped meanwhile. */
+ @Test(timeOut = 120000)
+ public void testAConnectionIsValidatedAgainOnceTheWindowHasPassed() throws Exception {
+ final String url = StubDriver.PREFIX + "past-the-window";
+ CachedConnection.aliveBypassNanos = TimeUnit.MILLISECONDS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+
+ final Connection first = CachedConnection.getConnection(url);
+ first.close();
+ Thread.sleep(20);
+ final Connection second = CachedConnection.getConnection(url);
+
+ assertSame(second, first);
+ verify(parent).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+
+ /** The window switched off validates every borrow, the way this pool did before it existed. */
+ @Test(timeOut = 120000)
+ public void testAWindowOfZeroValidatesEveryBorrow() throws Exception {
+ final String url = StubDriver.PREFIX + "window-of-zero";
+ CachedConnection.aliveBypassNanos = 0;
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+
+ final Connection first = CachedConnection.getConnection(url);
+ first.close();
+ final Connection second = CachedConnection.getConnection(url);
+
+ assertSame(second, first);
+ verify(parent).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+
+ /**
+ * A connection is trusted for the window that follows the last answer it gave, never for the
+ * window that follows its return to the pool. pgjdbc short-circuits both rollback() and
+ * commit() when the transaction state is IDLE, so a borrow that issued no statement - the open
+ * of a backend, a configuration change that leaves the base DNs alone, an import of nothing -
+ * puts a connection back without a byte reaching the server: stamping the return would mark a
+ * connection the database dropped meanwhile as the freshest one in the pool.
+ */
+ @Test(timeOut = 120000)
+ public void testTheReturnToThePoolIsNotTakenForProofOfLife() throws Exception {
+ final String url = StubDriver.PREFIX + "silent-return";
+ CachedConnection.aliveBypassNanos = TimeUnit.MILLISECONDS.toNanos(50);
+ final Connection dropped = mock(Connection.class);
+ when(dropped.isValid(anyInt())).thenReturn(false); // dropped while it was out of the pool
+ stub.answerWith(dropped);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+ Thread.sleep(80); // the answer of the login ages out of the window
+ borrowed.close(); // and the rollback of this return never leaves the driver
+ final Connection fresh = mock(Connection.class);
+ when(fresh.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(fresh);
+
+ final Connection next = CachedConnection.getConnection(url);
+
+ verify(dropped).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ verify(dropped).close();
+ assertSame(((CachedConnection) next).parent, fresh,
+ "a connection the database dropped was handed out on the strength of its return to the pool");
+ }
+
+ /**
+ * A proof taken while the database was going away does not outlive the distrust that reported it.
+ * <p>
+ * The stamp stands for the moment the connection was asked, not the moment its answer was filed:
+ * a validation is given {@link CachedConnection#VALIDATION_TIMEOUT_SECONDS}, and one that started
+ * before another operation reported a drop and returned after it would otherwise be the younger of
+ * the two. The connection would then be handed out unvalidated for the rest of the window - and
+ * LIFO puts it at the head of the deque, so it is the very one the next borrow takes - by the
+ * check that exists to stop exactly that.
+ */
+ @Test(timeOut = 120000)
+ public void testAProofTakenWhileTheDatabaseWentAwayIsNotTrusted() throws Exception {
+ final String url = StubDriver.PREFIX + "proof-across-a-drop";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1); // nothing here ages out of the window
+ final AtomicInteger validations = new AtomicInteger();
+ final Connection parent = mock(Connection.class);
+ // the database goes away while this validation is in flight: another operation of the backend
+ // reports the drop of its own connection before this one has answered
+ when(parent.isValid(anyInt())).thenAnswer(invocation -> {
+ CachedConnection.distrustPool(url);
+ validations.incrementAndGet();
+ return true;
+ });
+ stub.answerWith(parent);
+
+ CachedConnection.getConnection(url).close(); // established and returned, proven by its login
+ CachedConnection.distrustPool(url); // an operation reports a drop: what the pool holds predates it
+ CachedConnection.getConnection(url).close(); // validated, and a second drop is reported while it is
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertEquals(validations.get(), 2,
+ "a connection was trusted on a proof that started before the drop it is compared against");
+ assertSame(((CachedConnection) borrowed).parent, parent, "the connection answered and was still discarded");
+ }
+
+ /**
+ * The pool hands out the connection returned last. Without it the window would rarely apply: a
+ * connection reached only after a whole cycle of the pool has been idle far longer than it.
+ */
+ @Test(timeOut = 120000)
+ public void testTheConnectionReturnedLastIsBorrowedFirst() throws Exception {
+ final String url = StubDriver.PREFIX + "returned-last";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection older = mock(Connection.class);
+ when(older.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(older);
+ final Connection first = CachedConnection.getConnection(url);
+ final Connection newer = mock(Connection.class);
+ when(newer.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(newer);
+ final Connection second = CachedConnection.getConnection(url);
+ assertNotSame(second, first);
+ first.close();
+ second.close();
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, newer, "the pool cycled round to its coldest connection");
+ }
+
+ /**
+ * Whatever dropped one connection - a restart, a failover, a network that went away - dropped
+ * every connection established before it, and a borrow inside the window asks the database
+ * nothing: so the operation that saw the failure tells the pool, and the rest of that
+ * generation is validated once before it is trusted again.
+ */
+ @Test(timeOut = 120000)
+ public void testThePoolIsValidatedAgainAfterTheDatabaseDroppedAConnection() throws Exception {
+ final String url = StubDriver.PREFIX + "distrusted-generation";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+ CachedConnection.getConnection(url).close();
+
+ CachedConnection.distrustPool(url);
+ final Connection next = CachedConnection.getConnection(url);
+ next.close();
+ CachedConnection.getConnection(url).close();
+
+ // once for the generation the drop condemned, and not again for the borrow behind it
+ verify(parent, times(1)).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+
+ /**
+ * The whole of the trade the window makes, end to end: a connection the database dropped
+ * inside the window is handed out unvalidated - that is the cost - the operation it broke
+ * reports the drop, and from there the pool validates the generation the drop condemned
+ * instead of handing out the rest of it the same way.
+ */
+ @Test(timeOut = 120000)
+ public void testAConnectionDroppedInsideTheWindowIsHandedOutOnceAndThenValidated() throws Exception {
+ final String url = StubDriver.PREFIX + "dropped-inside-the-window";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+ CachedConnection.getConnection(url).close();
+
+ when(parent.isValid(anyInt())).thenReturn(false); // the database dropped it where it lay
+ final Connection dropped = CachedConnection.getConnection(url);
+ assertSame(((CachedConnection) dropped).parent, parent, "the pooled connection was not the one handed back");
+ verify(parent, never()).isValid(anyInt()); // handed out on the strength of its last answer
+
+ // the statement of the caller is where the drop surfaces, and the caller reports it
+ CachedConnection.distrustPool(url);
+ dropped.close();
+ final Connection fresh = mock(Connection.class);
+ when(fresh.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(fresh);
+
+ final Connection next = CachedConnection.getConnection(url);
+
+ verify(parent).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ verify(parent).close();
+ assertSame(((CachedConnection) next).parent, fresh, "the rest of the generation was handed out unvalidated");
+ }
+
+ /**
+ * Seeds the pool the way {@link CachedConnection#close()} fills it - at the end a borrow takes
+ * from - so that the connection named first here is the one the next borrow gets.
+ */
+ private static void seedPool(String url, Connection... parents) {
+ for (int i = parents.length - 1; i >= 0; i--) {
+ CachedConnection.cached.get(url).addFirst(new CachedConnection(url, parents[i]));
+ }
+ }
+
+ /**
+ * The borrows nothing compensates a dropped connection on - the open of a backend, the removal
+ * of its files, the start of an import - ask for a connection the pool validates whatever the
+ * window says. Each of them is one borrow of a cold path, and the one that opens a backend
+ * issues no statement at all: a connection dropped inside the window would surface there out of
+ * the rollback that releases it, with no statement to replay and nothing to tell the pool.
+ */
+ @Test(timeOut = 120000)
+ public void testTheBorrowsNothingCompensatesAreValidated() throws Exception {
+ final String url = StubDriver.PREFIX + "validated-borrow";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+ CachedConnection.getConnection(url).close();
+
+ final Connection borrowed = CachedConnection.getConnection(url, false);
+
+ assertSame(((CachedConnection) borrowed).parent, parent, "the pooled connection was not the one handed back");
+ verify(parent).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+
+ /**
+ * A connection closed under the borrow is not handed out on the strength of its last answer:
+ * the removal listener of the pool closes every connection it finds in the deque when the pool
+ * expires, and it iterates a weakly consistent view. The validation the window replaces
+ * answered that as well, out of a flag of the driver rather than out of a round trip.
+ */
+ @Test(timeOut = 120000)
+ public void testAConnectionThePoolClosedIsNotHandedOut() throws Exception {
+ final String url = StubDriver.PREFIX + "closed-inside-the-window";
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ final Connection parent = mock(Connection.class);
+ when(parent.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(parent);
+ CachedConnection.getConnection(url).close();
+ // closed where it lay, by the expiry of the pool: a closed connection answers isValid() with
+ // false as well, which is what discards it once the window stops trusting it
+ when(parent.isClosed()).thenReturn(true);
+ when(parent.isValid(anyInt())).thenReturn(false);
+ final Connection fresh = mock(Connection.class);
+ when(fresh.isValid(anyInt())).thenReturn(true);
+ stub.answerWith(fresh);
+
+ final Connection borrowed = CachedConnection.getConnection(url);
+
+ assertSame(((CachedConnection) borrowed).parent, fresh, "a closed connection was handed out on its last answer");
+ }
+
+ /**
+ * The window is clamped twice: to the idle time the pool keeps a connection for, since a window
+ * longer than that is one the pool can never back - the connection it was meant for is gone
+ * before it closes - and to {@link CachedConnection#MAX_ALIVE_BYPASS_MS} behind it, since the
+ * ttl has no upper bound of its own and a value the unit conversion saturates on would leave
+ * every connection of the pool trusted for the life of the server.
+ * <p>
+ * About the value the class settles on at initialization: the field the pool reads is assigned
+ * once, so a ttl set after that changes neither it nor the idle time it was clamped to.
+ */
+ @Test(timeOut = 120000)
+ public void testTheWindowIsClampedToTheIdleTimeOfThePool() {
+ System.setProperty(CachedConnection.TTL_PROPERTY, "15000");
+ System.setProperty(CachedConnection.ALIVE_BYPASS_PROPERTY, "500");
+ assertEquals(CachedConnection.getAliveBypassMillis(), 500L, "a window inside the ttl was not left alone");
+
+ System.setProperty(CachedConnection.ALIVE_BYPASS_PROPERTY, Long.toString(Long.MAX_VALUE));
+ assertEquals(CachedConnection.getAliveBypassMillis(), 15000L, "a window of Long.MAX_VALUE was not clamped");
+
+ System.setProperty(CachedConnection.TTL_PROPERTY, "100");
+ assertEquals(CachedConnection.getAliveBypassMillis(), 100L, "the clamp is the configured ttl, not the default");
+
+ // the ttl has no upper bound of its own, so the clamp to it does not bound the window either: both set
+ // to a value the conversion to nanoseconds saturates on would leave every connection of the pool
+ // trusted for the life of the server, which is the outcome this javadoc says the clamp rules out
+ System.setProperty(CachedConnection.TTL_PROPERTY, Long.toString(Long.MAX_VALUE));
+ System.setProperty(CachedConnection.ALIVE_BYPASS_PROPERTY, Long.toString(Long.MAX_VALUE));
+ assertEquals(CachedConnection.getAliveBypassMillis(), CachedConnection.MAX_ALIVE_BYPASS_MS,
+ "a window the ttl did not bound was left to saturate");
+ }
+
+ /**
+ * A setting worth warning about must leave the class usable. Both properties are read by the
+ * initializer of {@code aliveBypassNanos}, and both report an unusable value through the set of
+ * what has been said once already - which, declared below that initializer, would still be null
+ * when the initializer reaches it (JLS 12.4.2). A window longer than the ttl is exactly the
+ * tuning the javadoc of the property invites, and it would turn a log line into an
+ * ExceptionInInitializerError on the first borrow and a causeless NoClassDefFoundError on every
+ * one after it: no connection can be borrowed, so the backend cannot open at all.
+ * <p>
+ * Asserted on the class loaded afresh rather than on this one, which was initialized long
+ * before the property was set.
+ */
+ @Test(timeOut = 120000, dataProvider = "settingsWorthWarningAbout")
+ public void testASettingWorthWarningAboutStillInitializesTheClass(String ttl, String window, long expectedMillis)
+ throws Exception {
+ System.setProperty(CachedConnection.TTL_PROPERTY, ttl);
+ System.setProperty(CachedConnection.ALIVE_BYPASS_PROPERTY, window);
+
+ final Class<?> reloaded = loadedAfresh(CachedConnection.class);
+
+ assertNotSame(reloaded, CachedConnection.class, "the class under test was not loaded afresh");
+ final Field field = reloaded.getDeclaredField("aliveBypassNanos");
+ field.setAccessible(true);
+ assertEquals(field.getLong(null), TimeUnit.MILLISECONDS.toNanos(expectedMillis),
+ "the window the reloaded class settled on");
+ }
+
+ @DataProvider
+ public Object[][] settingsWorthWarningAbout() {
+ return new Object[][]{
+ // a window longer than the ttl: reported once and used as the ttl
+ {"15000", "60000", 15000L},
+ // not a number, and negative: reported once and ignored in favour of the default
+ {"15000", "half a second", CachedConnection.DEFAULT_ALIVE_BYPASS_MS},
+ {"15000", "-1", CachedConnection.DEFAULT_ALIVE_BYPASS_MS},
+ // the ttl is read by the same initializer, and reports its own value the same way
+ {"30s", "500", 500L}
+ };
+ }
+
+ /**
+ * The class again, defined by a loader of this test rather than taken from the one that has
+ * already initialized it: a static initializer runs once per loader, and this is about what it
+ * does. Every other class is delegated to the parent, so the reloaded one shares the types it
+ * is written against.
+ */
+ private static Class<?> loadedAfresh(Class<?> type) throws Exception {
+ final String name = type.getName();
+ final ClassLoader parent = type.getClassLoader();
+ final ClassLoader loader = new ClassLoader(parent) {
+ @Override
+ protected Class<?> loadClass(String candidate, boolean resolve) throws ClassNotFoundException {
+ if (!name.equals(candidate)) {
+ return super.loadClass(candidate, resolve);
+ }
+ Class<?> defined = findLoadedClass(candidate);
+ if (defined == null) {
+ final byte[] bytecode = bytecodeOf(candidate, parent);
+ defined = defineClass(candidate, bytecode, 0, bytecode.length);
+ }
+ if (resolve) {
+ resolveClass(defined);
+ }
+ return defined;
+ }
+ };
+ return Class.forName(name, true, loader);
+ }
+
+ private static byte[] bytecodeOf(String name, ClassLoader from) throws ClassNotFoundException {
+ try (final InputStream in = from.getResourceAsStream(name.replace('.', '/') + ".class")) {
+ if (in == null) {
+ throw new ClassNotFoundException(name);
+ }
+ final ByteArrayOutputStream bytecode = new ByteArrayOutputStream();
+ final byte[] chunk = new byte[8192];
+ for (int read; (read = in.read(chunk)) >= 0; ) {
+ bytecode.write(chunk, 0, read);
+ }
+ return bytecode.toByteArray();
+ } catch (IOException e) {
+ throw new ClassNotFoundException(name, e);
+ }
+ }
+
+ private static SQLException tooManyConnections() {
+ // 53300, too_many_connections, of the insufficient_resources class
+ return new SQLException("sorry, too many clients already", "53300");
+ }
+
+ /** A connection string of every dialect pointing at one host and port. */
+ private static String[] urlsOf(int port) {
+ return new String[]{
+ "jdbc:postgresql://127.0.0.1:" + port + "/opendj?user=opendj&password=opendj",
+ "jdbc:mysql://127.0.0.1:" + port + "/opendj?user=opendj&password=opendj",
+ "jdbc:oracle:thin:opendj/opendj@//127.0.0.1:" + port + "/free",
+ "jdbc:sqlserver://127.0.0.1:" + port + ";databaseName=opendj;user=opendj;password=opendj;encrypt=false"
+ };
+ }
+
+ private static int closedPort() throws Exception {
+ try (final ServerSocket socket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) {
+ return socket.getLocalPort();
+ } // closed again: nothing listens there any more
+ }
+
+ /**
+ * A socket that answers a connect and closes it at once. A port bound and released is the
+ * shape of a refused connect a test would reach for, but it races whatever else on the host
+ * may take that port; this one is the failure it stands for and belongs to nobody else.
+ */
+ private static ServerSocket rejectingSocket() throws Exception {
+ final ServerSocket socket = new ServerSocket(0, 50, InetAddress.getLoopbackAddress());
+ final Thread accepting = new Thread(() -> {
+ while (!socket.isClosed()) {
+ try {
+ socket.accept().close();
+ } catch (Exception closed) {
+ return;
+ }
+ }
+ }, "opendj-test-rejecting-socket");
+ accepting.setDaemon(true);
+ accepting.start();
+ return socket;
+ }
+
+ // The margin grows with the bound instead of dwarfing it: what this has to catch is a bound
+ // that is not in force, and a bound of 2 s that is not in force is not a wait of 12 s - it is
+ // the 30 s of the connect property, the 600 s of a driver, or no end at all.
+ private static void assertElapsedWithinBound(long startedAt, long boundMs) {
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ final long margin = Math.max(BOUND_MARGIN_MS, boundMs);
+ assertTrue(elapsed < boundMs + margin,
+ "gave up only after " + elapsed + " ms, past the " + boundMs + " ms it was bounded by");
+ }
+
+ /**
+ * Stands in for a database whose answer to a connect is the point of the test: the vendor
+ * codes and SQL states below are what the retry has to tell apart, and no engine is needed to
+ * produce them.
+ */
+ private static final class StubDriver implements Driver {
+ static final String PREFIX = "jdbc:opendj-stub:";
+ static final int ALWAYS = -1;
+
+ final AtomicInteger attempts = new AtomicInteger();
+ private volatile SQLException failure;
+ private volatile int failuresLeft;
+ private volatile Connection answer;
+
+ void failWith(SQLException failure, int times) {
+ this.failure = failure;
+ this.failuresLeft = times;
+ this.answer = null;
+ this.attempts.set(0);
+ }
+
+ void answerWith(Connection answer) {
+ this.failure = null;
+ this.failuresLeft = 0;
+ this.answer = answer;
+ this.attempts.set(0);
+ }
+
+ @Override
+ public Connection connect(String url, Properties info) throws SQLException {
+ if (!acceptsURL(url)) {
+ return null;
+ }
+ attempts.incrementAndGet();
+ if (failuresLeft != 0) {
+ if (failuresLeft > 0) {
+ failuresLeft--;
+ }
+ throw failure;
+ }
+ if (answer != null) {
+ return answer;
+ }
+ final Connection con = mock(Connection.class);
+ when(con.isValid(anyInt())).thenReturn(true);
+ return con;
+ }
+
+ @Override
+ public boolean acceptsURL(String url) {
+ return url != null && url.startsWith(PREFIX);
+ }
+
+ @Override
+ public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) {
+ return new DriverPropertyInfo[0];
+ }
+
+ @Override
+ public int getMajorVersion() {
+ return 1;
+ }
+
+ @Override
+ public int getMinorVersion() {
+ return 0;
+ }
+
+ @Override
+ public boolean jdbcCompliant() {
+ return false;
+ }
+
+ @Override
+ public Logger getParentLogger() {
+ return Logger.getLogger(StubDriver.class.getName());
+ }
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
index 93d33f2..487f36d 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
@@ -20,34 +20,64 @@
import org.opends.server.backends.jdbc.JDBCStorage.Conflict;
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.StorageRuntimeException;
+import org.opends.server.backends.pluggable.spi.TreeName;
import org.opends.server.backends.pluggable.spi.WriteOperation;
import org.opends.server.backends.pluggable.spi.WriteableTransaction;
import org.opends.server.types.DirectoryException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.DriverPropertyInfo;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
import java.sql.SQLException;
+import java.sql.SQLNonTransientConnectionException;
+import java.sql.SQLRecoverableException;
+import java.sql.Statement;
+import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Logger;
import static org.forgerock.i18n.LocalizableMessage.raw;
import static org.forgerock.opendj.ldap.ResultCode.OTHER;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyBoolean;
+import static org.mockito.Mockito.anyInt;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.startsWith;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.AFTER_LOCK_WAIT;
import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.NONE;
import static org.opends.server.backends.jdbc.JDBCStorage.Conflict.PROMPT;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
/**
- * Tests how a failure is classified as a transaction conflict, which is what decides whether
- * {@link JDBCStorage#write} replays the operation and whether its first replay is granted regardless of the
- * clock, how long the replays may go on for, and how long it waits before each of them.
+ * Tests how a failure is classified - as a transaction conflict of one class or another, or as a connection the
+ * database dropped - which is what decides whether {@link JDBCStorage#write} replays the operation, whether its
+ * first replay is granted regardless of the clock, how long the replays may go on for and how long it waits
+ * before each of them; and what {@code write()} itself does with that verdict, replay and pool alike.
* <p>
* Runs without a database: the failures the drivers report are reproduced as synthetic
- * {@link SQLException}s carrying the same vendor error number and SQLState.
+ * {@link SQLException}s carrying the same vendor error number and SQLState, and the writes that carry them run
+ * against mocked connections handed out by a driver of this test.
*/
@Test(sequential = true)
@SuppressWarnings("javadoc")
@@ -59,6 +89,35 @@
private static final String ORACLE = "oracle.jdbc.driver.T4CConnection";
private static final String POSTGRES = "org.postgresql.jdbc.PgConnection";
+ /** The tree every write of this test opens; the table name behind it is a hash of this name. */
+ private static final TreeName TREE = new TreeName("dc=example,dc=com", "id2entry");
+
+ private final StubDriver stub = new StubDriver();
+
+ /** Every test gets a pool of its own: the pools and the distrust of a pool are keyed by the url. */
+ private final AtomicInteger pools = new AtomicInteger();
+
+ /** The statement a create of a test runs, so that a test can assert that it ran - or that it did not. */
+ private PreparedStatement statements;
+
+ /** The connection behind the pool of a test, so that a test can assert the statement it was asked to prepare. */
+ private Connection engineConnection;
+
+ /**
+ * Connections whose class names carry the engine the way the drivers' own do - pgjdbc's
+ * {@code org.postgresql.jdbc.PgConnection}, Connector/J's {@code com.mysql.cj.jdbc.ConnectionImpl}. That name
+ * is what {@code driverNameOf()} matches an engine on, and the name of a mock is derived from the type it
+ * mocks, so a mock of plain {@link Connection} reaches no engine branch of {@code openTree()} at all. Lowercase
+ * because the match is case sensitive.
+ */
+ interface postgresConnection extends Connection
+ {
+ }
+
+ interface mysqlConnection extends Connection
+ {
+ }
+
/** A failure whose cause chain is a cycle, to check that walking it terminates. */
private static final class SelfCausedException extends RuntimeException
{
@@ -205,6 +264,155 @@
assertEquals(JDBCStorage.replayable(attempt, elapsedNanos, failure, driver), expected, name);
}
+ @DataProvider
+ public Object[][] connectionFailures()
+ {
+ return new Object[][] {
+ // class 08, connection exception: pgjdbc reports the next use of a connection the server dropped as 08003,
+ // and a socket that failed under it as 08006, while a connect that never came up is 08001
+ { "connection does not exist", sql(0, "08003"), true },
+ { "connection failure", sql(0, "08006"), true },
+ { "unable to establish connection", sql(0, "08001"), true },
+ // the FATAL message a pg_terminate_backend or a shutdown sends before the socket closes: the connection is
+ // gone, and only its next use would be reported as class 08
+ { "admin shutdown", sql(0, "57P01"), true },
+ { "crash shutdown", sql(0, "57P02"), true },
+ { "cannot connect now", sql(0, "57P03"), true },
+ // it reaches write() wrapped, exactly as a conflict does
+ { "wrapped once", new StorageRuntimeException(sql(0, "08006")), true },
+ { "wrapped twice",
+ new DirectoryException(OTHER, raw("unchecked"), new StorageRuntimeException(sql(0, "08003"))), true },
+ { "wrapped 57P0x", new StorageRuntimeException(sql(0, "57P01")), true },
+ // the types the JDBC contract gives a driver to say the connection is gone, whatever state it fills in:
+ // oracle reports ORA-03113 and ORA-01089 as SQLRecoverableException, and only happens to map them to 08006
+ { "recoverable", new SQLRecoverableException("closed connection", "72000", 3113), true },
+ { "non transient connection", new SQLNonTransientConnectionException("socket closed", "S1000", 0), true },
+ // a driver reports what happened as the next exception of a generic failure as readily as it reports it
+ // as the cause, and mssql-jdbc chains every error of a message it received that way
+ { "next exception", chained(sql(0, "HY000"), sql(0, "08006")), true },
+ // the drop of the rollback that releases a connection arrives suppressed into the failure of the operation
+ { "suppressed", suppressing(sql(2627, "23000"), sql(0, "08006")), true },
+
+ // a statement the database answered, however badly, leaves the connection usable
+ { "deadlock victim", sql(1205, "40001"), false },
+ { "primary key violation", sql(2627, "23000"), false },
+ // 53300 is the server refusing a further connection, not the loss of one already established
+ { "too many connections", sql(0, "53300"), false },
+ // a killed session on SQL Server: generateStateCode maps neither 596 nor its siblings, so with xopenStates
+ // off - the default - it arrives as "S"+errorState and no state tells it from a rejected statement. What
+ // does tell it apart is the connection the driver closed behind it, see the test below
+ { "mssql killed session", sql(596, "S0001"), false },
+ { "no SQLState", sql(0, null), false },
+ { "not a SQLException", new IllegalStateException("connection closed"), false },
+ { "no failure at all", null, false },
+ { "cyclic cause chain", new SelfCausedException(), false },
+ };
+ }
+
+ @Test(dataProvider = "connectionFailures")
+ public void testIsConnectionFailure(String name, Throwable failure, boolean expected)
+ {
+ assertEquals(JDBCStorage.isConnectionFailure(failure), expected, name);
+ }
+
+ /**
+ * A connection the database dropped is replayed on a connection the next attempt borrows of its own - but only
+ * while the transaction has not been committed yet. A drop reported by {@code commit()} leaves the outcome of
+ * the transaction unknown, and replaying a write that in fact committed applies it twice.
+ */
+ @Test
+ public void testADroppedConnectionIsReplayedOnlyBeforeTheCommit()
+ {
+ final SQLException dropped = sql(0, "08006");
+ assertEquals(JDBCStorage.replayReason(dropped, POSTGRES, false, false, false),
+ "a connection the database dropped");
+ assertNull(JDBCStorage.replayReason(dropped, POSTGRES, true, false, false),
+ "an in doubt transaction was replayed");
+ }
+
+ /**
+ * A driver that closed the connection has said the connection is gone whatever SQLState it filled in - which is
+ * the only way a killed SQL Server session is ever recognized, since it arrives as S0001.
+ */
+ @Test
+ public void testAConnectionTheDriverClosedIsADroppedOne()
+ {
+ final SQLException killed = sql(596, "S0001");
+ assertNull(JDBCStorage.replayReason(killed, MSSQL, false, false, false), "S0001 was replayed on its own");
+ assertEquals(JDBCStorage.replayReason(killed, MSSQL, false, false, true),
+ "a connection the database dropped");
+ assertNull(JDBCStorage.replayReason(killed, MSSQL, true, false, true),
+ "an in doubt transaction was replayed");
+ }
+
+ /**
+ * An attempt that committed part of its own work is not replayed, whatever the failure says: what it did no
+ * longer rolls back as a whole, and a WriteOperation is only idempotent in the database. RootContainer.open
+ * opens and registers the entry containers of every base DN in one write, and a replay of it fails with
+ * ERR_ENTRY_CONTAINER_ALREADY_REGISTERED, masking the failure that caused the replay.
+ */
+ @Test
+ public void testAnAttemptThatCommittedPartOfItsWorkIsNotReplayed()
+ {
+ assertNull(JDBCStorage.replayReason(sql(0, "40001"), POSTGRES, false, true, false), "a conflict was replayed");
+ assertNull(JDBCStorage.replayReason(sql(0, "08006"), POSTGRES, false, true, false), "a drop was replayed");
+ assertNull(JDBCStorage.replayReason(sql(596, "S0001"), MSSQL, false, true, true), "a drop was replayed");
+ }
+
+ /**
+ * A conflict is replayed on the strength of the engine having rolled the transaction back before it answered,
+ * so it is read from the failure of the operation and not from the release of the connection: a class 40 the
+ * release contributed - Oracle reports a transaction rolled back under it as ORA-02091, SQLState 40000 - would
+ * otherwise re-authorise the replay of a commit whose outcome is unknown, past the guard that exists for it.
+ * A drop is read from the release as well, which is the one place it is often stated at all.
+ */
+ @Test
+ public void testAConflictIsNotReadFromTheReleaseOfTheConnection()
+ {
+ final SQLException onRelease = suppressing(sql(2627, "23000"), sql(0, "40000"));
+ assertFalse(JDBCStorage.isRetryableConflict(onRelease, POSTGRES), "a conflict was read from the release");
+ assertNull(JDBCStorage.replayReason(onRelease, POSTGRES, true, false, false),
+ "a transaction the commit left in doubt was replayed");
+
+ // the same shape carrying a drop instead: read, since the release is where a drop is stated at all
+ assertTrue(JDBCStorage.isConnectionFailure(suppressing(sql(2627, "23000"), sql(0, "08006"))),
+ "a drop was not read from the release");
+ }
+
+ /** The connection is asked only where a state does not already say the connection is gone. */
+ @Test
+ public void testTheConnectionIsAskedWhetherTheDriverClosedIt() throws Exception
+ {
+ final Connection closed = mock(Connection.class);
+ when(closed.isClosed()).thenReturn(true);
+ final Connection alive = mock(Connection.class);
+ when(alive.isClosed()).thenReturn(false);
+ final Connection mute = mock(Connection.class);
+ when(mute.isClosed()).thenThrow(new SQLException("the connection cannot say"));
+
+ assertTrue(JDBCStorage.isConnectionFailure(sql(596, "S0001"), closed), "a killed session was not recognized");
+ assertFalse(JDBCStorage.isConnectionFailure(sql(2627, "23000"), alive), "a rejected statement was a drop");
+ assertTrue(JDBCStorage.isConnectionFailure(sql(0, "08006"), alive), "class 08 needs no connection to say so");
+ assertTrue(JDBCStorage.isConnectionFailure(sql(2627, "23000"), mute), "a connection that cannot answer");
+ }
+
+ /** A conflict is a rollback the engine completed before it answered, whichever phase reported it. */
+ @Test
+ public void testAConflictIsReplayedFromEitherPhase()
+ {
+ final SQLException conflict = sql(0, "40001");
+ assertEquals(JDBCStorage.replayReason(conflict, POSTGRES, false, false, false), "a conflict");
+ assertEquals(JDBCStorage.replayReason(conflict, POSTGRES, true, false, false), "a conflict");
+ }
+
+ /** Everything else fails the operation, as it did before either replay existed. */
+ @Test
+ public void testAFailureOfTheStatementIsNotReplayed()
+ {
+ assertNull(JDBCStorage.replayReason(sql(2627, "23000"), MSSQL, false, false, false));
+ assertNull(JDBCStorage.replayReason(sql(2627, "23000"), MSSQL, true, false, false));
+ }
+
/** The delay grows with the attempt, so that the replays outlast a contention lasting more than a few ms. */
@Test
public void testRetryDelayGrowsAndStaysBounded()
@@ -233,19 +441,545 @@
public void testConflictSummaryNamesTheStateAndTheNumber()
{
final String summary = JDBCStorage.conflictSummary(
- new DirectoryException(OTHER, raw("unchecked"), new StorageRuntimeException(sql(1205, "40001"))));
+ new DirectoryException(OTHER, raw("unchecked"), new StorageRuntimeException(sql(1205, "40001"))), POSTGRES);
assertTrue(summary.contains("40001"), summary);
assertTrue(summary.contains("1205"), summary);
assertTrue(summary.contains("synthetic failure"), summary);
}
+ /**
+ * That line is the only record a replay leaves, so it names the failure the replay was decided on. A write whose
+ * operation was rejected and whose release then reported a drop is replayed on the class 08 suppressed into the
+ * rejection, and naming the state of the rejected statement would describe a replay that did not happen.
+ */
+ @Test
+ public void testConflictSummaryNamesTheFailureTheReplayWasDecidedOn()
+ {
+ final String summary = JDBCStorage.conflictSummary(
+ new StorageRuntimeException(suppressing(sql(2627, "23000"), sql(0, "08006"))), POSTGRES);
+ assertTrue(summary.contains("08006"), summary);
+ assertFalse(summary.contains("23000"), summary);
+ }
+
/** A failure carrying no SQLException at all, and a cyclic cause chain, still have to yield something loggable. */
@Test
public void testConflictSummaryTerminatesWithoutASQLException()
{
- assertTrue(JDBCStorage.conflictSummary(new IllegalStateException("connection closed")).contains("closed"));
- assertTrue(JDBCStorage.conflictSummary(new SelfCausedException()).contains("SelfCausedException"));
- assertEquals(JDBCStorage.conflictSummary(null), "null");
+ assertTrue(JDBCStorage.conflictSummary(new IllegalStateException("connection closed"), POSTGRES).contains("closed"));
+ assertTrue(JDBCStorage.conflictSummary(new SelfCausedException(), POSTGRES).contains("SelfCausedException"));
+ assertEquals(JDBCStorage.conflictSummary(null, POSTGRES), "null");
+ }
+
+ /** A statement that carries neither a conflict nor a drop is still the one the summary names. */
+ @Test
+ public void testConflictSummaryFallsBackToTheFirstFailureOfTheChain()
+ {
+ final String summary = JDBCStorage.conflictSummary(new StorageRuntimeException(sql(2627, "23000")), POSTGRES);
+ assertTrue(summary.contains("23000"), summary);
+
+ // the fallback names the statement, not the rollback of the release behind it: this is where a replay
+ // decided on the closed flag of the connection alone lands - neither chain carries a verdict of its own -
+ // and the walk reaches the suppressed exceptions of a failure before its cause
+ final StorageRuntimeException killedSession = new StorageRuntimeException(sql(596, "S0001"));
+ killedSession.addSuppressed(sql(0, "25P02"));
+ final String decidedOnTheConnection = JDBCStorage.conflictSummary(killedSession, MSSQL);
+ assertTrue(decidedOnTheConnection.contains("S0001"), decidedOnTheConnection);
+ assertFalse(decidedOnTheConnection.contains("25P02"), decidedOnTheConnection);
+ }
+
+ /**
+ * The walk of a failure looks at a bounded number of links: mssql-jdbc chains every error of one message it
+ * received through {@code setNextException}, and a budget spent on those would never reach the cause a wrapper
+ * carries. Pinned from both sides - a drop on the last link of the budget is found and one link further is not
+ * - since a number nothing pins drifts unnoticed in either direction.
+ */
+ @Test
+ public void testTheWalkOfAFailureStopsAtItsBudget()
+ {
+ assertTrue(JDBCStorage.isConnectionFailure(chainEndingInADrop(64)), "a drop on the last link of the budget");
+ assertFalse(JDBCStorage.isConnectionFailure(chainEndingInADrop(65)), "a drop past the budget was walked to");
+ }
+
+ /**
+ * Opening a tree that is already there commits nothing, so the attempt stays replayable. The create table is
+ * guarded by a catalog read, and so is the create index on every engine but postgresql, so on an existing
+ * backend {@code openTree(name, true)} issues no statement at all - while
+ * {@code RootContainer.open()} opens every tree of every base DN in a single write whose first act is one of
+ * these. A flag raised on the catalog read alone would leave that write unreplayable for the life of the
+ * backend, the conflict replay of #867 included: {@code replayReason()} reads the flag before anything else.
+ */
+ @Test
+ public void testOpeningAnExistingTreeLeavesTheAttemptReplayable() throws Exception
+ {
+ final JDBCStorage storage = storageOverACatalogHolding(true);
+ final AtomicInteger attempts = new AtomicInteger();
+
+ storage.write(txn -> {
+ txn.openTree(TREE, true);
+ if (attempts.incrementAndGet() == 1)
+ {
+ throw new StorageRuntimeException(sql(0, "40001"));
+ }
+ });
+
+ assertEquals(attempts.get(), 2, "a transaction that committed nothing was not replayed");
+ verify(statements, never()).executeUpdate();
+ }
+
+ /**
+ * A tree that had to be created did commit - the create table commits, and mysql and oracle commit before a DDL
+ * statement of their own accord - so the attempt is out of the replay whatever the failure says: a
+ * {@link WriteOperation} is only idempotent in the database, and {@code RootContainer.open()} replayed after the
+ * trees of the first base DN were created registers that base DN a second time.
+ */
+ @Test
+ public void testCreatingATreeTakesTheAttemptOutOfTheReplay() throws Exception
+ {
+ final JDBCStorage storage = storageOverACatalogHolding(false);
+ final AtomicInteger attempts = new AtomicInteger();
+
+ try
+ {
+ storage.write(txn -> {
+ txn.openTree(TREE, true);
+ attempts.incrementAndGet();
+ throw new StorageRuntimeException(sql(0, "40001"));
+ });
+ fail("a transaction that had committed a create table was replayed");
+ }
+ catch (StorageRuntimeException expected)
+ {
+ assertTrue(JDBCStorage.isRetryableConflict(expected, POSTGRES), "the conflict was not the failure raised");
+ }
+ assertEquals(attempts.get(), 1, "a transaction that committed part of its work was replayed");
+ verify(statements).executeUpdate();
+ }
+
+ /**
+ * The rollback that unwinds a failed attempt is often the first place a drop is stated outright, and on a
+ * driver that reports a killed session as a plain vendor error it is the only one. It is joined to the failure
+ * being unwound rather than dropped on the floor, so that the classifiers below read it: without it the attempt
+ * would lean on the driver having flipped its closed flag already, and a driver that has not gives neither the
+ * replay nor the distrust.
+ */
+ @Test
+ public void testTheRollbackOfAFailedAttemptIsNotSwallowed() throws Exception
+ {
+ final long window = CachedConnection.aliveBypassNanos;
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ try
+ {
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ final Connection dropped = mock(Connection.class);
+ when(dropped.isValid(anyInt())).thenReturn(true);
+ when(dropped.isClosed()).thenReturn(false); // the driver has not flipped its flag yet
+
+ final JDBCStorage storage = storageOver(pooled, dropped);
+ final Connection first = storage.getConnection();
+ final Connection second = storage.getConnection();
+ first.close();
+ second.close();
+ // proven alive by the connect itself, and inside the window ever since: nothing has validated
+ verify(dropped, never()).isValid(anyInt());
+
+ // only the rollback that unwinds the attempt says the connection is gone: the release behind it
+ // goes through, so the dropped connection is back at the head of the pool - where the replay
+ // borrows it again - with nothing else to report what it saw
+ doThrow(new SQLException("connection reset", "08006")).doNothing().when(dropped).rollback();
+
+ final AtomicInteger attempts = new AtomicInteger();
+ storage.write(txn -> {
+ if (attempts.incrementAndGet() == 1)
+ {
+ throw new StorageRuntimeException(sql(596, "S0001")); // a killed session, as mssql-jdbc reports it
+ }
+ });
+
+ assertEquals(attempts.get(), 2, "the drop the rollback reported was not replayed");
+ // the distrust reached the pool: the borrow of the replay validated instead of trusting the
+ // last answer of a connection that predates the drop
+ verify(dropped, times(1)).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+ finally
+ {
+ CachedConnection.aliveBypassNanos = window;
+ }
+ }
+
+ /**
+ * A drop the release of the connection reported reaches the pool as well as the replay. It is suppressed into
+ * the failure of the operation (JLS 14.20.3.1) rather than replacing it, so the attempt sees it only on the
+ * chains of that failure - and the pool has no other way of hearing of it: the rest of that generation would
+ * otherwise be handed out unvalidated one by one until the pool runs out of it.
+ */
+ @Test
+ public void testADropReportedByTheReleaseReachesThePool() throws Exception
+ {
+ final long window = CachedConnection.aliveBypassNanos;
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ try
+ {
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ final Connection released = mock(Connection.class);
+ when(released.isValid(anyInt())).thenReturn(true);
+
+ final JDBCStorage storage = storageOver(pooled, released);
+ // both are proven alive and back in the pool; the one released last is the one the write borrows
+ final Connection first = storage.getConnection();
+ final Connection second = storage.getConnection();
+ first.close();
+ second.close();
+
+ // from here the database has dropped the connection at the head of the pool: the operation is
+ // rejected for its own reasons, the rollback that unwinds the attempt goes through, and the release
+ // behind it is where the drop surfaces. Chained, so that the drop lands on the second rollback: an
+ // unchained stub fails the first one - the rollback of the attempt - and pins the sibling test above
+ doNothing().doThrow(new SQLException("connection reset", "08006")).when(released).rollback();
+
+ final AtomicInteger attempts = new AtomicInteger();
+ storage.write(txn -> {
+ if (attempts.incrementAndGet() == 1)
+ {
+ throw new StorageRuntimeException(sql(2627, "23000"));
+ }
+ });
+
+ assertEquals(attempts.get(), 2, "the drop suppressed into the failure was not replayed");
+ // the return to the pool that seeded it, the rollback of the attempt, and the release behind it - which
+ // is the one that reported, since the stub above lets the rollback of the attempt through
+ verify(released, times(3)).rollback();
+ // the distrust reached the pool from the release: the borrow of the replay validated instead of
+ // trusting the last answer of a connection established before the drop
+ verify(pooled).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+ finally
+ {
+ CachedConnection.aliveBypassNanos = window;
+ }
+ }
+
+ /**
+ * A read is never replayed - two of the read operations of this server are not idempotent - but a drop it ran
+ * into still has to reach the pool, which has no other way of hearing of one: a borrow inside the window asks
+ * the database nothing, so the statement that broke is the only place the drop is ever seen. The release of
+ * the connection counts as such a statement: its rollback is the one round trip a read that found nothing
+ * makes.
+ */
+ @Test
+ public void testADropAReadRanIntoReachesThePool() throws Exception
+ {
+ final long window = CachedConnection.aliveBypassNanos;
+ CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
+ try
+ {
+ final Connection pooled = mock(Connection.class);
+ when(pooled.isValid(anyInt())).thenReturn(true);
+ final Connection released = mock(Connection.class);
+ when(released.isValid(anyInt())).thenReturn(true);
+
+ final JDBCStorage storage = storageOver(pooled, released);
+ final Connection first = storage.getConnection();
+ final Connection second = storage.getConnection();
+ first.close();
+ second.close();
+ // both are proven alive and inside the window: nothing has validated
+ verify(released, never()).isValid(anyInt());
+
+ // the read is rejected for its own reasons, and the release behind it - a read issues no rollback of its
+ // own - is where the connection the database dropped says so
+ doThrow(new SQLException("connection reset", "08006")).when(released).rollback();
+ try
+ {
+ storage.read(txn -> {
+ throw new StorageRuntimeException(sql(2627, "23000"));
+ });
+ fail("the failure of the read was swallowed");
+ }
+ catch (StorageRuntimeException expected)
+ {
+ assertTrue(JDBCStorage.isConnectionFailure(expected), "the drop of the release did not reach the failure");
+ }
+
+ storage.getConnection().close(); // the borrow that follows it validates instead of trusting
+
+ verify(pooled).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
+ }
+ finally
+ {
+ CachedConnection.aliveBypassNanos = window;
+ }
+ }
+
+ /**
+ * The create index of the postgres branch is asked of the catalog first, although postgresql has "create index
+ * if not exists": that statement commits whether it creates anything or not, and unguarded it would take every
+ * write that opens a tree out of the replay - {@code RootContainer.open()} and its ~25 trees per suffix
+ * included.
+ */
+ @Test
+ public void testThePostgresIndexIsAskedOfTheCatalogBeforeItIsCreated() throws Exception
+ {
+ final JDBCStorage storage = storageOverAnEngine(postgresConnection.class, true);
+ final AtomicInteger attempts = new AtomicInteger();
+
+ storage.write(txn -> {
+ txn.openTree(TREE, true);
+ if (attempts.incrementAndGet() == 1)
+ {
+ throw new StorageRuntimeException(sql(0, "40001"));
+ }
+ });
+
+ assertEquals(attempts.get(), 2, "a transaction that committed nothing was not replayed");
+ verify(statements, never()).executeUpdate();
+ }
+
+ /**
+ * postgresql runs DDL inside the transaction, so a create index the engine rolled back has committed nothing:
+ * {@code write()} rolls the attempt back whole and replays it. Raising the flag in front of the statement -
+ * which is what mysql and oracle need, since they commit before a DDL of their own accord - would turn a
+ * deadlock the engine itself undid into a hard failure of the open.
+ */
+ @Test
+ public void testACreateIndexPostgresRolledBackLeavesTheAttemptReplayable() throws Exception
+ {
+ final JDBCStorage storage = storageOverAnEngine(postgresConnection.class, false);
+ when(statements.executeUpdate()).thenThrow(sql(0, "40P01")).thenReturn(0);
+ final AtomicInteger attempts = new AtomicInteger();
+
+ storage.write(txn -> {
+ attempts.incrementAndGet();
+ txn.openTree(TREE, true);
+ });
+
+ assertEquals(attempts.get(), 2, "a create index the engine rolled back was not replayed");
+ verify(engineConnection, times(2)).prepareStatement(startsWith("create index if not exists k_"));
+ }
+
+ /**
+ * mysql commits before a DDL statement whether asked to or not, so a create index that failed there has
+ * committed everything the transaction did before it just as surely as one that succeeded: the attempt is out
+ * of the replay whatever the failure says.
+ */
+ @Test
+ public void testACreateIndexMysqlCommittedBeforeTakesTheAttemptOutOfTheReplay() throws Exception
+ {
+ final JDBCStorage storage = storageOverAnEngine(mysqlConnection.class, false);
+ when(statements.executeUpdate()).thenThrow(sql(1213, "40001"));
+ final AtomicInteger attempts = new AtomicInteger();
+
+ try
+ {
+ storage.write(txn -> {
+ attempts.incrementAndGet();
+ txn.openTree(TREE, true);
+ });
+ fail("a transaction whose create index had committed before it was replayed");
+ }
+ catch (StorageRuntimeException expected)
+ {
+ assertTrue(JDBCStorage.isRetryableConflict(expected, MYSQL), "the conflict was not the failure raised");
+ }
+ assertEquals(attempts.get(), 1, "an attempt that committed part of its work was replayed");
+ verify(engineConnection).prepareStatement(startsWith("create index k_"));
+ }
+
+ /**
+ * The connection the tree names are stamped on is closed as the attempt is unwound, and an unchecked throw out
+ * of a driver's {@code close()} there would replace the exception being unwound (JLS 14.20.2) - the very one
+ * the replay is decided on, and the only one that says what went wrong. The stamp is a diagnostic aid: it is
+ * joined to the failure instead, and the replay goes ahead.
+ */
+ @Test
+ public void testAFailingCommentConnectionDoesNotReplaceTheFailureOfTheWrite() throws Exception
+ {
+ final Connection stamp = mock(Connection.class);
+ when(stamp.createStatement()).thenReturn(mock(Statement.class)); // the lock bound of the stamp session
+ // the readback of the stored comment fails, so the stamp is given up on - with its connection open
+ when(stamp.prepareStatement(anyString())).thenThrow(new SQLException("no readback in this test", "42000"));
+ doThrow(new IllegalStateException("the driver threw out of close()")).when(stamp).close();
+ final JDBCStorage storage = storageOverAnEngine(postgresConnection.class, true, stamp);
+ final AtomicInteger attempts = new AtomicInteger();
+
+ storage.write(txn -> {
+ txn.openTree(TREE, true); // opens the stamp session, which is closed as this attempt is unwound
+ if (attempts.incrementAndGet() == 1)
+ {
+ throw new StorageRuntimeException(sql(0, "40001"));
+ }
+ });
+
+ assertEquals(attempts.get(), 2, "the failure of the comment connection replaced the conflict being unwound");
+ verify(stamp).close();
+ }
+
+ @BeforeClass
+ public void registerStubDriver() throws Exception
+ {
+ DriverManager.registerDriver(stub);
+ }
+
+ @AfterClass
+ public void deregisterStubDriver() throws Exception
+ {
+ DriverManager.deregisterDriver(stub);
+ }
+
+ /**
+ * A storage whose pool hands out one connection of this test, over a catalog that either holds the table of
+ * {@link #TREE} or does not. The connection is a mock of no recognized driver, which is how the engines that
+ * guard their create index - and mssql, which has none - reach {@code openTree}.
+ */
+ private JDBCStorage storageOverACatalogHolding(boolean theTable) throws Exception
+ {
+ final Connection con = mock(Connection.class);
+ final JDBCStorage storage = storageOver(con);
+
+ statements = mock(PreparedStatement.class);
+ final String tableName = storage.getTableName(TREE);
+ final DatabaseMetaData metaData = mock(DatabaseMetaData.class);
+ // a result set of its own per call: the catalog is asked once per attempt, and a replayed attempt
+ // reading a result set the previous one had already walked to its end would find no table there
+ when(metaData.getTables(any(), any(), any(), any())).thenAnswer(invocation -> {
+ final ResultSet tables = mock(ResultSet.class);
+ when(tables.next()).thenReturn(theTable, false);
+ when(tables.getString("TABLE_NAME")).thenReturn(tableName);
+ return tables;
+ });
+
+ when(con.isValid(anyInt())).thenReturn(true);
+ when(con.getMetaData()).thenReturn(metaData);
+ when(con.prepareStatement(anyString())).thenReturn(statements);
+ return storage;
+ }
+
+ /**
+ * A storage whose pool hands out one connection of the given engine, over a catalog holding the table of
+ * {@link #TREE} and either holding its {@code k_} index or not. The index guard and the statement behind it
+ * are the branches {@code openTree()} takes per engine, and a mock of plain {@link Connection} reaches none
+ * of them - so the name the mock ends up with is asserted here rather than assumed.
+ */
+ private JDBCStorage storageOverAnEngine(Class<? extends Connection> engine, boolean theIndex,
+ Connection... behind) throws Exception
+ {
+ final Connection con = mock(engine);
+ final String engineName = engine.getSimpleName().replace("Connection", "");
+ assertTrue(JDBCStorage.driverNameOf(con).contains(engineName),
+ "a mock of " + engine.getSimpleName() + " reaches no " + engineName + " branch: "
+ + JDBCStorage.driverNameOf(con));
+ engineConnection = con;
+ // the connections behind it answer the connects the pool does not make: the stamp of a tree name opens one
+ // of its own, straight through the driver, since the caller of openTree() is holding a pooled connection
+ final Connection[] answers = new Connection[behind.length + 1];
+ answers[0] = con;
+ System.arraycopy(behind, 0, answers, 1, behind.length);
+ final JDBCStorage storage = storageOver(answers);
+
+ statements = mock(PreparedStatement.class);
+ final String tableName = storage.getTableName(TREE);
+ final DatabaseMetaData metaData = mock(DatabaseMetaData.class);
+ // a result set of its own per call, for the reason the catalog of the test above hands out one: a replayed
+ // attempt reading a result set the previous one had already walked to its end would find nothing there
+ when(metaData.getTables(any(), any(), any(), any())).thenAnswer(invocation -> {
+ final ResultSet tables = mock(ResultSet.class);
+ when(tables.next()).thenReturn(true, false);
+ when(tables.getString("TABLE_NAME")).thenReturn(tableName);
+ return tables;
+ });
+ when(metaData.getIndexInfo(any(), any(), any(), anyBoolean(), anyBoolean())).thenAnswer(invocation -> {
+ final ResultSet indexes = mock(ResultSet.class);
+ when(indexes.next()).thenReturn(theIndex, false);
+ when(indexes.getString("INDEX_NAME")).thenReturn("k_" + tableName.substring("opendj_".length()));
+ return indexes;
+ });
+
+ when(con.isValid(anyInt())).thenReturn(true);
+ when(con.getMetaData()).thenReturn(metaData);
+ when(con.prepareStatement(anyString())).thenReturn(statements);
+ // the tree name the sweep stamps the table with runs on a connection of its own and is a diagnostic aid: a
+ // failure of it only logs, and this fixture is about the index statement rather than about the comment
+ when(con.createStatement()).thenThrow(new SQLException("no session statement in this test", "42000"));
+ return storage;
+ }
+
+ /**
+ * A storage of a pool of its own, which connects to the given connections in turn and answers every connect
+ * beyond them with the last. Every test gets a url of its own: both the pools and the distrust of a pool are
+ * keyed by the connection string, so a shared one would carry the state of one test into the next.
+ */
+ private JDBCStorage storageOver(Connection... connections) throws Exception
+ {
+ final JDBCBackendCfg cfg = mock(JDBCBackendCfg.class);
+ when(cfg.getDBDirectory()).thenReturn(StubDriver.PREFIX + pools.incrementAndGet());
+ final JDBCStorage storage = new JDBCStorage(cfg, null);
+ storage.accessMode = AccessMode.READ_WRITE;
+ stub.answerWith(connections);
+ return storage;
+ }
+
+ /** A driver of this test, so that the pool the write borrows from needs no database behind it. */
+ private static final class StubDriver implements Driver
+ {
+ static final String PREFIX = "jdbc:opendj-retry-stub:";
+
+ private volatile Connection[] answers = new Connection[0];
+ private final AtomicInteger connects = new AtomicInteger();
+
+ void answerWith(Connection... answers)
+ {
+ this.answers = answers;
+ this.connects.set(0);
+ }
+
+ @Override
+ public Connection connect(String url, Properties info)
+ {
+ if (!acceptsURL(url) || answers.length == 0)
+ {
+ return null;
+ }
+ // the last one answers every connect beyond the ones named, so a pool that opens more than the
+ // test set up gets a working connection rather than a null the driver contract reads as "not mine"
+ return answers[Math.min(connects.getAndIncrement(), answers.length - 1)];
+ }
+
+ @Override
+ public boolean acceptsURL(String url)
+ {
+ return url != null && url.startsWith(PREFIX);
+ }
+
+ @Override
+ public DriverPropertyInfo[] getPropertyInfo(String url, Properties info)
+ {
+ return new DriverPropertyInfo[0];
+ }
+
+ @Override
+ public int getMajorVersion()
+ {
+ return 1;
+ }
+
+ @Override
+ public int getMinorVersion()
+ {
+ return 0;
+ }
+
+ @Override
+ public boolean jdbcCompliant()
+ {
+ return false;
+ }
+
+ @Override
+ public Logger getParentLogger()
+ {
+ return Logger.getLogger(StubDriver.class.getName());
+ }
}
private static SQLException sql(int errorCode, String sqlState)
@@ -339,4 +1073,30 @@
// halve the effective step and change the run without either row saying so
assertEquals(clockReads.get(), expectedClockReads, name + ": clock reads");
}
+
+ /** The second failure as the next exception of the first, the way a driver chains the errors of one message. */
+ private static SQLException chained(SQLException first, SQLException next)
+ {
+ first.setNextException(next);
+ return first;
+ }
+
+ /** A chain of the given number of next exceptions whose last link is a connection that broke. */
+ private static SQLException chainEndingInADrop(int links)
+ {
+ final SQLException head = sql(2627, "23000");
+ SQLException tail = head;
+ for (int link = 2; link <= links; link++)
+ {
+ tail = chained(tail, sql(0, link == links ? "08006" : "23000")).getNextException();
+ }
+ return head;
+ }
+
+ /** The second failure suppressed into the first, the way a failing close() joins the failure of an operation. */
+ private static SQLException suppressing(SQLException failure, SQLException onRelease)
+ {
+ failure.addSuppressed(onRelease);
+ return failure;
+ }
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java
index 05b055e..25c5c14 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java
@@ -214,12 +214,13 @@
/**
* A driver reports the vendor error of a failed statement as the next exception of a generic
- * one at least as often as it reports it as the cause. Reading only the cause chain classifies
- * a lock timeout as a rejection, which leaves the tree unstamped until the next start over a
- * moment of contention.
+ * one at least as often as it reports it as the cause, and the statement of a try-with-resources
+ * carries what its {@code close()} saw as a suppressed exception. Reading fewer chains than the
+ * classifiers of a write read classifies a lock timeout - or a connection that broke - as a
+ * rejection, which leaves the tree unstamped for the life of the backend.
*/
@Test
- public void testFailureScopeWalksBothChains() {
+ public void testFailureScopeWalksEveryChain() {
final SQLException reportedAsTheCause = new SQLException("statement failed",
new SQLException("lock wait timeout exceeded", "HY000", 1205));
assertEquals(JDBCStorage.failureScope(reportedAsTheCause, JDBCStorage.Dialect.MYSQL),
@@ -236,6 +237,13 @@
assertEquals(JDBCStorage.failureScope(connectionGone, JDBCStorage.Dialect.MYSQL),
JDBCStorage.FailureScope.SESSION, "a connection exception on the next-exception chain was missed");
+ // the close() of the statement is where a connection that broke under a stamp is often the
+ // only witness, and it joins the failure being unwound as a suppressed exception (JLS 14.20.3.1)
+ final SQLException reportedByTheClose = new SQLException("statement failed");
+ reportedByTheClose.addSuppressed(new SQLException("connection closed", "08006"));
+ assertEquals(JDBCStorage.failureScope(reportedByTheClose, JDBCStorage.Dialect.MYSQL),
+ JDBCStorage.FailureScope.SESSION, "a connection exception suppressed into the failure was missed");
+
// a driver that chains an exception back to itself must not make the walk loop
final SQLException selfReferring = new SQLException("statement failed");
selfReferring.setNextException(selfReferring);
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
index b650301..22e8ea1 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
@@ -22,6 +22,7 @@
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.Cursor;
import org.opends.server.backends.pluggable.spi.Importer;
+import org.opends.server.backends.pluggable.spi.ReadOnlyStorageException;
import org.opends.server.backends.pluggable.spi.ReadOperation;
import org.opends.server.backends.pluggable.spi.ReadableTransaction;
import org.opends.server.backends.pluggable.spi.TreeName;
@@ -45,6 +46,7 @@
import java.util.Collections;
import java.util.List;
import java.util.NoSuchElementException;
+import java.util.Properties;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -56,6 +58,7 @@
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
@@ -140,6 +143,40 @@
protected abstract String getJdbcUrl();
+ /**
+ * The second property bounding a login is a socket read timeout on mysql, oracle and sql
+ * server: in force for the whole life of the connection it would fail every statement slower
+ * than it - an import batch, the statistics of a freshly loaded table - so it has to be lifted
+ * as soon as the login is through (#872).
+ */
+ @Test(timeOut = 120000)
+ public void testLoginBoundDoesNotOutliveTheLogin() throws Exception {
+ final String url = createBackendCfg().getDBDirectory();
+ final CachedConnection.ConnectDialect dialect = CachedConnection.ConnectDialect.of(url);
+ assertNotNull(dialect, "the dialect of the container is one this backend bounds: " + CachedConnection.safeUrl(url));
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "2");
+ try {
+ // the bound this lifts has to be in force first, or the assertion below holds of a
+ // connection that never carried one: established here with the very properties the
+ // borrow uses, and read back off the socket of this driver
+ final Properties bounding = new Properties();
+ assertTrue(dialect.bound(url, bounding, 2),
+ "the read bound of the login is not set for this dialect, so there is nothing to lift");
+ try (final Connection bounded = DriverManager.getConnection(url, bounding)) {
+ assertEquals(bounded.getNetworkTimeout(), 2000,
+ "the property this dialect names does not bound the socket of its login");
+ }
+
+ // a pooled connection would be handed back without being established again
+ CachedConnection.cached.invalidate(url);
+ try (final Connection con = CachedConnection.getConnection(url)) {
+ assertEquals(con.getNetworkTimeout(), 0, "the read bound of the login is still in force");
+ }
+ } finally {
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ }
+ }
+
private static ByteString key(int i) {
return ByteString.valueOfUtf8(String.format("key%02d", i));
}
@@ -284,6 +321,86 @@
}
}
+ /**
+ * A storage opened READ_ONLY must still hand out the write transaction {@code RootContainer.open()} asks for
+ * there - otherwise the offline export-ldif, verify-index and backendstat fail before reading anything - and
+ * that transaction must serve exactly what the open needs and nothing more: opening an existing tree, reads,
+ * cursors and record counts, while every mutation, including a delete through a cursor it opened, is
+ * refused (#874).
+ */
+ @Test
+ public void testReadOnlyTransactionReadsButRefusesWrites() throws Exception {
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null);
+ final TreeName tree = new TreeName("testReadOnlyTransaction", "tree");
+ final TreeName absent = new TreeName("testReadOnlyTransaction", "absent");
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ txn.put(tree, key(0), value(0));
+ txn.put(tree, key(1), value(1));
+ }
+ });
+ storage.close();
+
+ storage.open(AccessMode.READ_ONLY);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ // what RootContainer.open() does through this transaction in read-only mode
+ txn.openTree(tree, false);
+ assertEquals(txn.read(tree, key(0)), value(0));
+ assertEquals(txn.getRecordCount(tree), 2);
+ try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+ assertTrue(cursor.next());
+ assertEquals(cursor.getKey(), key(0));
+ try {
+ cursor.delete();
+ fail("delete() through a cursor of a read-only transaction must fail");
+ } catch (UnsupportedOperationException expected) {}
+ }
+
+ assertReadOnly("openTree(createOnDemand)", () -> txn.openTree(absent, true));
+ assertReadOnly("put", () -> txn.put(tree, key(2), value(2)));
+ assertReadOnly("update", () -> txn.update(tree, key(0), old -> value(3)));
+ assertReadOnly("delete", () -> txn.delete(tree, key(0)));
+ assertReadOnly("deleteTree", () -> txn.deleteTree(tree));
+ }
+ });
+
+ // nothing above reached the database
+ storage.close();
+ storage.open(AccessMode.READ_WRITE);
+ storage.read(new ReadOperation<Void>() {
+ @Override
+ public Void run(ReadableTransaction txn) throws Exception {
+ assertEquals(txn.getRecordCount(tree), 2);
+ assertEquals(txn.read(tree, key(0)), value(0));
+ return null;
+ }
+ });
+ } finally {
+ try {
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.deleteTree(tree);
+ }
+ });
+ } catch (Exception ignored) {}
+ storage.close();
+ }
+ }
+
+ private static void assertReadOnly(String operation, Runnable mutation) {
+ try {
+ mutation.run();
+ fail(operation + " must fail on a read-only storage");
+ } catch (ReadOnlyStorageException expected) {}
+ }
+
/** Buffer-served repositioning relies on the database collating keys in unsigned byte order. */
@Test
public void testCursorKeyOrderIsUnsigned() throws Exception {
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
index eef3190..6706f7b 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
@@ -1251,6 +1251,40 @@
}
}
+ /**
+ * export-ldif, verify-index and backendstat open a root container of their own, in READ_ONLY mode, when the
+ * backend is not already open - a stopped server or a disabled backend. RootContainer.open() asks the storage
+ * for a write transaction even in that mode, since that is where it opens the compressed schema and the entry
+ * containers, so a storage refusing to hand one out fails the three tools before they read anything (#874).
+ * <p>
+ * testReadOnly() above does not cover this: it expects a ReadOnlyStorageException and a storage that fails the
+ * open throws one too, from RootContainer.open() rather than from the write it is meant to be checking.
+ */
+ @Test
+ public void testOfflineToolsOpenBackendReadOnly() throws Exception
+ {
+ // Put the backend offline, so that the tools open a read-only root container of their own
+ backend.finalizeBackend();
+ try
+ {
+ final ByteArrayOutputStream exported = new ByteArrayOutputStream();
+ try (final LDIFExportConfig exportConfig = new LDIFExportConfig(exported))
+ {
+ backend.exportLDIF(exportConfig);
+ }
+ assertThat(exported.toString(StandardCharsets.UTF_8.name())).contains(testBaseDN.toString());
+
+ final VerifyConfig verifyConfig = new VerifyConfig();
+ verifyConfig.setBaseDN(testBaseDN);
+ verifyConfig.addCompleteIndex("dn2id");
+ assertThat(backend.verifyBackend(verifyConfig)).isEqualTo(0);
+ }
+ finally
+ {
+ backend.openBackend();
+ }
+ }
+
@Test
public void test_issue_496() throws Exception {
int resultCode = TestCaseUtils.applyModifications(true,
--
Gitblit v1.10.0