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 d59e1f4b22..acb9e869ec 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 @@ -37,6 +37,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.LongFunction; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -234,6 +235,54 @@ public class CachedConnection implements Connection { /** Where the sweep closes what it reaped, so that a close which does not return keeps it: see {@link Pool#sweep}. */ private static volatile Executor closer = DIRECT_EXECUTOR; + /** + * Returns the bound of one attempt to establish a connection, as configured by the {@value + * #CONNECT_TIMEOUT_PROPERTY} system property; 0 for the operator asking for no bound of its own. + * A value beyond what a millisecond bound can carry is taken down to it: three of the four + * dialects state their properties in milliseconds, and a value that saturates the conversion + * bounds nothing. + *
+ * Read here rather than at each connect so that every connection this backend establishes is + * bounded by the same configured value - the borrows of this pool and the connection {@code + * JDBCStorage} opens outside it for the tree catalog of a backend (#888) alike. A connect + * bounded tighter than the login of the deployment takes is a backend that stops opening, and + * one place to read the property is what keeps the two from drifting apart. + */ + static long getConnectTimeoutSeconds() { + return Math.min(getNonNegativeProperty(CONNECT_TIMEOUT_PROPERTY, DEFAULT_CONNECT_TIMEOUT_SECONDS, "s"), + Integer.MAX_VALUE / 1000); + } + + /** + * Returns the deadline of a whole borrow, as configured by the {@value #POOL_TIMEOUT_PROPERTY} + * system property; 0 for the operator asking for no deadline at all. + *
+ * Read here for the reason the bound of a connect is: it is what a database taking no + * connection for the moment is waited out for, and the connection {@code JDBCStorage} opens + * outside this pool for the tree catalog of a backend (#888) waits it out for exactly as long - + * one property, one meaning, whichever of the two is asking. + */ + static long getPoolTimeoutSeconds() { + return getNonNegativeProperty(POOL_TIMEOUT_PROPERTY, DEFAULT_POOL_TIMEOUT_SECONDS, "s"); + } + + /** + * The moment a borrow of this length gives up, or {@link Long#MAX_VALUE} where it gives up + * never - a property of 0, and a value so large that the milliseconds of it would overflow. + *
+ * The sum is guarded and not only the product: a value under the clamp above but large enough + * that the moment it names is past the end of the epoch would wrap to a deadline already behind + * us, and a borrow configured to wait practically forever would give up on its first retryable + * failure - the opposite of what was asked for. + */ + static long deadlineOf(long startedAt, long poolTimeoutSeconds) { + if (poolTimeoutSeconds == 0 || poolTimeoutSeconds >= Long.MAX_VALUE / 1000) { + return Long.MAX_VALUE; + } + final long deadline = startedAt + poolTimeoutSeconds * 1000; + return deadline < startedAt ? Long.MAX_VALUE : deadline; + } + /** * 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. @@ -399,7 +448,10 @@ private static void reportBoundBelowBorrowers(String connectionString, Pool pool if (borrowers <= pool.max()) { return; } - final long poolTimeoutSeconds = getNonNegativeProperty(POOL_TIMEOUT_PROPERTY, DEFAULT_POOL_TIMEOUT_SECONDS, "s"); + // through the helper the borrows and the catalog connect both read it by: the same property + // has to mean the same thing wherever it is asked, and a clamp that helper grows the day the + // deadline needs one - getConnectTimeoutSeconds() already has one - must not be missed here + final long poolTimeoutSeconds = getPoolTimeoutSeconds(); final String wait = poolTimeoutSeconds == 0 ? "waits for one to be returned for as long as that takes" : "waits up to " + poolTimeoutSeconds + "s for one to be returned and fails if none is"; @@ -1079,14 +1131,11 @@ static Connection getConnection(String connectionString, boolean trusted) throws final Pool pool = poolOf(connectionString); 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 connectTimeoutSeconds = getConnectTimeoutSeconds(); + final long poolTimeoutSeconds = getPoolTimeoutSeconds(); final long ttlMillis = getCacheTtlMillis(); final long startedAt = System.currentTimeMillis(); - final long deadline = (poolTimeoutSeconds == 0 || poolTimeoutSeconds >= Long.MAX_VALUE / 1000) - ? Long.MAX_VALUE : startedAt + poolTimeoutSeconds * 1000; + final long deadline = deadlineOf(startedAt, poolTimeoutSeconds); // A thread already holding a connection is not made to wait for one: the two are held at // the same time, so waiting for the first to come back would wait for itself. final boolean reentrant = pool.heldByCurrentThread(); @@ -1493,16 +1542,71 @@ private static void warnPoolFull(String connectionString, String message) { // 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) { + warnStall(connectionString, "", startedAt, + waitedMs -> stallMessage(connectionString, attempts, waitedMs, cause)); + } + + /** + * The same for a connect this class makes for somebody outside the pool - the connection the + * tree catalog of a backend is written on (#888) - which waits for no pooled connection and + * must not be described as one. + *
+ * Throttled apart from the borrows of the same url as well as worded apart from them: the two
+ * stall on the same database for the same reason, so a borrow that warned a moment ago would
+ * otherwise silence the connect that is about to fail - the one of the two an operator has no
+ * other line about.
+ */
+ static void warnStallOutsidePool(String connectionString, String what, int attempts, long startedAt,
+ SQLException cause) {
+ warnStall(connectionString, "|" + what, startedAt,
+ waitedMs -> outsidePoolStallMessage(connectionString, what, attempts, waitedMs, cause));
+ }
+
+ private static void warnStall(String connectionString, String throttleKeySuffix, long startedAt,
+ LongFunction
+ * Built apart from the logging of it for the reason {@link #stallMessage} is: what it has to
+ * keep is a rule a test can hold it to, and the shipped path reaches the throttle only where a
+ * connect really has stalled for a second. The suffix is what keeps the two waits apart - a
+ * borrow of the pool and the connect the tree catalog of a backend is made on (#888) stall on
+ * the same database for the same reason, and a borrow that reported a moment ago must not
+ * silence the connect that is about to fail, which has no other line about it at all.
+ *
+ * Filing the moment is part of deciding it, so that two threads asking at once report once.
+ */
+ static boolean stallWarningDue(String connectionString, String throttleKeySuffix, long startedAt, long now) {
if (now - startedAt < STALL_WARNING_AFTER_MS) {
- return;
+ return false;
}
final AtomicLong lastOfThisUrl =
- lastStallWarning.computeIfAbsent(safeUrl(connectionString), url -> new AtomicLong());
+ lastStallWarning.computeIfAbsent(safeUrl(connectionString) + throttleKeySuffix, 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)));
- }
+ return now - last >= STALL_WARNING_INTERVAL_MS && lastOfThisUrl.compareAndSet(last, now);
+ }
+
+ /**
+ * The stall of a connect made outside the pool, as it reaches the log. Built apart from the
+ * logging of it for the reason {@link #stallMessage} is: 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 outsidePoolStallMessage(String connectionString, String what, int attempts, long waitedMs,
+ SQLException cause) {
+ return String.format("%s takes no further connection: the %s connection of this backend is opened outside the"
+ + " pool and has been retrying for %d ms (%d attempts), last error: %s", safeUrl(connectionString), what,
+ waitedMs, attempts, redact(cause.getMessage(), connectionString));
}
/**
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 a0059e5a07..86a3fdfb8e 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
@@ -25,6 +25,7 @@
import org.forgerock.opendj.config.server.ConfigurationChangeListener;
import org.forgerock.opendj.ldap.ByteSequence;
import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.server.config.server.JDBCBackendCfg;
import org.opends.server.backends.pluggable.spi.*;
import org.opends.server.core.ServerContext;
@@ -36,6 +37,7 @@
import java.io.Closeable;
import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.sql.*;
@@ -823,31 +825,37 @@ public void close() {
// that it is not reissued for every tree on every open; disabling and re-enabling the
// backend is the way to try again once the privilege has been granted
unstampableTrees.clear();
+ // what this storage knows of its catalog holds no longer than the open it learnt it in: the
+ // table may well be gone by the next one, dropped by an offline tool run in the meantime
+ catalogTableOpened=false;
+ enrolledTrees.clear();
// A closed backend has no use for its connections. They used to stay open - close() only
// flipped the status - so disabling or removing a JDBC backend left them behind, and with
// nothing left to expire the pool entry they could stay open for good (issue #878).
releasePool();
}
- // The trees this storage has taken an interest in, and the tables they map to. listTrees() -
- // and through it removeStorageFiles() - reads this, so a tree only belongs here once this
- // backend uses it: see toTableName() below for the trees that are merely asked about.
+ // The trees this storage has taken an interest in, and the tables they map to: a memo, so that
+ // naming the table of a tree costs a map lookup rather than a digest. What a backend owns is
+ // recorded in its catalog and not here (#888) - listTrees() and removeStorageFiles() read that
+ // - but the distinction the two names below draw is kept all the same: a tree merely asked
+ // about is not one this storage has taken an interest in, and it stays out of the memo.
final LoadingCache
- * Which of the two a statement takes therefore says who owns the tree it names: a path that
- * creates or writes one - openTree(), clearTree(), deleteTree(), put(), update(), delete() -
- * takes the enrolling {@link #getTableName(TreeName)}, and a read-only path - read(),
- * getRecordCount(), isExistsTable() and the cursor - takes {@link #readTableName(TreeName)},
- * which computes this only for a tree that is not enrolled already. Every tree this backend
- * owns passes through openTree(name, true) as it is opened, so listTrees() still names the
- * complete owned set.
+ * Which of the two names a statement takes therefore says whether this backend is claiming the
+ * tree it names: a path that creates or writes one - openTree(), clearTree(), deleteTree(), put(),
+ * update(), delete() - takes the enrolling {@link #getTableName(TreeName)}, and a path that only
+ * asks - read(), getRecordCount(), isExistsTable(), the cursor, and the read of what the catalog
+ * records - takes {@link #readTableName(TreeName)}, which computes this only for a tree that is not
+ * enrolled already. What a clear may drop is decided by the catalog of the backend (#888) and no
+ * longer by this memo, so an entry of it puts no table up for removal; the two names are what keeps
+ * the memo an account of the trees this backend claims all the same.
*/
static String toTableName(TreeName treeName) {
try {
@@ -869,6 +877,85 @@ String getTableName(TreeName treeName) {
return tree2table.get(treeName);
}
+ /**
+ * The pseudo base DN of the tree naming the trees of a backend. Every real tree of a backend is
+ * named after an entry container, whose prefix is a normalized DN and so always holds a "=",
+ * which an identifier of this form cannot collide with.
+ */
+ static final String CATALOG_BASE_DN="opendj_catalog";
+
+ /**
+ * The base DN the compressed schema trees were named under before #881 gave each backend a pair
+ * of its own. It carries no backend qualifier, so on a database addressed by several backends -
+ * which nothing forbids (#873) - that pair of trees is the same pair for all of them, and a
+ * backend must not put a tree another one may be the owner of up for removal. It is the pair
+ * {@code PersistentCompressedSchema} migrates from and never writes to again, and it is left
+ * exactly where it lies: the definitions of a backend that has not been started since the
+ * upgrade are still in it. The pair each backend owns is named after its backend id, is under no
+ * such literal, and is enrolled like any other tree.
+ */
+ static final String SHARED_COMPRESSED_SCHEMA_BASE_DN="compressed_schema";
+
+ /**
+ * The pair named under {@link #SHARED_COMPRESSED_SCHEMA_BASE_DN}, spelled out here because the
+ * names are private to {@code PersistentCompressedSchema} - where they are the LEGACY_ pair of
+ * #881. They are never enrolled, so nothing but this constant can name them - and a tool asking
+ * a backend what trees it holds has to be told about them all the same, which is what {@link
+ * #listTrees()} uses this for.
+ */
+ static final List
+ * A table is named after the hash of its tree name, so the catalog of a database can neither be
+ * filtered by a per-backend prefix nor read back into a {@link TreeName}. Without a record of its
+ * own a backend can therefore only name the trees this very process has already touched - which
+ * is precisely what {@link #removeStorageFiles()} cannot have, running as it does before the root
+ * container is open. In the offline {@code import-ldif} nothing has touched a tree at all, so
+ * {@code --clearBackend} used to clear nothing whatsoever (#888).
+ *
+ * The catalog is per backend and named after the backend id alone: a process that has opened
+ * nothing can still find its table, and backends sharing one database URL - which nothing
+ * forbids (#873) - never name each other's trees. The id goes in escaped, for the reason {@link
+ * #escapedBackendId} states: a name that does not survive being read back is a table of this
+ * backend that its own clear cannot recognize.
+ */
+ TreeName getCatalogTree() {
+ return new TreeName(CATALOG_BASE_DN, escapedBackendId());
+ }
+
+ /**
+ * Whether the table of the catalog was created, or found, by this storage. A tree is enrolled on
+ * every open - about 25 of them for a stock suffix - and asking the catalog whether the table is
+ * there would cost a metadata round trip per tree.
+ */
+ private volatile boolean catalogTableOpened=false;
+
+ /**
+ * Serializes the one step above: two transactions opening trees at the same time would otherwise
+ * both find the table of the catalog absent and both create it, the second failing the open it
+ * belongs to. Held across the lookup and the statement that answer it, and across nothing else.
+ */
+ private final Object catalogLock=new Object();
+
+ /**
+ * The trees the catalog already records at the table this version would record them at, read
+ * from it when this storage first opens it and added to as it enrols. A tree named here needs no
+ * row written for it: the row would be the one that is already there, and writing one is a
+ * statement and a commit on a connection this backend then has to have opened - a stock suffix
+ * has about 25 trees, and every open after the first enrols none of them.
+ *
+ * A row recording another table than {@link #getTableName} would give is not in here: what a
+ * removal drops is the table the row records, so a row of a version naming its tables otherwise
+ * has to be rewritten rather than trusted. Held no longer than the open it was read in, like
+ * {@link #catalogTableOpened}, and given up whenever the catalog itself is.
+ */
+ private final Set
+ * It is established the way a pooled connection is and not the way a stamp connection is: the
+ * bounds of {@link CachedConnection.ConnectDialect} rather than of {@link Dialect}, so that a
+ * login which never answers is bounded, a bound the administrator set in the connection string is
+ * left exactly as they set it, and the read bound of the login is lifted as soon as the login is
+ * through (#872). A stamp is a diagnostic aid and gives up rather than queue behind another
+ * session; a catalog row is the state a clear reads, and it waits for its lock rather than dying
+ * on a read bound. The isolation is the pool's for the same reason: this connection issues the
+ * ordinary DML of this class, and the repeatable read a mysql server defaults to gap-locks a
+ * catalog two transactions enrol into.
+ *
+ * The bound of the connect is the one the pool bounds its own connects by, read from {@link
+ * CachedConnection#CONNECT_TIMEOUT_PROPERTY} where an operator set it: a login of this database
+ * takes what it takes whoever is asking, so a deployment which had to raise that property must not
+ * meet a bound of this code's own here - a connect failing where the pooled one beside it succeeds
+ * is a backend that stops opening on an installation that opened before this connection existed. A
+ * property of 0 is the operator asking for no bound of the connect, and it is honoured here as it
+ * is by the pool. What does bound an attempt besides is the deadline of the retry below, which is
+ * the pool's own rule and applies to a borrow in exactly the same way; it is no bound of this
+ * code's own choosing.
+ *
+ * The deadline of the whole thing is the pool's as well, {@link
+ * CachedConnection#POOL_TIMEOUT_PROPERTY}: a database that takes no connection for the
+ * moment - at its connection limit with one of ours on its way back to the pool, or still
+ * recovering - is waited out here exactly as a borrow waits it out, by the predicate the pool
+ * decides that by ({@link CachedConnection#isWorthRetrying}) and with the same backoff. Without
+ * it this connect makes one attempt where the borrow beside it makes many, and loses a race the
+ * pooled connection of the very same operation wins. 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 rather than retried behind its back.
+ *
+ * What this deadline is not is the deque of the pool: the caller of {@code openTree()} is holding
+ * a pooled connection already, so waiting for a peer to return one would be waiting for the very
+ * thread that is waiting. It is the pool's retry that is wanted here and not its queue, which is
+ * why the loop below is its own rather than a borrow of {@link CachedConnection#getConnection}.
+ * What one attempt is, is the login and the set-up behind it, exactly as an attempt of a borrow is
+ * ({@code CachedConnection.connect}): a session the server takes and then kills off answers the
+ * first statement of the set-up rather than the login, and it is the same refusal either way.
+ *
+ * That is also what this wait is weaker than a borrow at, and it is worth writing down rather than
+ * leaving to be discovered: a borrow can be answered by a peer handing a connection back, while
+ * nothing here can be answered by anything but a new login. Against a server at its connection
+ * limit whose remaining slots this backend's own pool is holding idle, the borrows of that pool
+ * clear and this does not - it waits out the deadline and reports the refusal. The deadline is
+ * therefore what bounds it, and a deployment which has set both properties to 0 has asked for a
+ * wait with no end to it here as much as in the pool.
+ *
+ * One attempt is bounded by the configured connect timeout and by what is left of that deadline,
+ * whichever is the shorter, exactly as an attempt of a borrow is: an attempt left to run its own
+ * bound out past the deadline would overrun it by a whole connect timeout, and turning the
+ * per-attempt bound off must not turn the deadline off with it. So the connect property of 0 that
+ * the round before this one made honoured is the operator asking for no bound of their own
+ * here as it is in the pool, and what is left unbounded by both properties at 0 is left unbounded
+ * here too - a login the database accepts and never finishes then parks the transaction that asked
+ * for it. A borrow of the pool parks in exactly the same way, on exactly the same pair of settings.
+ * It does not park the rest of this storage: {@code openCatalog()} establishes this connection
+ * before it takes its lock, for that very reason.
+ *
+ * What every wait here does hold is the caller: this runs inside the write transaction that reached
+ * {@code openTree}, so a retry that waits out a database refusing connections holds that
+ * transaction's pooled connection, the permit of the pool that connection carries (#878) and every
+ * lock the transaction has already taken, for as long as it waits. On a stock suffix {@code
+ * RootContainer.open()} is one such write over every tree of the backend.
+ *
+ * And what it spends besides is the window {@link #write} bounds its own replay by, which is the
+ * shorter of the two by default - ten seconds against a minute - and is spent by this wait
+ * rather than added to it: this loop runs inside one attempt of that one. A refusal {@code write()}
+ * would replay - mysql answers its connection limit with {@code 08004}, postgres reports a database
+ * still coming up as {@code 57P03}, and both are read as a connection this backend lost - waited out
+ * here for a minute reaches that loop with its window six times over, so it is thrown unreplayed:
+ * the retry would have cost the caller the very replay it had before there was any retry here at
+ * all. So the deadline is the shorter of {@link CachedConnection#POOL_TIMEOUT_PROPERTY} and what is
+ * left of that window, taken from the caller by {@link CatalogSession#boundedAlsoBy}. The window is
+ * not something the property could express: lowering it under ten seconds shortens every borrow of
+ * the pool with it. A path carrying no such window - the importer, which has no replay above it -
+ * waits the property out in full, and a deployment which cannot afford a minute of that sets the
+ * property to what it can afford, the same property bounding the same wait as it bounds a borrow.
+ *
+ * What the window does not bound is one attempt, which is taken from the deadline of the
+ * pool as it always was. The two are different questions: the window says how long it is worth
+ * waiting before handing the failure to a loop that can still replay it, while a login takes what
+ * this database takes whoever is asking. Cut to what is left of a ten second window, a deployment
+ * that raised {@link CachedConnection#CONNECT_TIMEOUT_PROPERTY} to two minutes because its login
+ * needs them would meet a catalog connect failing where the pooled connection beside it succeeds -
+ * the backend that stops opening. So one slow attempt may outlast the window, exactly as one slow
+ * conflict outlasts it in {@link #write} itself; what may not is a second attempt begun after the
+ * window has already run out, which is a wait bought with a replay that no longer exists.
+ *
+ * @param budgetDeadline the moment the replay window of the caller runs out, as {@link
+ * System#currentTimeMillis()} reads it, or {@link Long#MAX_VALUE} where nothing above this
+ * connect replays - the same "no deadline at all" this class reads out of {@link
+ * CachedConnection#deadlineOf}, so that the shorter of the two is a plain {@code min}.
+ */
+ Connection newCatalogConnection(long budgetDeadline) throws SQLException {
+ // poolKey() rather than the configuration as it stands, for the reason newStampConnection()
+ // gives: this connection is not pooled, but it is a connection to the database of this
+ // storage, and db-directory may be changed on a running backend. Reading it again here would
+ // write the catalog of this backend into whichever database the configuration names now,
+ // while its tables are created, read and dropped over the connection open() registered -
+ // rows in one database and tables in another, which is #888 again by another route (#878)
+ final String connectionString=poolKey();
+ final CachedConnection.ConnectDialect dialect=CachedConnection.ConnectDialect.of(connectionString);
+ final long connectTimeoutSeconds=CachedConnection.getConnectTimeoutSeconds();
+ final long poolTimeoutSeconds=CachedConnection.getPoolTimeoutSeconds();
+ final long startedAt=System.currentTimeMillis();
+ final long poolDeadline=CachedConnection.deadlineOf(startedAt, poolTimeoutSeconds);
+ // the deadline of the whole wait, which is the shorter of the pool's own and what is left of
+ // the replay window of the caller. The bound of one attempt below is taken from the pool's
+ // alone, deliberately: a login the operator bounded at two minutes because that is what this
+ // database takes must not be cut to what is left of a ten second window - that is the connect
+ // dying where the pooled one beside it succeeds, which is a backend that stops opening. One
+ // slow attempt may still outlast the window, exactly as one slow conflict does; what may not
+ // is a second attempt begun after the window has run out, which is a wait for nothing
+ final long deadline=Math.min(poolDeadline, budgetDeadline);
+ long backoffMs=0;
+ int attempts=0;
+ while (true) {
+ attempts++;
+ try {
+ // the bound of one attempt and not of the whole wait, the way the pool bounds its own:
+ // an attempt left to run its bound out past the deadline would overrun it by a full
+ // connect timeout, and turning the per-attempt bound off must not turn this one off.
+ // Taken from the deadline of the pool and not from the shorter of the two: see above
+ return connectCatalog(connectionString, dialect,
+ CachedConnection.attemptSeconds(connectTimeoutSeconds, poolDeadline));
+ }catch (SQLException e) {
+ if (!CachedConnection.isWorthRetrying(e, dialect)) {
+ // redacted the way the pool redacts the failure of its own connects: a driver renders
+ // the connection string it could not use into its message as readily as not, and the
+ // connection string of this backend carries the password of the account it works as.
+ // This failure is reported in full - ERR_OPEN_ENV_FAIL, or the log of a clear
+ throw CachedConnection.reported(e, connectionString);
+ }
+ final long now=System.currentTimeMillis();
+ final long remaining=deadline-now;
+ if (remaining<=0) {
+ // which of the two bounds ended it, so that an operator reading the line knows whether
+ // the property is the thing to raise: where the replay window of the caller is the
+ // shorter one, raising the property moves nothing
+ throw catalogConnectTimedOut(connectionString, poolTimeoutSeconds,
+ deadline==budgetDeadline, now-startedAt, attempts, e);
+ }
+ CachedConnection.warnStallOutsidePool(connectionString, "tree catalog", attempts, startedAt, e);
+ backoffMs=Math.min(backoffMs==0 ? 1 : backoffMs*2, CachedConnection.MAX_BACKOFF_MS);
+ try {
+ Thread.sleep(Math.min(backoffMs, remaining));
+ }catch (InterruptedException interrupted) {
+ // the flag is put back - Thread.sleep() clears it, and every frame above this one reads
+ // it to decide whether to unwind - and the wait is over: whoever asked this thread to
+ // stop is not answered by going on to sleep out the rest of a pool timeout. The driver
+ // failure is what this reports, it being the reason there was anything to wait for, and
+ // the interrupt is carried on it as suppressed so that a connect cut short by a
+ // shutdown is not read off the log as a database that would not take a connection
+ Thread.currentThread().interrupt();
+ final SQLException reported=CachedConnection.reported(e, connectionString);
+ reported.addSuppressed(interrupted);
+ throw reported;
+ }
+ }catch (RuntimeException e) {
+ // a driver reporting a connect it will not make as an unchecked failure names the
+ // connection string just as readily, and it is not one of the two states a retry waits
+ // out: reported and handed on, exactly as the pool hands its own on. reportedUnchecked()
+ // answers with the original where it holds no credential, so nothing of a plain
+ // programming error is hidden by this
+ final Exception reported=CachedConnection.reportedUnchecked(e, connectionString);
+ if (reported instanceof SQLException) { // redacted, and reported as the connect failure it is
+ throw (SQLException) reported;
+ }
+ throw (RuntimeException) reported; // the original: it holds no credential of this backend
+ }
+ }
+ }
+
+ /**
+ * The failure of a catalog connect that was worth retrying and ran the deadline out: a timeout by
+ * type, so that a caller can tell it from the first refusal, and carrying the state and the vendor
+ * code of the last failure of the driver rather than one of its own.
+ *
+ * Not the {@code 08001} the pool answers a borrow of this shape with, and the difference is not
+ * cosmetic: this failure is raised inside {@link #write}, whose classification reads every state
+ * of class {@code 08} as a connection the database dropped ({@link #saysTheConnectionIsGone}). A
+ * manufactured one would put an attempt whose pooled connection is perfectly healthy into the
+ * replay and call {@link #distrustPool} on it over a database that had simply refused a new
+ * connection.
+ *
+ * It buys exactly that and no more, which is worth being precise about: where the driver's own
+ * refusal is of class {@code 08} - mysql answers its connection limit with {@code 08004} - the
+ * attempt is classified as a dropped connection whatever this method does, the original being the
+ * cause of this one and every chain of a failure being walked. What this keeps is the promise that
+ * the retry changes no classification: a refusal reaches {@code write()} as the same thing it
+ * reached it as before there was any retry here at all.
+ *
+ * Which of the two bounds ended the wait is named rather than left to be guessed: the property is
+ * the thing to raise only where the property is what ran out, and where the replay window of the
+ * caller is the shorter one - the default has it at a sixth of the property - raising the property
+ * moves nothing at all.
+ */
+ private static SQLTimeoutException catalogConnectTimedOut(String connectionString, long poolTimeoutSeconds,
+ boolean endedByReplayWindow, long waitedMs, int attempts, SQLException last) {
+ final SQLTimeoutException timeout=new SQLTimeoutException("no connection to "
+ +CachedConnection.safeUrl(connectionString)+" could be opened for the tree catalog within "
+ +waitedMs+"ms ("+attempts+" attempts, "+(endedByReplayWindow
+ ? "what was left of the replay window of the write that asked for it, which is the shorter"
+ +" bound here: "+CachedConnection.POOL_TIMEOUT_PROPERTY+" is "+poolTimeoutSeconds+"s"
+ : CachedConnection.POOL_TIMEOUT_PROPERTY+"="+poolTimeoutSeconds+"s")
+ +"): the database took no connection for the moment,"
+ +" last error: "+CachedConnection.redact(last.getMessage(), connectionString),
+ last.getSQLState(), last.getErrorCode());
+ timeout.initCause(CachedConnection.reported(last, connectionString));
+ return timeout;
+ }
+
+ /**
+ * One attempt of {@link #newCatalogConnection}, established and set up or left holding nothing.
+ * Failures leave here as the driver reported them, checked and unchecked alike: what a retry is
+ * decided on is the chain of the original, and the redaction is the caller's - a redacted copy is
+ * rebuilt link by link, so redacting an attempt that is about to be retried would pay for a
+ * failure nobody ever sees.
+ */
+ private Connection connectCatalog(String connectionString, CachedConnection.ConnectDialect dialect,
+ long timeoutSeconds) throws SQLException {
+ // A driver is free to write into the map it is handed, so every attempt gets one of its own.
+ final Properties properties=new Properties();
+ final boolean readBoundSet=dialect!=null && timeoutSeconds>0
+ && dialect.bound(connectionString, properties, timeoutSeconds);
+ final Connection con=DriverManager.getConnection(connectionString, properties);
+ try {
+ con.setAutoCommit(false);
+ con.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
+ }catch (SQLException | RuntimeException e) { // nothing else holds this connection yet: it would leak
+ closeQuietly(con, e);
+ throw e;
+ }
+ if (readBoundSet) {
+ try {
+ // only where this code set one: a read bound of the connection string is the
+ // administrator's and is not lifted along with it, exactly as the pool leaves it
+ con.setNetworkTimeout(Runnable::run, 0);
+ }catch (SQLException | RuntimeException e) {
+ // A driver that will not take the bound back leaves it in force for the life of the
+ // connection, and that bound is the one this attempt was given - near the end of the
+ // deadline of the retry, a second. The connection is kept all the same, which is the
+ // pool's own answer to this failure: it stops pooling such a connection and still hands
+ // it to the borrower that is waiting. Failing here instead would stop the backend opening
+ // on a driver whose setNetworkTimeout is not implemented at all, where the pooled
+ // connection beside it works - and there is no state to fail with that write() does not
+ // read as a connection the database dropped. So it is reported, at the bound in force.
+ logger.warn(LocalizableMessage.raw("jdbc: the catalog connection of backend %s keeps the %ds read bound its login was given, so a statement of the catalog slower than that fails on it: %s",
+ config.getBackendId(), timeoutSeconds, stackTraceToSingleLineString(e)));
+ }
+ }
+ return con;
+ }
+
+ /** Closes a connection nothing holds yet, reporting the failure of the close on the one being unwound. */
+ private static void closeQuietly(Connection con, Throwable unwinding) {
+ try {
+ con.close();
+ }catch (SQLException | RuntimeException e) {
+ // the unchecked one as well: this runs from the catch of a failure it must not replace
+ // (JLS 14.20.2), which is the rule every close of this class keeps
+ unwinding.addSuppressed(e);
+ }
+ }
+
+ /**
+ * The connection the catalog table of a backend is read and written on, and the transaction over
+ * it. No other connection touches that table.
+ *
+ * It belongs to a write transaction and not to the storage, and is opened at the first row that
+ * transaction has to write - so what costs a physical connect is a write which enrols a tree the
+ * catalog does not already record, and nothing else: a read-only storage opens none, a
+ * transaction that opens no tree opens none, and neither does one whose trees are all recorded
+ * already, which is every open after the first ({@link JDBCStorage#enrolledTrees} is the storage's and
+ * outlives them). The open of a backend is therefore one connect, and so is every later write
+ * that names a tree for the first time - {@code dsconfig create-backend-index} reaches exactly
+ * that, opening its new tree inside a write of its own on a running server. The connect is
+ * retried the way the pool retries its own for that reason; see {@link JDBCStorage#newCatalogConnection}.
+ *
+ * Storage-scoped rather than transaction-scoped it cannot be: it is closed with the transaction
+ * because the rows it writes are the transaction's, and a connection outliving them would be a
+ * second pooled-connection lifetime for this class to get right.
+ *
+ * Why the rows are not written on the caller's connection is in {@link
+ * WriteableTransactionTransactionImpl#enrolInCatalog}: they have to be committed, and that commit
+ * must not be the caller's. Why the read is not either is in {@link
+ * WriteableTransactionTransactionImpl#readEnrolledTrees}: a select of the caller's transaction
+ * would hold a lock on the catalog table for the whole life of that transaction, and the rows it
+ * decides are written from here.
+ */
+ final class CatalogSession implements Closeable {
+ private Connection con;
+ private WriteableTransactionTransactionImpl txn;
+
+ // The moment the replay window of the write() this session belongs to runs out, as
+ // System.nanoTime() reads it - null where nothing above this session replays, which is the
+ // importer and nothing else. Boxed rather than given a sentinel: nanoTime() is documented to
+ // return an arbitrary long, so there is no reading of it that could stand for "no window".
+ private Long replayWindowEndsAt;
+
+ /**
+ * Tells this session the wall-clock window the {@link JDBCStorage#write} above it bounds its
+ * replay by, which the connect of the catalog may not outlast: the connect runs inside one
+ * attempt of that loop, so a wait longer than what is left of the window reaches it with the
+ * window already spent and is thrown unreplayed - see {@link JDBCStorage#newCatalogConnection}.
+ * Called once per attempt, before the operation runs; a session nobody calls it on waits the
+ * pool timeout out in full, which is what the importer does.
+ */
+ void boundedAlsoBy(long replayWindowEndsAtNanos) {
+ replayWindowEndsAt=replayWindowEndsAtNanos;
+ }
+
+ /**
+ * That window as a deadline of the clock the connect measures itself by, or {@link
+ * Long#MAX_VALUE} where there is no window - the value {@link CachedConnection#deadlineOf}
+ * gives a wait with no end, so that the shorter of the two is a plain {@code min}. The two
+ * readings are taken here rather than one of them being carried in: a nanoTime window and a
+ * currentTimeMillis deadline are two clocks, and they can only be put together at one moment.
+ * A window already spent gives the moment itself, which is one attempt and then a timeout.
+ */
+ private long budgetDeadline() {
+ if (replayWindowEndsAt==null) {
+ return Long.MAX_VALUE;
+ }
+ final long leftNanos=replayWindowEndsAt-System.nanoTime();
+ final long now=System.currentTimeMillis();
+ return leftNanos<=0 ? now : now+leftNanos/1_000_000L;
+ }
+
+ /** The connection, opened at the first read or write the catalog needs and shared by the rest. */
+ Connection connection() throws SQLException {
+ if (con==null) {
+ con=newCatalogConnection(budgetDeadline());
+ }
+ return con;
+ }
+
+ /** Whether this session is holding a connection already, so that a caller knows what it made. */
+ boolean isEstablished() {
+ return con!=null;
+ }
+
+ /**
+ * A transaction over that connection, for its row statements alone: an upsert and a delete are
+ * per engine, and writing the catalog through the very ones every other tree is written through
+ * is what keeps its rows the same shape as theirs. It opens no tree and stamps no table, so the
+ * sessions it carries of its own are never opened.
+ */
+ WriteableTransactionTransactionImpl transaction() throws SQLException {
+ final Connection con=connection();
+ if (txn==null) {
+ txn=new WriteableTransactionTransactionImpl(con);
+ }
+ return txn;
+ }
+
+ void commit() throws SQLException {
+ con.commit();
+ }
+
+ /**
+ * What a failed statement left behind must not poison the write of the next row: postgres
+ * refuses every further statement of a transaction whose statement failed (25P02) until it is
+ * rolled back, and this connection outlives the row that failed on it.
+ */
+ void reset() {
+ if (con!=null) {
+ try {
+ con.rollback();
+ }catch (SQLException | RuntimeException e) {
+ // the unchecked one as well: a driver is free to answer a rollback on a connection the
+ // database dropped with one, and this runs from the catch of a failure it must not
+ // replace - the caller goes on to report that failure, and in createCatalogTable() to
+ // tolerate a table another session created while this one was creating it
+ close();
+ }
+ }
+ }
+
+ /**
+ * The unchecked failure of a close is taken like the checked one, and the session is given up
+ * in a finally: this is called from {@link #reset}, which runs from the catch of a failure it
+ * must not replace (JLS 14.20.2) - {@code createCatalogTable()} goes on from there to tolerate
+ * a table another session created while this one was creating it. A session whose connection
+ * would not close is left holding none rather than holding a dead one.
+ *
+ * The catch of {@code write()}'s own finally is the same guard one layer out, kept as the
+ * belt to this one's braces: it was the only guard while this method let an unchecked failure
+ * past, and a session that stops swallowing must not have to be found through a failure it
+ * replaced.
+ */
+ @Override
+ public void close() {
+ if (con!=null) {
+ try {
+ con.close();
+ }catch (SQLException | RuntimeException e) {
+ logger.trace(LocalizableMessage.raw("jdbc: unable to close the catalog connection: %s", stackTraceToSingleLineString(e)));
+ }finally {
+ con=null;
+ txn=null;
+ }
+ }
+ }
+ }
+
// The connection the comment statements of one sweep of openTree() calls share. Opening a
// backend opens every tree it holds (about 25 for a stock suffix), so a connection per stamp
// would mean that many physical connects on the first open after an upgrade - the one open
@@ -1134,21 +1632,32 @@ void reset() {
if (con!=null) {
try {
con.rollback();
- }catch (SQLException e) {
+ }catch (SQLException | RuntimeException e) {
+ // the unchecked one as well: a driver is free to answer a rollback on a connection the
+ // database dropped with one, and this runs from the catch of a failure it must not
+ // replace - the caller goes on to report that failure, and in createCatalogTable() to
+ // tolerate a table another session created while this one was creating it
close();
}
}
}
+ /**
+ * The unchecked failure of a close is taken like the checked one, and the session is given up
+ * in a finally, for the reason {@link CatalogSession#close} gives: this runs from {@link
+ * #reset}, which runs from the catch of a failure it must not replace - and a stamp that
+ * failed must never become the outcome of the open it was issued from.
+ */
@Override
public void close() {
if (con!=null) {
try {
con.close();
- }catch (SQLException e) {
+ }catch (SQLException | RuntimeException e) {
logger.trace(LocalizableMessage.raw("jdbc: unable to close the comment connection: %s", stackTraceToSingleLineString(e)));
+ }finally {
+ con=null;
}
- con=null;
}
mysqlBackslashEscape=null; // it described the session that has just gone
}
@@ -1337,8 +1846,10 @@ private static FailureScope scopeOf(SQLException e, Dialect dialect) {
}
// Returns the comment currently stored on the table, or null when there is none. The dialect is
- // passed in rather than read off the connection: this runs on the stamp connection, which is
- // not a pooled one, and only for the dialects commentTable() recognizes.
+ // passed in rather than read off the connection: the stamp sweep runs this on a connection of its
+ // own, a clear runs it on the pooled one it did its work on, and both only for the dialects
+ // commentTable() recognizes. It is a read and nothing else, and CachedConnection.close() rolls
+ // back before the connection is handed on, so a clear leaves no transaction of its own behind.
String readStoredComment(Connection con, Dialect dialect, String tableName) throws SQLException {
final String sql;
final String arg;
@@ -1353,7 +1864,7 @@ String readStoredComment(Connection con, Dialect dialect, String tableName) thro
break;
case ORACLE:
sql="select comments from user_tab_comments where table_name=?";
- arg=tableName.toUpperCase();
+ arg=tableName.toUpperCase(Locale.ROOT);
break;
case MICROSOFT:
sql="select cast(value as nvarchar(4000)) from sys.extended_properties where class=1 and major_id=object_id(?) and minor_id=0 and name='MS_Description'";
@@ -1415,7 +1926,7 @@ boolean updateTableStatistics(Connection con, Collection
+ * An "opendj" table still standing at that point is named by no catalog of this backend, and its
+ * name says nothing about whose it is - a table is named after the hash of its tree name. What
+ * does say so is the comment a table is stamped with as it is opened (#866): the tree name in
+ * plain text. A table whose stamp names a tree of a base DN this backend does not serve belongs to
+ * a backend sharing this database (#873) and is passed over in silence; one whose stamp names a
+ * tree of this backend is reported as its own, and so as removable by hand; one carrying no stamp
+ * at all - left by a version stamping no table, or by a database that refused the comment - can be
+ * attributed to nobody and is reported as exactly that. A stamp the database would not give up is
+ * reported apart from all of these: it says nothing either way, and counting it as a table without
+ * a stamp would turn a connection that died halfway into a confident line about tables this
+ * backend may well own.
+ *
+ * The silence has a cost worth stating: a table stamped with a tree of a base DN that was taken
+ * out of the configuration while the backend was disabled reads exactly like a table of a backend
+ * sharing the database, the stamp naming the tree and never the backend it belonged to, so it is
+ * passed over too. What is left of such a base DN is found by its stamp and removed by hand.
+ *
+ * The shared compressed schema pair is left out of all of it: it is kept on purpose (#881), so it
+ * is no leftover of anything, and naming it here would be asking for the removal of the one thing
+ * this code goes out of its way to spare.
+ *
+ * A clear which removed no tree of this backend is called out ahead of all of it: #888 was exactly
+ * such a clear, and it went by without a word in the log. A backend upgraded in place is the one
+ * case where a clear drops nothing while there is something to drop - nothing enrols a tree before
+ * {@link #removeStorageFiles()} runs, so the first offline clear of such a backend finds no
+ * catalog at all - and the line says so rather than leaving it to be found out.
+ *
+ * The catalog table is no term of that count. It is dropped like any other and by the same loop,
+ * so a catalog standing over rows that name nothing - a backup restored older than the tables it
+ * was taken beside - is one table dropped and not one tree removed, and the line has to fire there
+ * too: what an operator meets in that case is the same clear that removed none of their data.
+ *
+ * A database which would not say what is standing gets a line of its own, whatever the clear
+ * dropped. What was left behind is exactly what could not be found out there, so it is no more a
+ * clear that left nothing than one that left something, and the count of what it did drop is the
+ * only thing that can still be stated: reporting it through the line above would say "the clear
+ * dropped no table at all" of a clear that dropped a dozen.
+ *
+ * A row of the catalog the read passed over is reported wherever the clear got to, that line
+ * depending on nothing this database was asked afterwards: what such a row records is outside the
+ * namespace {@link #leftoverTables} scans, so no other line here can name it. The row itself does
+ * not survive the clear - the catalog names itself last and the loop drops that table with every
+ * row still in it - which is why the line is the only surviving copy of what the row said, and
+ * why it names what the row recorded rather than telling an operator to go and look. Nothing this
+ * version writes makes such a row - {@link #getTableName} names every table {@code opendj_
+ * It is a term of the "dropped nothing" line all the same, and for one state only: a catalog whose
+ * table is there names itself, so a clear reading any row at all normally drops that one and the
+ * term is carried by the drop count beside it. Where it is not is where the catalog table went
+ * between the read of its rows and the loop that drops them - another process clearing the same
+ * backend - and there the clear has read a row, dropped nothing, and has this row as the whole of
+ * what it can say. Without the term it says nothing at all, which is the silence of #888.
+ */
+ void reportClearOutcome(Connection con, TableScope scope, int dropped, int droppedTrees, int missingTrees,
+ List
+ * The row is gone by the time this prints and what it recorded is not: the catalog names itself
+ * last, so the loop drops the table holding these rows along with every other - and where that
+ * table went on its own between the two lookups, it took them with it just the same. That is what
+ * the line has to say, and why it carries the recorded name rather than sending an operator to a
+ * table that is no longer there.
+ */
+ private void reportSkippedRows(List
+ * A read the database refused is passed to the caller rather than answered as an absent stamp: the
+ * two say different things, and the second would let a connection that died halfway be reported as
+ * a row of tables nothing can be said about. An engine with no readback of its own is the same
+ * distinction one step earlier, and is answered by the caller: it puts every table of such an
+ * engine where nothing was asked of it belongs, which is not where a table without a stamp goes.
+ */
+ private TreeName stampedTree(Connection con, Dialect dialect, String tableName) throws SQLException {
+ final String comment=readStoredComment(con, dialect, tableName);
+ if (comment==null || comment.isEmpty()) {
+ return null;
+ }
+ try {
+ return TreeName.valueOf(comment);
+ } catch (RuntimeException e) { // a comment of somebody else's making: no stamp of this backend's kind
+ return null;
+ }
+ }
+
+ /**
+ * The base DN the compressed schema trees of this backend are named under since #881, spelled out
+ * here for the reason {@link #SHARED_COMPRESSED_SCHEMA_TREES} is: the prefix is built by a private
+ * method of {@code PersistentCompressedSchema}, escapes and all. A table stamped with one of these
+ * carries this backend's id in plain text, so a clear that finds one standing can say whose it is.
+ */
+ private String ownCompressedSchemaBaseDN() {
+ return SHARED_COMPRESSED_SCHEMA_BASE_DN+"_"+escapedBackendId();
+ }
+
+ /**
+ * The backend id as one component of a tree name. A tree name is {@code /
+ * The database is the half that has to narrow. Asked with a null catalog the question spans the
+ * whole server on some drivers - Connector/J reads a null catalog as "any database" since 8.0, and
+ * its databaseTerm being CATALOG it ignores the schema pattern besides - and every answer of such a
+ * lookup decides something a table of the same name in another database must have no say in. A
+ * clear skips the row of a table that is gone so that it can go on, and a foreign table answering
+ * for it turns that skip into an unqualified "drop table" of a table that is not in this database,
+ * failing the clear on this attempt and on every attempt after it. An open of a tree creates its
+ * table where there is none, and a foreign table answering for it skips the creation, leaving the
+ * catalog naming a tree whose table is not here. Two backends of the stock backend id in two
+ * databases of one server name their tables alike, so this is the ordinary layout and not a corner
+ * of one.
+ *
+ * The schema is the half that must not narrow to one name. The statements this scope guards are
+ * unqualified, and an unqualified name resolves across a path of schemas: the whole
+ * {@code search_path} on postgresql, the default schema of the user and then {@code dbo} on sql
+ * server. A lookup narrowed to {@code current_schema()} alone would be the stricter question of the
+ * two - an installation whose tables were created in {@code public} while the connection now works
+ * in a schema of its own reads and writes them unqualified all the same, and asking only about that
+ * schema would report them absent: the clear would drop nothing, which is #888 over again, and the
+ * next open would create a second, empty set of tables shadowing the populated ones for every later
+ * unqualified reference. The path is asked of the connection, so that a lookup answers for exactly
+ * the tables the statements behind it reach - no more and no fewer.
+ */
+ static final class TableScope {
+ /** The database of the connection, or {@code null} where the driver names none - oracle has none. */
+ final String catalog;
+ /**
+ * The schemas an unqualified name of this connection resolves in, nearest first, or {@code null}
+ * where the schema is no dimension of this engine - mysql, whose schema is its database - or
+ * where the connection would not say. A null path narrows nothing, which is the question this
+ * class asked before there was anything to narrow it by.
+ */
+ final List
+ * The row is written whenever the catalog does not already record this tree at this table -
+ * and not only when the table is created - so that a backend of an installation upgraded to a
+ * version keeping a catalog fills it in at its first read-write open instead of waiting for
+ * its trees to be created again. What the catalog already records is read once, when this
+ * storage first opens it; see {@link #enrolledTrees}.
+ *
+ * The row is written on a connection of the catalog's own and committed there, never on the one
+ * this transaction runs on. It has to be committed: the open which fills the catalog of a
+ * backend upgraded from a version keeping none creates no table at all, so there is nothing
+ * else of {@link #openTree} to carry those rows, and a transaction failing after them would
+ * take every one back - leaving the tables named by nothing and the next clear dropping
+ * nothing, which is #888 over again. And that commit must not be this transaction's:
+ * {@code RootContainer.open()} opens every tree of every base DN in a single write, a commit
+ * anywhere inside it takes the whole write out of the replay - {@link #replayReason} reads
+ * {@link #partlyCommitted} before it asks anything else - and a deadlock at the twentieth tree
+ * would then fail the backend open where master replayed it. A connection of its own is what
+ * gives the row a commit that is not the caller's.
+ */
+ void enrolInCatalog(TreeName treeName) {
+ final TreeName catalog=getCatalogTree();
+ if (catalog.equals(treeName)) {
+ return; // the catalog holds no row of its own: catalogTables() adds it when its table is there
+ }
+ if (SHARED_COMPRESSED_SCHEMA_BASE_DN.equals(treeName.getBaseDN())) {
+ return; // a tree this backend may not be the only owner of: see the constant
+ }
+ openCatalog(catalog);
+ if (enrolledTrees.contains(treeName)) {
+ return; // already recorded, at the table this open would record it at
+ }
+ try {
+ catalogSession.transaction().upsert(catalog,
+ ByteString.valueOfUtf8(treeName.toString()),
+ ByteString.valueOfUtf8(getTableName(treeName)));
+ // committed where it is written, so that the row is there before the table on every
+ // engine and not only where the "create table" below happens to carry it - and on the
+ // catalog's own connection, so that this commit is none of the caller's: see above
+ catalogSession.commit();
+ enrolledTrees.add(treeName);
+ } catch (SQLException | RuntimeException e) {
+ // the unchecked one as well, exactly as unenrolFromCatalog() takes it: upsert() answers a
+ // failed statement with a StorageRuntimeException of its own, and what that statement left
+ // behind has to be rolled back all the same. This connection outlives the row that failed
+ // on it and carries every remaining tree of this open - postgres refuses every further
+ // statement of a transaction whose statement failed (25P02), so a reset skipped here fails
+ // the twenty-odd enrolments behind it with a cause nowhere near the one that started it
+ catalogSession.reset();
+ throw e instanceof StorageRuntimeException ? (StorageRuntimeException) e : new StorageRuntimeException(e);
+ }
+ }
+
+ /**
+ * The connection the catalog is read and written on, established where this transaction has
+ * not established it yet.
+ *
+ * The unchecked failure of the connect is taken like the checked one, the way every other
+ * catalog path of this class takes it: {@link JDBCStorage#newCatalogConnection} hands on a
+ * driver's unchecked answer to a connect it will not make as the unchecked failure it is, so a
+ * catch of {@code SQLException} alone would let that one past unwrapped and without the line
+ * saying which connection of this backend could not be made.
+ */
+ Connection catalogConnection() {
+ try {
+ return catalogSession.connection();
+ } catch (SQLException | RuntimeException e) {
+ throw e instanceof StorageRuntimeException ? (StorageRuntimeException) e
+ : new StorageRuntimeException("jdbc: backend "+config.getBackendId()
+ +" could not open the connection its tree catalog is read and written on", e);
+ }
+ }
+
+ /**
+ * Makes the catalog of this backend usable, once per open of the storage: its table is created
+ * where there is none, and what it already records is read where there is one.
+ *
+ * Serialized on the storage, so that two transactions opening trees at the same time cannot both
+ * find the table absent and both go on to create it. It serializes this storage and nothing
+ * else, which is why the create tolerates a table that turned up while it was being made: an
+ * offline tool beside a running server is a pair no lock of one process can order. The stamp -
+ * the one thing under it that is nobody's dependency - is issued outside it.
+ *
+ * Two things are kept out of the lock because they are the slow ones. The flag is read before
+ * it is taken at all, which is every {@code openTree} of this storage but the first few: it is
+ * volatile and it is raised after {@link #enrolledTrees} has been filled, so a reader that sees
+ * it up sees that memo whole. And the connection of the catalog is established before it: that
+ * connect retries a database taking no connection for the moment for up to the deadline of a
+ * borrow (see {@link JDBCStorage#newCatalogConnection}), and made under the lock it would hold
+ * every other transaction of this storage that goes on to open a tree for the whole of that
+ * wait - a queue the borrows of the pool, each waiting on its own thread, never form.
+ *
+ * What that costs is a connect to every transaction which finds the flag down and then loses the
+ * race for the lock. The loser gives that connection up rather than hold it: the winner has
+ * filled {@link #enrolledTrees}, so the caller is about to find its tree recorded and write
+ * nothing at all, and the session is lazy - the rarer loser that does have a tree to enrol opens
+ * another. The race is for the first openTree of a storage, so what this can cost against a
+ * database with no connection to give is one refused login per racing transaction, where the
+ * connect made under the lock cost one and made the others wait out the same refusal in turn.
+ */
+ void openCatalog(TreeName catalog) {
+ if (catalogTableOpened) {
+ return;
+ }
+ // what this call established, and not what the transaction was already holding: deleteTree()
+ // opens the session before it drops anything, so a later openTree of the same transaction
+ // must not give away a connection it did not make
+ final boolean established=!catalogSession.isEstablished();
+ catalogConnection();
+ final boolean lostTheRace;
+ synchronized (catalogLock) {
+ lostTheRace=catalogTableOpened;
+ if (!lostTheRace) {
+ if (isExistsTable(catalog)) {
+ readEnrolledTrees(catalog);
+ } else {
+ createCatalogTable(catalog);
+ // nothing to read from a table that has just been created, and nothing this open
+ // enrols may be skipped as already recorded
+ }
+ catalogTableOpened=true;
+ }
+ }
+ if (lostTheRace) {
+ if (established) {
+ // the winner has filled enrolledTrees, so the caller is about to find its tree recorded
+ // and write nothing: the connection this call made is given up rather than held idle for
+ // the rest of the transaction, and the session being lazy, the rarer loser that does
+ // have a tree to enrol opens another. Outside the lock, for the reason the connect is:
+ // a close is a round trip of its own, and against a database that has stopped answering
+ // it does not return at all - connectCatalog() lifts the read bound of the login on
+ // every connection it hands back, so there is no bound of ours left to end this one
+ catalogSession.close();
+ }
+ return;
+ }
+ // stamped with its tree name like any table of a tree (#866), and for a reason of its own: a
+ // clear reports what it did not drop, and the catalog of a backend sharing this database
+ // (#873) is the one table such a report could otherwise attribute to nobody. It costs one
+ // stamp per open of the storage, not one per tree - the flag above is what keeps it to one -
+ // and it is issued outside the lock: it is a diagnostic aid on a session and a bound of its
+ // own, with no business holding up every openTree of this storage
+ commentTable(catalog, dialectOf(con), stampSession);
+ }
+
+ /**
+ * Reads what the catalog already records, so that the trees it names are not enrolled again on
+ * an open which would write the rows that are already there; see {@link #enrolledTrees}. Run
+ * once per open of the storage, behind the very flag that keeps the catalog from being opened
+ * again, and it costs the one select a clear pays for anyway.
+ *
+ * Read on the catalog's own connection and committed there, so that no transaction of a caller
+ * ever touches the catalog table. A select of the caller's transaction would hold a lock on it
+ * until that transaction ended - the whole of {@code RootContainer.open()} - and the rows this
+ * read decides are written on the catalog's connection: a clear of this backend queueing for the
+ * table in between would then be waiting for the caller while the caller waited for it, a pair
+ * of sessions no deadlock detector of the database can see, one of them being blocked inside
+ * this process rather than in the server. {@link #removeStorageFiles()} and {@link #listTrees()}
+ * read that table on a connection of their own, which is the same argument read the other way:
+ * neither is inside a transaction of a caller, and both are done with it when they commit.
+ */
+ void readEnrolledTrees(TreeName catalog) {
+ try {
+ final Connection catalogCon=catalogSession.connection();
+ for (final Map.Entry
+ * A read-write open of a JDBC backend needs the privilege to create this table, where a version
+ * keeping no catalog issued no DDL at all on an installation whose tables were already there.
+ * An account that may write its rows but not create a table is a configuration this can meet,
+ * so the failure says which table it was and why the backend wanted it, rather than reaching
+ * the operator as a bare SQL error inside ERR_OPEN_ENV_FAIL.
+ */
+ void createCatalogTable(TreeName catalog) {
+ final String tableName=getTableName(catalog);
+ try {
+ final Connection catalogCon=catalogSession.connection();
+ try (final PreparedStatement statement=catalogCon.prepareStatement("create table "+tableName+" ("+getTableDialect()+")")) {
+ // bulk like every other create table of this backend (#882): it is DDL nobody waits on,
+ // and the class of a client operation is not what a statement of this kind can be given
+ execute(statement, StatementBound.BULK);
+ }
+ catalogCon.commit();
+ } catch (SQLException | RuntimeException e) {
+ // the unchecked one as well, for the reason enrolInCatalog() takes it: what the statement
+ // left behind has to be rolled back whatever class the failure arrived in, this connection
+ // being the one the rows of this open are written on
+ catalogSession.reset();
+ // a table that turned up between the lookup and this statement is what was wanted, whoever
+ // made it: the lock this runs under orders the transactions of one storage, and an offline
+ // tool beside a running server - the pair #888 is about - is ordered by nothing at all.
+ // The lookup is asked inside a catch and must not become the answer: it goes to the
+ // database on the caller's connection, which is often the very thing that has just failed,
+ // and it reports its own failure as a StorageRuntimeException - thrown from here it would
+ // replace the create failure below with a bare metadata error saying nothing about the
+ // catalog. So a lookup that will not answer is carried by the failure it could not settle.
+ boolean alreadyThere;
+ try {
+ alreadyThere=isExistsTable(catalog);
+ } catch (RuntimeException lookup) {
+ e.addSuppressed(lookup);
+ alreadyThere=false;
+ }
+ if (alreadyThere) {
+ logger.debug(LocalizableMessage.raw("jdbc: table %s was created by another session while this one was creating it: %s",
+ tableName, stackTraceToSingleLineString(e)));
+ return;
+ }
+ throw new StorageRuntimeException("jdbc: backend "+config.getBackendId()+" could not create table "
+ +tableName+", which holds the catalog naming the trees it owns: a read-write open of a JDBC"
+ +" backend needs the privilege to create it, and a clear of one names nothing without it", e);
+ }
+ }
+
+ /**
+ * Takes the tree out of the catalog: a row is what puts a table up for removal, and this one is
+ * gone. Written and committed on the catalog's own connection, like the enrolment - see {@link
+ * #enrolInCatalog} - which is what keeps the caller's transaction from being able to roll it
+ * back over a table that is already dropped.
+ */
+ void unenrolFromCatalog(TreeName treeName, boolean enrolled) {
+ final TreeName catalog=getCatalogTree();
+ if (catalog.equals(treeName)) {
+ catalogTableOpened=false; // its own table is gone: the next enrolment creates it again
+ enrolledTrees.clear(); // and records every tree anew, this one having recorded nothing
+ return;
+ }
+ if (SHARED_COMPRESSED_SCHEMA_BASE_DN.equals(treeName.getBaseDN())) {
+ // the symmetry of enrolInCatalog() and nothing more: no row of this pair was ever written,
+ // so the delete would find none. What keeps the pair out of a clear is that a clear drops
+ // what the catalog names and the catalog does not name them; see the constant
+ return;
+ }
+ if (!enrolled) {
+ // no row to delete - the catalog table is not there at all - and nothing to order this
+ // against: a tree the catalog does not name is not one an enrolment may skip
+ enrolledTrees.remove(treeName);
+ return;
+ }
+ try {
+ // deleteRow() and not delete(): the read-only check belongs to the caller of deleteTree,
+ // which made it, and the transaction this row is written through is one of this class's own
+ catalogSession.transaction().deleteRow(catalog, ByteString.valueOfUtf8(treeName.toString()));
+ catalogSession.commit();
+ } catch (SQLException | RuntimeException e) {
+ // the unchecked one as well: deleteRow() answers a failed statement with a
+ // StorageRuntimeException, and what that statement left behind has to be rolled back all
+ // the same - this connection outlives the row that failed on it
+ catalogSession.reset();
+ throw e instanceof StorageRuntimeException ? (StorageRuntimeException) e : new StorageRuntimeException(e);
+ } finally {
+ // taken out of what this storage knows the catalog records after the delete and never
+ // before it, so that the memo and the catalog never disagree in the direction that
+ // makes an enrolment write a row a committed delete then takes back out. After the
+ // attempt whatever became of it: a delete that failed leaves a row the next enrolment
+ // has to write again rather than skip as already recorded, which costs an upsert of a
+ // row that is already there and no more.
+ //
+ // What no ordering of these two lines can do is order this against an openTree of the
+ // very same tree on another thread, and it is worth saying which fix was ruled out
+ // rather than leaving it to be proposed again. Such an openTree landing between the
+ // commit above and this line skips its enrolment - the memo still names the tree - and
+ // goes on to create the table, leaving a table nothing names; the ordering before this
+ // one reached the same end state by the other route, the enrolment writing a row this
+ // delete then removed. A lock over the memo and the row closes neither, since the
+ // table is created and dropped outside it either way: only a lock held across the DDL
+ // of both would, and that one deadlocks. A transaction holding catalogLock and blocked
+ // in the database on a "drop table" of a tree a second transaction of this storage is
+ // still writing would be waiting for that transaction, while it waited for the lock at
+ // its next openTree - a cycle the database cannot see, where today it is a plain wait
+ // that ends when the second transaction does.
+ //
+ // So the catalog is consistent given that no two transactions open and delete the same
+ // tree at once, and that is the layer above's to keep: a tree is opened read-write and
+ // deleted from the configuration framework, which orders the changes of one entry, or
+ // from EntryContainer.clear() with the backend disabled.
+ enrolledTrees.remove(treeName);
+ }
+ }
+
+ /** Whether a delete of this tree has a row of the catalog to take out; see {@link #unenrolFromCatalog}. */
+ boolean isEnrolledTree(TreeName treeName) {
+ if (getCatalogTree().equals(treeName) || SHARED_COMPRESSED_SCHEMA_BASE_DN.equals(treeName.getBaseDN())) {
+ return false;
+ }
+ return catalogTableOpened || isExistsTable(getCatalogTree());
+ }
+
+ /**
+ * Whether the table already carries the index of this name, asked where the table itself is
+ * asked for - see {@link TableScope}. A table name carries no backend id and no database, so two
+ * databases of one server hold identical table and index names, and Connector/J 8 binds
+ * no schema predicate for a null catalog: a neighbouring database answering here would skip the
+ * create index of this one for good, leaving every "where k>? order by k" batch of every cursor
+ * a full scan behind it.
+ */
boolean isExistsIndex(String tableName, String indexName) throws SQLException {
+ final TableScope scope=takeTableScope();
+ // the index lookup takes the operation bound of #882 like every other catalog read of this
+ // class: it asks a data dictionary rather than the data, so a wait here is the metadata lock
+ // of another session - and it is narrowed to the scope every table lookup here is narrowed to
return bounded(con, StatementBound.OPERATION, () -> {
// approximate=true: with false the oracle driver runs ANALYZE on every call
- try (final ResultSet rs = con.getMetaData().getIndexInfo(null, null, tableName, false, true)) {
+ try (final ResultSet rs = con.getMetaData().getIndexInfo(scope.catalog, null, tableName, false, true)) {
while (rs.next()) {
- if (indexName.equalsIgnoreCase(rs.getString("INDEX_NAME"))) {
+ if (indexName.equalsIgnoreCase(rs.getString("INDEX_NAME")) && scope.covers(rs)) {
return true;
}
}
@@ -2280,6 +3816,34 @@ public void clearTree(TreeName treeName) {
@Override
public void deleteTree(TreeName treeName) {
checkReadOnly();
+ // The row is taken out on the catalog's own connection rather than left to this transaction:
+ // that transaction is the last thing the delete could still be rolled back by - write()
+ // replays a class 40 conflict and rethrows everything else unreplayed - and the row would be
+ // rolled back over a table that is already gone, with nothing ever to put it right: a deleted
+ // tree is not opened again, so no enrolment and no unenrolment reaches it a second time. It
+ // holds for the branch where there is no table to drop as much as for the one where the drop
+ // commits of its own accord.
+ // That connection is opened here, before anything is dropped: a connect this backend cannot
+ // make costs nothing at this point, where one failing after the drop would leave exactly the
+ // half-done state the sentence above is about.
+ final boolean enrolled=isEnrolledTree(treeName);
+ if (enrolled) {
+ catalogConnection();
+ }
+ // The table dropped is the one the tree names, where a clear drops the one its row records.
+ // The two are the same table by the time anything is deleted: a row recording another one is
+ // not taken as an enrolment - readEnrolledTrees() keeps it out of enrolledTrees - so the
+ // openTree that every delete of a tree comes after has rewritten it to this name.
+ // A row is written before its table is created and taken out after its table is dropped,
+ // never the other way round: of the two ways a half-done change can end, a catalog naming a
+ // table that is not there is the one the removal is ready for - it skips such a row and says
+ // so - while a table nothing names is adopted with its stale rows by the next open of that
+ // tree and is dropped by no clear ever after. So this is deliberately not the mirror of
+ // openTree(): an unenrolment left pending before the drop would be committed by the drop
+ // itself on mysql and oracle, where DDL commits the transaction it finds open before it
+ // executes, and would then stand even where the drop goes on to fail - ORA-00054 on a tree
+ // another session holds, say, which write() does not replay, it being neither a class 40
+ // state nor ORA-00060.
if (isExistsTable(treeName)) {
try {
commitStatement("drop table " + getTableName(treeName), true);
@@ -2287,7 +3851,8 @@ public void deleteTree(TreeName treeName) {
throw new StorageRuntimeException(e);
}
}
- // forget the mapping so listTrees() consumers (updateTableStatistics) skip the dropped table
+ unenrolFromCatalog(treeName, enrolled);
+ // the memoized table name of a tree nothing holds any more is of no use to anyone
tree2table.invalidate(treeName);
unstampableTrees.remove(treeName); // a table recreated later deserves a fresh stamp attempt
}
@@ -2382,6 +3947,16 @@ public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f) {
@Override
public boolean delete(TreeName treeName, ByteSequence key) {
checkReadOnly();
+ return deleteRow(treeName, key);
+ }
+
+ /**
+ * The statement of {@link #delete} without its read-only check, for the rows this class writes
+ * on a transaction of its own making: the catalog of a backend is written through a transaction
+ * over a connection of its own, whose access mode is read again as it is built, and the check
+ * that matters was made by the caller of {@code openTree} or {@code deleteTree}.
+ */
+ boolean deleteRow(TreeName treeName, ByteSequence key) {
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()));
@@ -2523,8 +4098,11 @@ public void delete() throws NoSuchElementException, UnsupportedOperationExceptio
throw new UnsupportedOperationException();
}
if (writeTableName==null) {
- // the enrolling name, unlike the read statements above: this writes to the tree, so
- // it is one this backend owns, and removeStorageFiles() has to know about it
+ // the enrolling name, unlike the read statements above: this writes to the tree, so it is
+ // one this backend owns and its table belongs in the memo of the storage. What a clear
+ // drops is what the catalog of the backend names (#888), and openTree(name, true) is the
+ // one thing that writes there - a tree written through a cursor is one the backend opened
+ // to get the cursor, which is where its row comes from
writeTableName=getTableName(treeName);
}
try (final PreparedStatement statement=con.prepareStatement("delete from "+writeTableName+" where h="+hashParam(con)+" and k=?")){
@@ -2637,9 +4215,185 @@ public boolean positionToIndex(int index) {
}
}
+ /**
+ * {@inheritDoc}
+ *
+ * Answered from the catalog of the backend rather than from the trees this process happens to
+ * have touched: {@link #removeStorageFiles()} runs before anything has touched one (#888).
+ *
+ * What a tool has to be shown is not what a clear may drop: the shared compressed schema trees
+ * are deliberately not enrolled - a backend must not offer a tree another one may own for removal
+ * - and would go unnamed by {@code dbtest} for it, so they are added here when their tables are
+ * there. {@link #catalogTables(Connection, TableScope)} is what the removal reads, and it
+ * names them not.
+ *
+ * The catalog itself is among the names, being a tree of this backend like any other: {@code
+ * dbtest list-raw-dbs} counts it and {@code dump-raw-db} resolves its name, which is the one way
+ * of seeing from outside the server what a clear of this backend would drop.
+ */
@Override
public Set
+ * The table name is taken from the row rather than recomputed from the tree name, so that a
+ * removal drops what was enrolled even if the naming of tables were ever to change.
+ *
+ * The scope is the caller's rather than asked for here: it is not free of a round trip - pgjdbc
+ * answers both halves of it with a select of its own - and a clear and a listTrees() both narrow a
+ * lookup of their own by it, so they pass what they have instead of every reader asking twice over.
+ */
+ Map
+ * A row this backend cannot have written is skipped and reported rather than trusted. What a clear
+ * drops is the table a row records, dropped by that name, so a row recording something outside the
+ * namespace this backend names its tables in points at a table that is nobody's business of this
+ * one's - and a row naming no tree at all, or naming one that is not a tree name, would otherwise
+ * fail every clear from here on rather than the one thing it describes.
+ *
+ * Every row passed over is described into {@code skippedRows}, the warn above being addressed to
+ * whoever is reading the log at that moment and this to the account a clear gives of itself: such
+ * a row is a tree the clear cannot see, so what the row records is dropped by nothing - while the
+ * row itself goes with the catalog table it sits in, which the clear names last and drops. That is
+ * what makes the line the only surviving copy of what such a row said, and why it carries the
+ * recorded name. A reader with nobody to tell - a read of {@code dbtest}, or the one an enrolment makes
+ * - hands in a list of its own and lets it go, which is one allocation per read of a whole table
+ * and no convention to get wrong.
+ */
+ Map
+ * No database is needed for any of it. The url is a postgresql one the pgjdbc driver cannot parse,
+ * so {@code DriverManager} falls through to the probe of this class, while {@code
+ * CachedConnection.ConnectDialect} still reads it as postgres - which is what makes the connect fill
+ * in bounds at all, and what a url of an engine of nobody's would not.
+ */
+@SuppressWarnings("javadoc")
+public class CatalogConnectionTestCase extends DirectoryServerTestCase {
+
+ /**
+ * What a caller with no replay above it hands the connect: the importer is the one such caller in
+ * the product, and every case here that is not about the window itself asks the way it asks, so
+ * that what it pins is the property and not a window of the test's own.
+ */
+ private static final long NO_REPLAY_WINDOW = Long.MAX_VALUE;
+
+ private ProbeDriver probeDriver;
+
+ @BeforeClass
+ public void registerProbeDriver() throws SQLException {
+ probeDriver = new ProbeDriver();
+ DriverManager.registerDriver(probeDriver);
+ }
+
+ @AfterClass(alwaysRun = true)
+ public void deregisterProbeDriver() throws SQLException {
+ if (probeDriver != null) {
+ DriverManager.deregisterDriver(probeDriver);
+ }
+ }
+
+ /**
+ * Nothing of one case reaches the next: the probe is a field of the class and a case that fails
+ * before its finally would otherwise leave its stubbed connection, its refusals or the properties
+ * of its last attempt to be read by whatever runs after it.
+ */
+ @BeforeMethod
+ public void resetProbe() {
+ probeDriver.lastProperties = null;
+ probeDriver.answer = null;
+ probeDriver.refusal = null;
+ probeDriver.refusalsLeft.set(0);
+ probeDriver.attempts.set(0);
+ probeDriver.refusalDelayMs = 0;
+ probeDriver.interruptOnAttempt = false;
+ }
+
+ private static JDBCStorage storageFor(String url) {
+ final JDBCBackendCfg cfg = mockCfg(JDBCBackendCfg.class);
+ when(cfg.getBackendId()).thenReturn("catalogProbe");
+ when(cfg.getDBDirectory()).thenReturn(url);
+ return new JDBCStorage(cfg, null);
+ }
+
+ /**
+ * The bound of the connect is the one the operator configured for this backend's connects, and
+ * not a literal of the code: a deployment whose login legitimately takes longer than the default
+ * raises {@link CachedConnection#CONNECT_TIMEOUT_PROPERTY} for it, and a catalog connect bounded
+ * tighter than that fails in 08001 - which is no conflict a write replays, so the backend stops
+ * opening on an installation that opened before this connection existed.
+ *
+ * Asked with no deadline over it, so that what the case pins is the configured bound alone: what
+ * a deadline does to it is the case below.
+ */
+ @Test
+ public void testTheCatalogConnectTakesTheConfiguredBound() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "120");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ try {
+ probeDriver.lastProperties = null;
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW).close();
+ assertBoundedAt(probeDriver.lastProperties, 120);
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * One attempt is never left to run past the deadline the retry of this connect is given, which is
+ * the deadline of a borrow ({@link CachedConnection#POOL_TIMEOUT_PROPERTY}): an attempt bounded
+ * looser than what is left of it would overrun it by a whole connect timeout. The pool bounds its
+ * own attempts by exactly this rule, and a connect established the way a pooled one is takes it.
+ */
+ @Test
+ public void testTheCatalogConnectIsNeverBoundedPastTheDeadlineOfItsRetry() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "120");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "20");
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW).close();
+ // a range and not the 20 exactly: the bound is what is left of the deadline when the attempt
+ // is made, so a pause of a second anywhere before it - a collection, the first touch of a
+ // class on a loaded box - makes it 19, and the case is about the deadline and not the clock
+ assertBoundedWithin(probeDriver.lastProperties, 15, 20);
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A database taking no connection for the moment - at its connection limit, or still
+ * recovering - is waited out rather than reported: one attempt loses a race the borrow beside it
+ * wins, and this connect is on the critical path of the first read-write open of every backend.
+ */
+ @Test
+ public void testTheCatalogConnectWaitsOutADatabaseTakingNoConnectionForTheMoment() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ // both, since both are read on every connect: an ambient connect bound would change the
+ // per-attempt bound these cases run under without changing anything they assert on
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "60");
+ probeDriver.refusal = new SQLException("too many clients already", "53300");
+ probeDriver.refusalsLeft.set(2);
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW).close();
+ assertEquals(probeDriver.attempts.get(), 3,
+ "a connect refused for the moment was not retried the way a borrow of the pool retries it");
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * And everything else is the caller's to see rather than waited out behind its back: a password
+ * that is not accepted does not become a minute of silence and then the same failure.
+ *
+ * A guard rather than a regression test, and worth saying so: the head before this connect had a
+ * retry made one attempt and reported it, so it satisfies this case by having no loop at all.
+ * What the case is here for is the loop that does exist staying this narrow - a predicate widened
+ * to any refusal turns a wrong password into a minute of silence per open, and nothing else in
+ * this suite would notice.
+ */
+ @Test
+ public void testTheCatalogConnectDoesNotRetryAFailureThatWillNotClear() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ // both, since both are read on every connect: an ambient connect bound would change the
+ // per-attempt bound these cases run under without changing anything they assert on
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "60");
+ probeDriver.refusal = new SQLException("password authentication failed", "28P01");
+ probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ fail("a connect that will not clear was retried instead of being reported");
+ } catch (SQLException expected) {
+ assertEquals(expected.getSQLState(), "28P01", "the failure of the driver was not the one reported");
+ assertEquals(probeDriver.attempts.get(), 1, "a failure that will not clear was attempted more than once");
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * The wait ends at the deadline of a borrow, as a timeout by type and carrying the state of the
+ * driver's own last refusal: a state of this code's making would be read by {@code write()} as a
+ * connection the database dropped, and the retry must change no classification.
+ */
+ @Test
+ public void testTheCatalogConnectGivesUpAtTheDeadlineOfABorrow() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ // both, since both are read on every connect: an ambient connect bound would change the
+ // per-attempt bound these cases run under without changing anything they assert on
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "1");
+ // a vendor code beside the state, since both are carried over: an oracle failure says what it
+ // is in the ORA number and not in the SQLState, so a timeout dropping the code would answer 0
+ // where the classifier reading it expects the driver's own
+ probeDriver.refusal = new SQLException("the database system is starting up", "57P03", 3113);
+ probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ fail("a connect refused for the whole deadline was not given up on");
+ } catch (SQLTimeoutException expected) {
+ // the state of the driver's own refusal and not one of this code's making: a manufactured
+ // 08001 is read by write() as a connection the database dropped, which would replay an
+ // attempt whose pooled connection is healthy and distrust the pool over it
+ assertEquals(expected.getSQLState(), "57P03",
+ "the failure the deadline ended carried another state than the driver's own");
+ assertEquals(expected.getErrorCode(), 3113,
+ "the failure the deadline ended dropped the vendor code of the driver's own");
+ assertNotNull(expected.getCause(), "the driver's own failure was not carried as the cause");
+ assertTrue(probeDriver.attempts.get() > 1,
+ "the deadline was reached without the connect having been retried at all");
+ // and it says which of the two bounds ran out, the property being the thing to raise only
+ // where the property is what ended the wait
+ assertTrue(expected.getMessage().contains(CachedConnection.POOL_TIMEOUT_PROPERTY + "=1s"),
+ "the timeout did not name the bound that ended it: " + expected.getMessage());
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * The wait may not outlast the replay window of the {@code write()} it runs inside: this loop
+ * runs within one attempt of that one, so a refusal waited out past the window reaches it with
+ * the window already spent and is thrown unreplayed - the retry would cost the caller the replay
+ * it had before there was a retry here at all. The shorter of the two bounds is the deadline of
+ * the wait; the bound of one attempt is not taken from it, and this case pins that too.
+ */
+ @Test
+ public void testTheCatalogConnectDoesNotOutlastTheReplayWindowOfItsCaller() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ // a minute, which is the default and six times the window of a write: the property is what
+ // this connect would wait out if the window did not reach it
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "60");
+ probeDriver.refusal = new SQLException("the database system is starting up", "57P03");
+ probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
+ final long startedAt = System.currentTimeMillis();
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(startedAt + 300);
+ fail("a connect refused past the replay window of its caller was not given up on");
+ } catch (SQLTimeoutException expected) {
+ final long waited = System.currentTimeMillis() - startedAt;
+ assertTrue(waited < 30_000,
+ "the connect waited " + waited + " ms, which is the pool timeout rather than the window above it");
+ assertTrue(probeDriver.attempts.get() > 1,
+ "the window was spent without the connect having been retried at all");
+ assertEquals(expected.getSQLState(), "57P03",
+ "the failure the window ended carried another state than the driver's own");
+ // the line has to send an operator to the right knob: raising the pool timeout moves
+ // nothing where the window of the write is the shorter bound
+ assertTrue(expected.getMessage().contains("replay window"),
+ "the timeout did not say which of the two bounds ended it: " + expected.getMessage());
+ // and one attempt keeps the bound the operator configured for a login of this database:
+ // the window decides how long it is worth retrying, not how long a login may take, and
+ // an attempt cut to what is left of the window is the connect dying where the pooled one
+ // beside it succeeds - the backend that stops opening
+ assertBoundedAt(probeDriver.lastProperties, CachedConnection.DEFAULT_CONNECT_TIMEOUT_SECONDS);
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A thread asked to stop is not answered by sleeping out the rest of a pool timeout: the flag is
+ * put back - every frame above this one reads it to decide whether to unwind - and the driver's
+ * own refusal is what the caller is told, the interrupt riding along with it so that a connect
+ * cut short by a shutdown is not read off the log as a database refusing connections.
+ */
+ @Test
+ public void testTheCatalogConnectReportsAnInterruptRatherThanSleepingPastIt() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "60");
+ probeDriver.refusal = new SQLException("the database system is starting up", "57P03");
+ probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
+ // raised inside the attempt rather than by another thread racing this one: the first backoff
+ // is a millisecond, and a sleep entered with the flag already up throws at once
+ probeDriver.interruptOnAttempt = true;
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ fail("a connect interrupted while it waited was not reported at all");
+ } catch (SQLException expected) {
+ assertFalse(expected instanceof SQLTimeoutException,
+ "an interrupt was reported as the deadline of the wait running out");
+ assertEquals(expected.getSQLState(), "57P03",
+ "the interrupt replaced the driver's own failure instead of riding along with it");
+ assertEquals(probeDriver.attempts.get(), 1, "the wait went on past the interrupt");
+ boolean carried = false;
+ for (final Throwable suppressed : expected.getSuppressed()) {
+ carried |= suppressed instanceof InterruptedException;
+ }
+ assertTrue(carried, "the interrupt was dropped rather than carried on the failure reported");
+ assertTrue(Thread.currentThread().isInterrupted(),
+ "the flag Thread.sleep() cleared was not put back, so nothing above can read it");
+ } finally {
+ Thread.interrupted(); // cleared here, or every case running after this one on this thread meets it
+ probeDriver.interruptOnAttempt = false;
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A connect of this backend that stalls says so in the log, and is throttled apart from the
+ * borrows of the pool that address the same database: the two stall for the same reason, and a
+ * borrow that reported a moment ago would otherwise silence the connect that is about to fail -
+ * which is the one of the two an operator has no other line about.
+ *
+ * The stall is a real one rather than a call of the formatter: the guard of the throttle passes
+ * only where a connect has been retrying for a second, so nothing under it is reached by a case
+ * whose refusals come back at once.
+ */
+ @Test
+ public void testAStallOfTheCatalogConnectIsThrottledApartFromTheBorrowsOfThePool() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "60");
+ // one refusal, slow enough that the connect has been retrying for longer than the guard of
+ // the throttle, and then a connection: what is asserted is the warning, not the failure
+ probeDriver.refusal = new SQLException("the database system is starting up", "57P03");
+ probeDriver.refusalsLeft.set(1);
+ probeDriver.refusalDelayMs = CachedConnection.STALL_WARNING_AFTER_MS + 100;
+ try {
+ // a url of its own: the throttle is keyed by url, and a case sharing one with another
+ // would read that one's stamp instead of its own
+ storageFor(ProbeDriver.STALL_URL).newCatalogConnection(NO_REPLAY_WINDOW).close();
+ final long now = System.currentTimeMillis();
+ final long longEnoughAgo = now - 2 * CachedConnection.STALL_WARNING_AFTER_MS;
+ // the connect reported its stall: the moment is filed, so the next one inside the interval
+ // is not due. Nothing else of this suite touches this url
+ assertFalse(CachedConnection.stallWarningDue(ProbeDriver.STALL_URL, "|tree catalog", longEnoughAgo, now),
+ "the connect stalled for longer than the guard and reported nothing");
+ // and a borrow of the pool on that very url is still due one of its own, which is the half
+ // of the throttle key that keeps the two waits from silencing each other
+ assertTrue(CachedConnection.stallWarningDue(ProbeDriver.STALL_URL, "", longEnoughAgo, now),
+ "a stall of this connect silenced the borrows of the pool addressing the same database");
+ } finally {
+ probeDriver.refusalDelayMs = 0;
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * Nothing configured is the default of the pool, which is what this connect used to take always.
+ * The deadline is pinned rather than left to its own default: the attempt takes the shorter of
+ * the two, so a pool timeout set anywhere - the surefire configuration, the environment, another
+ * suite - would otherwise decide what this case asserts.
+ */
+ @Test
+ public void testTheCatalogConnectTakesTheDefaultWhereNothingIsConfigured() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ try {
+ probeDriver.lastProperties = null;
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW).close();
+ assertBoundedAt(probeDriver.lastProperties, CachedConnection.DEFAULT_CONNECT_TIMEOUT_SECONDS);
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A property of 0 is the operator asking for no bound at all - the pool reads it that way - and a
+ * connect that bounded itself anyway would be answering a setting with the opposite of it. Nothing
+ * is handed to the driver then, and there is no read bound to lift once the login is through.
+ *
+ * Both properties, which is what the pool itself says leaves a connect unbounded: the deadline of
+ * the retry bounds the attempt where there is one, so turning the per-attempt bound off alone
+ * leaves the attempt bounded by what is left of that deadline - the case above.
+ */
+ @Test
+ public void testTheCatalogConnectIsUnboundedWhereTheOperatorTurnedTheBoundOff() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, "0");
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ try {
+ probeDriver.lastProperties = null;
+ final Connection con = storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ con.close();
+ assertNotNull(probeDriver.lastProperties, "no properties were handed to the driver at all");
+ assertTrue(probeDriver.lastProperties.isEmpty(),
+ "a connect the operator asked for no bound on was bounded anyway: " + probeDriver.lastProperties);
+ verify(con, never()).setNetworkTimeout(any(), anyInt());
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * What the connection is handed back for: rows of its own, committed where they are written. The
+ * read bound of the login is lifted as soon as the login is through (#872) - it is a bound of the
+ * connect and not of the statements of the catalog - and the isolation is the pool's, a repeatable
+ * read gap-locking a catalog two transactions enrol into.
+ */
+ @Test
+ public void testTheCatalogConnectionIsSetUpForItsRows() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ // the read bound is lifted only where the attempt was given one, and the attempt takes the
+ // shorter of the two properties: a deadline of 0 elsewhere would leave nothing to lift here
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ try {
+ final Connection con = storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ con.close();
+ verify(con).setAutoCommit(false);
+ verify(con).setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
+ verify(con).setNetworkTimeout(any(), eq(0));
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A driver that will not take the read bound of the login back does not cost the backend its
+ * catalog: the pool meets the same failure and hands the connection to the borrower waiting for
+ * it, and a connect failing where the pooled one beside it succeeds is a backend that stops
+ * opening on an installation which opened before this connection existed. Reported, and kept.
+ *
+ * A guard rather than a regression test, like the one above: the head before this round already
+ * caught that failure and returned the connection, so nothing of the round makes this case pass.
+ * It is here because the answer was reached for twice - once as "fail the connect", which this
+ * round took back out - and a third attempt at it would go unnoticed otherwise.
+ */
+ @Test
+ public void testTheCatalogConnectionIsKeptWhereTheReadBoundWillNotComeOff() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ final Connection keeping = mock(Connection.class);
+ doThrow(new SQLFeatureNotSupportedException("no network timeout here"))
+ .when(keeping).setNetworkTimeout(any(), anyInt());
+ probeDriver.answer = keeping;
+ try {
+ final Connection con = storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ assertNotNull(con, "a connection whose read bound would not come off was not handed back");
+ verify(con).setAutoCommit(false);
+ verify(con, never()).close();
+ } finally {
+ restore(previous);
+ restorePool(previousPool);
+ }
+ }
+
+ /**
+ * A connection whose set-up failed is held by nobody - the caller is answered with the failure -
+ * so it is closed here or it leaks for the life of the process, one per open of a storage.
+ */
+ @Test
+ public void testAConnectionWhoseSetUpFailsIsClosed() throws Exception {
+ final String previous = System.getProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ final String previousPool = System.getProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ // pinned like every other case of this class: both are read from the system properties on every
+ // connect, so an ambient value would have this case exercise another path than the one it names
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, "0");
+ final Connection failing = mock(Connection.class);
+ doThrow(new SQLException("no transaction here", "08006")).when(failing).setAutoCommit(false);
+ probeDriver.answer = failing;
+ try {
+ storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW);
+ fail("a connection this backend could not set up was handed to the catalog");
+ } catch (SQLException expected) {
+ // the failure of the set-up itself, reported to the caller rather than swallowed
+ } finally {
+ probeDriver.answer = null;
+ restore(previous);
+ restorePool(previousPool);
+ }
+ verify(failing).close();
+ }
+
+ /** What the dialect of this url declares, at the given number of seconds. */
+ private static void assertBoundedAt(Properties handed, long seconds) {
+ assertNotNull(handed, "no properties were handed to the driver at all");
+ final CachedConnection.ConnectDialect dialect = CachedConnection.ConnectDialect.of(ProbeDriver.URL);
+ assertNotNull(dialect, "the url of this test is read as an engine of nobody's, so it is bounded by nothing");
+ for (final String property : dialect.connectProperties) {
+ assertEquals(handed.getProperty(property), Long.toString(seconds * dialect.connectUnitsPerSecond),
+ property + " did not reach the driver at the configured bound");
+ }
+ assertEquals(handed.getProperty(dialect.readProperties[0]),
+ Long.toString(seconds * dialect.readUnitsPerSecond),
+ dialect.readProperties[0] + " did not reach the driver at the configured bound");
+ }
+
+ /** The same where the value is what is left of a deadline, which no case may pin to the millisecond. */
+ private static void assertBoundedWithin(Properties handed, long atLeastSeconds, long atMostSeconds) {
+ assertNotNull(handed, "no properties were handed to the driver at all");
+ final CachedConnection.ConnectDialect dialect = CachedConnection.ConnectDialect.of(ProbeDriver.URL);
+ assertNotNull(dialect, "the url of this test is read as an engine of nobody's, so it is bounded by nothing");
+ final String property = dialect.connectProperties[0];
+ final String handedValue = handed.getProperty(property);
+ assertNotNull(handedValue, property + " did not reach the driver at all");
+ final long seconds = Long.parseLong(handedValue) / dialect.connectUnitsPerSecond;
+ assertTrue(seconds >= atLeastSeconds && seconds <= atMostSeconds,
+ property + " reached the driver at " + seconds + "s, outside the deadline it is taken from ("
+ + atLeastSeconds + ".." + atMostSeconds + "s)");
+ }
+
+ private static void restore(String previous) {
+ if (previous == null) {
+ System.clearProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY);
+ } else {
+ System.setProperty(CachedConnection.CONNECT_TIMEOUT_PROPERTY, previous);
+ }
+ }
+
+ private static void restorePool(String previous) {
+ if (previous == null) {
+ System.clearProperty(CachedConnection.POOL_TIMEOUT_PROPERTY);
+ } else {
+ System.setProperty(CachedConnection.POOL_TIMEOUT_PROPERTY, previous);
+ }
+ }
+
+ /** Records the properties a catalog connection hands its driver, and connects to nothing. */
+ private static final class ProbeDriver implements Driver {
+ /**
+ * A postgresql url with a port that is not a number: pgjdbc cannot parse it and answers the
+ * DriverManager with null - or with a failure, which it records and walks past all the same -
+ * so this probe is the driver that ends up answering, while ConnectDialect still reads the
+ * prefix as postgres.
+ */
+ static final String URL = "jdbc:postgresql://catalog-probe:not-a-port/db";
+
+ /**
+ * The same for the one case about the stall warning, which reads the throttle this connect
+ * files its report in: that throttle is keyed by url, so a case sharing one with any other
+ * would be asserting on whichever of them ran first.
+ */
+ static final String STALL_URL = "jdbc:postgresql://catalog-probe-stall:not-a-port/db";
+
+ volatile Properties lastProperties;
+
+ /** The connection to answer with, for a test about what is done with it; a fresh mock otherwise. */
+ volatile Connection answer;
+
+ /** How many attempts to refuse before answering, and with what; for the cases about the retry. */
+ final AtomicInteger refusalsLeft = new AtomicInteger();
+ volatile SQLException refusal;
+
+ /** How long a refused attempt takes, for the one case that needs a wait the throttle counts. */
+ volatile long refusalDelayMs;
+
+ /**
+ * Whether an attempt raises the interrupt flag of the thread asking for it, so that the case
+ * about an interrupted wait does not have to race a backoff of one millisecond from outside.
+ */
+ volatile boolean interruptOnAttempt;
+
+ /** Every attempt this driver was asked to make, refused ones included. */
+ final AtomicInteger attempts = new AtomicInteger();
+
+ @Override
+ public Connection connect(String url, Properties info) throws SQLException {
+ if (!acceptsURL(url)) {
+ return null; // not ours: DriverManager goes on to the next driver
+ }
+ attempts.incrementAndGet();
+ lastProperties = info;
+ if (refusal != null && refusalsLeft.getAndDecrement() > 0) {
+ if (refusalDelayMs > 0) {
+ try {
+ Thread.sleep(refusalDelayMs);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ if (interruptOnAttempt) {
+ Thread.currentThread().interrupt();
+ }
+ throw refusal;
+ }
+ return answer != null ? answer : mock(Connection.class);
+ }
+
+ @Override
+ public boolean acceptsURL(String url) {
+ return url != null && (url.startsWith(URL) || url.startsWith(STALL_URL));
+ }
+
+ @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 java.util.logging.Logger getParentLogger() throws SQLFeatureNotSupportedException {
+ throw new SQLFeatureNotSupportedException();
+ }
+ }
+}
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 ee1e112d18..2c339a9ceb 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
@@ -41,6 +41,8 @@
import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Predicate;
import java.util.logging.Logger;
import static org.forgerock.i18n.LocalizableMessage.raw;
@@ -49,6 +51,7 @@
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@@ -96,6 +99,13 @@ public class JDBCStorageRetryTest extends DirectoryServerTestCase
/** The connection behind the pool of a test, so that a test can assert the statement it was asked to prepare. */
private Connection engineConnection;
+ /**
+ * The connection the tree catalog of a storage of this test is written on, so that a test can assert what was
+ * written there. Nothing else can: it is opened straight through the driver rather than borrowed from the pool,
+ * and the rows it carries are the ones {@code statements} is asserted never to have carried.
+ */
+ private Connection catalogConnection;
+
/**
* 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
@@ -443,6 +453,45 @@ public void testOpeningAnExistingTreeLeavesTheAttemptReplayable() throws Excepti
verify(statements, never()).executeUpdate();
}
+ /**
+ * The first read-write open of a backend upgraded from a version keeping no catalog creates the catalog and
+ * writes one row per tree, and creates no table of its own: every tree it opens is already there. None of that
+ * may commit anything of the caller's - {@code RootContainer.open()} opens every tree of every base DN in a
+ * single write, and a commit anywhere inside it takes the whole open out of the replay for the life of that
+ * attempt, so a deadlock at the twentieth tree would fail the backend start-up that master replayed. The rows
+ * still have to be committed, since nothing else of this open would carry them: a connection of the catalog's
+ * own is what makes the two compatible.
+ */
+ @Test
+ public void testFillingTheCatalogOfAnUpgradedBackendLeavesTheAttemptReplayable() throws Exception
+ {
+ // every table of this backend is there except the one the catalog is kept in, which is the shape of an
+ // installation whose tables predate the catalog
+ final AtomicReference
+ * PostgreSQL resolves an unqualified reference across the whole {@code search_path} while an unqualified
+ * {@code create} lands in {@code current_schema()} alone, so the two are the same schema only as long as
+ * nothing was put in front of the one the tables were made in. Adding a schema of its own to a role is the
+ * standard remedy since PG15 took {@code CREATE} off {@code public}, and it makes them differ on an
+ * installation whose tables are already there: the backend goes on reading and writing them unqualified,
+ * and a lookup asking only about {@code current_schema()} would report every one of them absent. What that
+ * would cost is this issue over again - the clear would drop nothing and say nothing, which is #888 - and
+ * one thing worse besides: the next open would create a second, empty set of tables in the schema ahead,
+ * and from that commit on they would shadow the populated ones for every later unqualified reference.
+ *
+ * The connection string carries the path rather than a role being altered, because the pools of this
+ * backend are keyed by it: a storage of another url is a storage of connections of its own, where an
+ * {@code ALTER ROLE} would leave every connection already pooled resolving the way it always did.
+ */
+ @Test
+ public void testAClearFindsATableOfAnotherSchemaOfTheSearchPath() throws Exception {
+ final TreeName tree = new TreeName("testSearchPath", "tree");
+ final String backendId = getBackendId() + "_searchPath";
+ // the tables of an installation made before anything was put in front of the schema they are in
+ final JDBCStorage created = new JDBCStorage(createBackendCfg(backendId), null);
+ final String tableName = created.getTableName(tree);
+ // the same backend, over connections resolving in a schema of its own first and in the one the
+ // tables are in behind it: what they reach unqualified is unchanged, what they create is not
+ final String aheadOfThem = getJdbcUrl() + "¤tSchema=" + AHEAD_ON_THE_PATH + ",public";
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(backendId, aheadOfThem), null);
+ try {
+ try {
+ created.open(AccessMode.READ_WRITE);
+ created.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+ } finally {
+ created.close();
+ }
+ assertTrue(isExistsTable(tableName), "the case did not make the table it is about");
+
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final Statement st = con.createStatement()) {
+ st.execute("create schema if not exists " + AHEAD_ON_THE_PATH);
+ }
+ storage.open(AccessMode.READ_WRITE);
+ try (final Connection con = DriverManager.getConnection(aheadOfThem)) {
+ // the fixture is the whole of the case: without this the two schemas are the same one and
+ // the assertions below hold of the version this case is about as well
+ assertEquals(con.getSchema(), AHEAD_ON_THE_PATH,
+ "the connections of this storage do not work in the schema put ahead of the tables");
+ assertNotEquals(con.getSchema(), "public", "the tables of this case are not in public after all");
+ }
+
+ assertTrue(storage.listTrees().contains(tree),
+ "a tree whose table this connection reads unqualified was named by none of them");
+
+ // the other half of what the narrowing decides, and the destructive one: openTree() creates a
+ // table where its lookup answers that there is none, and an unqualified "create table" lands in
+ // current_schema() - the schema ahead of the tables. A lookup asking about that schema alone
+ // would answer no here and leave the populated table in public orphaned behind a second, empty
+ // one, from this commit on. The clear below drops what the catalog names and would go on
+ // passing while it happened, which is why this is asserted here rather than left to it
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+ assertFalse(isExistsTableInSchema(AHEAD_ON_THE_PATH, tableName),
+ "the open created a second table in the schema ahead of the tables, shadowing the populated one");
+ assertTrue(isExistsTableInSchema("public", tableName),
+ "the open did not leave the populated table where it is");
+
+ storage.removeStorageFiles();
+
+ assertFalse(isExistsTable(tableName),
+ "the clear left a table it reaches unqualified standing, for living in another schema of the search path");
+ } finally {
+ // the same backend id, so this clears what either half of the case created - including the
+ // run where the clear under test drops nothing and the tables would otherwise be left for
+ // whatever case of this class runs next
+ clearQuietly(storage);
+ clearQuietly(new JDBCStorage(createBackendCfg(backendId), null));
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final Statement st = con.createStatement()) {
+ st.execute("drop schema if exists " + AHEAD_ON_THE_PATH + " cascade");
+ }
+ }
+ }
+
+ /**
+ * Whether the table is in that one schema, which is the question the case above asks and the one
+ * {@code TestCase.isExistsTable} cannot answer: it walks every schema the connection can see, so a
+ * table created in the wrong one of the two reads there exactly like a table created in the right
+ * one. Asked of {@code information_schema} with the schema and the name bound rather than through
+ * {@code getTables()}, whose schema is a pattern - {@code opendj_ahead} would match a schema named
+ * {@code opendjXahead} as readily, {@code _} being a single-character wildcard there.
+ */
+ private boolean isExistsTableInSchema(String schema, String tableName) throws SQLException {
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final PreparedStatement st = con.prepareStatement(
+ "select 1 from information_schema.tables where table_schema=? and lower(table_name)=lower(?)")) {
+ st.setString(1, schema);
+ st.setString(2, tableName);
+ try (final ResultSet rs = st.executeQuery()) {
+ return rs.next();
+ }
+ }
+ }
+
}
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 9cc4d976ad..7e4241b767 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
@@ -15,8 +15,10 @@
*/
package org.opends.server.backends.jdbc;
+import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.ByteStringBuilder;
+import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.server.config.server.JDBCBackendCfg;
import org.opends.server.backends.pluggable.PluggableBackendImplTestCase;
import org.opends.server.backends.pluggable.spi.AccessMode;
@@ -41,12 +43,16 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Properties;
+import java.util.Set;
+import java.util.TreeSet;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -121,12 +127,68 @@ protected Backend createBackend() {
@Override
protected JDBCBackendCfg createBackendCfg() {
+ return createBackendCfg(getBackendId());
+ }
+
+ /**
+ * A configuration of another backend on the database of this suite: backends sharing one database
+ * URL is a configuration nothing forbids, and what one of them clears must be its own tables.
+ */
+ protected JDBCBackendCfg createBackendCfg(String backendId) {
JDBCBackendCfg backendCfg = mockCfg(JDBCBackendCfg.class);
- when(backendCfg.getBackendId()).thenReturn(getBackendId());
+ when(backendCfg.getBackendId()).thenReturn(backendId);
when(backendCfg.getDBDirectory()).thenReturn(getJdbcUrl());
return backendCfg;
}
+ /**
+ * The same, reached over a connection string of the caller's own: the pools of this backend are
+ * keyed by it, so a case wanting connections established differently - in another schema of the
+ * search path, say - asks for them by asking for another url.
+ */
+ protected JDBCBackendCfg createBackendCfg(String backendId, String jdbcUrl) {
+ final JDBCBackendCfg backendCfg = createBackendCfg(backendId);
+ when(backendCfg.getDBDirectory()).thenReturn(jdbcUrl);
+ return backendCfg;
+ }
+
+ /**
+ * The same, serving the given base DN: what a clear compares the tree stamp of a table against
+ * when it says whether the table is this backend's own or another's (#866).
+ */
+ protected JDBCBackendCfg createBackendCfg(String backendId, DN baseDN) {
+ final JDBCBackendCfg backendCfg = createBackendCfg(backendId);
+ final TreeSet
+ * The two storages are two backends and not one addressing the same database, which is what the
+ * case is about: what a backend owns is recorded in a catalog named after its backend id and
+ * outlives the process that opened the tree, so a second storage of the same id would be shown
+ * the tree its own earlier open had enrolled - and would be right to be.
*/
@Test
public void testProbingATreeDoesNotPutItUpForRemoval() throws Exception {
@@ -334,9 +401,9 @@ public void run(WriteableTransaction txn) throws Exception {
});
owner.close();
- // a second storage on the same database, which never opened that tree - the shape of two
+ // a second backend on the same database, which never opened that tree - the shape of two
// backends addressing one database
- final JDBCStorage other = new JDBCStorage(createBackendCfg(), null);
+ final JDBCStorage other = new JDBCStorage(createBackendCfg(getBackendId() + "_probe"), null);
try {
other.open(AccessMode.READ_WRITE);
other.read(new ReadOperation
+ * The row is written and committed on a connection of the catalog's own, so this holds on every
+ * engine for the same reason: nothing the caller's transaction does - or fails to do - reaches it.
+ * On the branch before this one the row rode the caller's connection, and the case was green on
+ * postgres for a reason of that engine alone (openTree() asks there for the cursor index of every
+ * tree on every open and commits that, carrying the row with it) while the other three lost it.
+ */
+ @Test
+ public void testAReopenedTreeStaysInTheCatalogWhenItsTransactionFails() throws Exception {
+ final TreeName tree = new TreeName("testCatalogEnrolRollback", "tree");
+ final JDBCStorage setUp = new JDBCStorage(createBackendCfg(getBackendId() + "_enrol"), null);
+ try { // the tables of the backend, made by a storage that then goes away
+ setUp.open(AccessMode.READ_WRITE);
+ setUp.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+ } finally {
+ setUp.close();
+ }
+ // and the rest of what an installation upgraded to a version keeping a catalog holds: a
+ // catalog naming none of those tables
+ emptyTheCatalog(setUp.getTableName(setUp.getCatalogTree()));
+
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(getBackendId() + "_enrol"), null);
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ assertFalse(storage.listTrees().contains(tree), "the catalog of the case was not emptied");
+
+ try {
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ // the table is there, so this open creates none: the enrolment is the only thing
+ // this transaction has written when it fails
+ txn.openTree(tree, true);
+ // terminal, and no conflict for write() to replay: everything this transaction
+ // still owes goes back, and the row naming a standing table must not be part of it
+ throw new IllegalStateException("the transaction of an openTree failed");
+ }
+ });
+ fail("the write was expected to fail");
+ } catch (Exception expected) {
+ // what the case is about is what the failure left behind
+ }
+
+ assertTrue(storage.listTrees().contains(tree),
+ "the catalog forgot a tree whose table is standing: a clear of this backend would drop nothing");
+ } finally {
+ clearQuietly(storage);
+ }
+ }
+
+ /**
+ * A tree the catalog already records at the table this version records it at is not enrolled
+ * again by an open: the row would be the row that is already there. A row recording any other
+ * table is, though - a removal drops the table the row records, so a row naming one this backend
+ * would not create leaves the real table standing, named by nothing and dropped by no clear ever
+ * after. Which of the two a row is has to be decided by what it records and not by its presence.
+ */
+ @Test
+ public void testARowRecordingAnotherTableIsEnrolledAgain() throws Exception {
+ final TreeName tree = new TreeName("testCatalogStaleRow", "tree");
+ final JDBCStorage setUp = new JDBCStorage(createBackendCfg(getBackendId() + "_staleRow"), null);
+ try { // the table and its row, by a storage that then goes away
+ setUp.open(AccessMode.READ_WRITE);
+ setUp.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+ } finally {
+ setUp.close();
+ }
+ final String catalogTable = setUp.getTableName(setUp.getCatalogTree());
+ // what a version naming its tables otherwise would have left: a row of the right tree
+ // recording a table this one would never create
+ recordAnotherTable(catalogTable, "opendj_00000000000000000000000000000000000000000000000000000000");
+
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(getBackendId() + "_staleRow"), null);
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl())) {
+ assertEquals(
+ storage.catalogTables(con, JDBCStorage.TableScope.of(storage, con)).get(tree),
+ storage.getTableName(tree),
+ "a row recording a table this backend does not hold was left as it was: its tree is named at a table no clear can drop");
+ }
+ } finally {
+ clearQuietly(storage);
+ }
+ }
+
+ /**
+ * A clear which removed no tree of its backend says why, and says it where the one table it did
+ * drop was its own catalog: a catalog standing over rows that name nothing - the state a backup
+ * restored beside older tables leaves - is one drop and no tree removed, which is the outcome of
+ * #888 exactly and not a clear that did something.
+ *
+ * Decided on the drops of trees and not on every drop for that reason. Counted the other way the
+ * line is silent here, since dropping the catalog makes the count one.
+ */
+ @Test
+ public void testAClearWhichRemovedNoTreeSaysWhyEvenWhereItDroppedItsCatalog() throws Exception {
+ final DN baseDN = DN.valueOf("dc=clear-catalog-only,dc=com");
+ final TreeName owned = new TreeName(baseDN.toNormalizedUrlSafeString(), "id2entry");
+ final ReportingStorage storage =
+ new ReportingStorage(createBackendCfg(getBackendId() + "_catalogOnly", baseDN));
+ final String catalogTable = storage.getTableName(storage.getCatalogTree());
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(owned, true);
+ }
+ });
+ // the catalog table is there and names nothing, so the clear below has exactly one table to
+ // drop - its own - and leaves the tree standing, named by nothing
+ emptyTheCatalog(catalogTable);
+ storage.close();
+
+ storage.removeStorageFiles();
+
+ assertFalse(isExistsTable(catalogTable), "the clear left its own catalog table standing");
+ assertTrue(isExistsTable(storage.getTableName(owned)),
+ "a table named by no catalog was dropped: nothing may be dropped that cannot be attributed");
+ storage.assertReported("a clear which dropped its catalog and removed no tree of the backend"
+ + " said nothing about why, which is the silence of #888",
+ "removed no tree of this backend", "has to be started once");
+ } finally {
+ clearQuietly(storage);
+ // left standing on purpose above: its catalog is gone, so no clear of this backend names it
+ dropTableIfExists(storage.getTableName(owned));
+ }
+ }
+
+ /**
+ * A row recording a name outside the namespace this backend names its tables in is passed over
+ * rather than reaching a {@code drop table} built from a value read back out of a table - and the
+ * clear accounts for it, no other line of its report being able to: what such a row records is
+ * outside the {@code opendj} names the scan of what a clear left standing walks, and is dropped
+ * by nothing. The row is not there to be read again either - the catalog names itself last, so
+ * the clear drops that table with the row still in it - which is why the line is asserted here
+ * along with the drop: it is the only surviving copy of what the row said.
+ *
+ * Nothing this version writes makes such a row, which is why the case makes one by hand.
+ */
+ @Test
+ public void testAClearAccountsForACatalogRowItCannotActOn() throws Exception {
+ final TreeName tree = new TreeName("testCatalogForeignRow", "tree");
+ final ReportingStorage storage = new ReportingStorage(createBackendCfg(getBackendId() + "_foreignRow"));
+ final String tableName = storage.getTableName(tree);
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ }
+ });
+ final String catalogTable = storage.getTableName(storage.getCatalogTree());
+ recordAnotherTable(catalogTable, "a_table_of_something_else");
+
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl())) {
+ final List
+ * Taken from the drops themselves and not from the map the loop walks: the map is built with the
+ * catalog put last by hand, so an assertion on it would hold of any loop at all - one that sorted
+ * the keys, or copied them into a HashSet, included.
+ */
+ @Test
+ public void testAClearDropsTheCatalogAfterEveryTreeItNames() throws Exception {
+ final TreeName first = new TreeName("testCatalogDropOrder", "first");
+ final TreeName second = new TreeName("testCatalogDropOrder", "second");
+ final List
+ * What the case pins is one term of that condition. It pins it on a database holding nothing of
+ * anybody else that the scan cannot attribute - the fragments asserted are the counts this case
+ * owns, and a neighbour of another suite leaving an unstamped table would make the line fire for
+ * a reason of its own rather than fail this.
+ */
+ @Test
+ public void testAClearWhichDroppedNothingSaysWhyWhereARowItPassedOverIsAllItHad() throws Exception {
+ final DN baseDN = DN.valueOf("dc=clear-catalog-race,dc=com");
+ final TreeName owned = new TreeName(baseDN.toNormalizedUrlSafeString(), "id2entry");
+ final ReportingStorage storage =
+ new ReportingStorage(createBackendCfg(getBackendId() + "_catalogRace", baseDN));
+ final String catalogTable = storage.getTableName(storage.getCatalogTree());
+ final String ownedTable = storage.getTableName(owned);
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(owned, true);
+ }
+ });
+ // the one row of the catalog now records a name outside the namespace this backend names
+ // its tables in, so the read passes it over and the catalog names itself alone
+ recordAnotherTable(catalogTable, "a_table_of_something_else");
+ // and nothing of this backend is left standing for the scan to attribute to it
+ dropTableBehindTheBackend(ownedTable);
+ storage.close();
+
+ // the table goes while the clear is running, which is what leaves the clear with nothing
+ // dropped: an offline tool clearing the same backend a moment earlier. At the second
+ // lookup and not the first, so that the rows are read before the table goes - the first
+ // is catalogTables() asking whether there is a catalog at all
+ storage.takeAwayAtLookupNumber(catalogTable, 2);
+
+ storage.removeStorageFiles();
+
+ assertFalse(isExistsTable(catalogTable), "the catalog table this case takes away was still there");
+ storage.assertReported("a clear which dropped nothing at all and passed a row over said nothing"
+ + " about why, which is the silence of #888",
+ "the clear removed no tree of this backend", "it dropped 0 table(s) in all",
+ "0 of the trees its catalog names had lost their table already",
+ "and 0 table(s) of this backend were named by no catalog");
+ storage.assertReported("the row the clear could not act on was named by no line of it",
+ "a_table_of_something_else", "passed over");
+ } finally {
+ clearQuietly(storage);
+ dropTableIfExists(ownedTable);
+ dropTableIfExists(catalogTable);
+ }
+ }
+
+ /**
+ * Takes every row out of a catalog, leaving the tables it named standing: what a backend upgraded
+ * from a version keeping no catalog holds before its first read-write open fills one in.
+ */
+ private void emptyTheCatalog(String catalogTable) throws SQLException {
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final Statement st = con.createStatement()) {
+ st.executeUpdate("delete from " + catalogTable);
+ }
+ }
+
+ /**
+ * Records the given table for every row of a catalog, as a version naming its tables otherwise
+ * would have left them: the row names the right tree and a table this version never creates.
+ */
+ private void recordAnotherTable(String catalogTable, String tableName) throws SQLException {
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final PreparedStatement statement = con.prepareStatement("update " + catalogTable + " set v=?")) {
+ statement.setBytes(1, tableName.getBytes(StandardCharsets.UTF_8));
+ statement.executeUpdate();
+ }
+ }
+
+ /** Empties the recorded table name of every row of a catalog, as a version recording none would have left it. */
+ private void emptyTheRecordedTableNames(String catalogTable) throws SQLException {
+ try (final Connection con = DriverManager.getConnection(getJdbcUrl());
+ final PreparedStatement statement = con.prepareStatement("update " + catalogTable + " set v=?")) {
+ statement.setBytes(1, new byte[0]);
+ statement.executeUpdate();
+ }
+ }
+
+ /**
+ * Drops a table a clear left standing on purpose, so that it is not left behind for the rest of
+ * the class. A failure here is swallowed rather than replacing the failure of the case it cleans
+ * up after: what it leaves is dropped by the dropStaleTrees() of the next run of the class.
+ */
+ private void dropTableIfExists(String tableName) {
+ try {
+ if (isExistsTable(tableName)) {
+ dropTableBehindTheBackend(tableName);
+ }
+ } catch (SQLException ignored) {
+ }
+ }
+
+ /**
+ * A storage which keeps the lines every clear it runs reports, so that a case can hold that
+ * account to what it says.
+ *
+ * Those lines change no state whatsoever, so a case asserting on the database a clear leaves
+ * behind passes just as well with all of them deleted - which is how this report came to be
+ * changed in three rounds of review with nothing able to fail. Here rather than in the case that
+ * needed it first, for the same reason: the next line of the report wants an assertion too, and a
+ * helper per case is what got the report where it was. See {@link JDBCStorage#reportClearLine}.
+ */
+ protected static final class ReportingStorage extends JDBCStorage {
+ private final List
+ * Which lookup matters, and the count is not decoration: a clear asks about its catalog table
+ * twice - once in {@code catalogTables()} to decide whether there is a catalog to read at all,
+ * and once in the drop loop, per entry. Taken away before the first, the clear reads no row,
+ * passes none over and reports nothing, which is a different case from this one.
+ *
+ * Dropped on the very connection the lookup is made on, and not on one of the test's own: the
+ * clear holds its read of the catalog table until it commits, so a {@code drop table} issued
+ * from a second session would queue behind the transaction that is waiting for this call to
+ * return. Inside that transaction the drop takes no lock it does not already hold, and it is
+ * committed with the loop - or, on the two engines committing DDL as they go, at once.
+ */
+ void takeAwayAtLookupNumber(String tableName, int nth) {
+ lookupsToLetPass.set(nth - 1);
+ tableToTakeAway = tableName;
+ }
+
+ @Override
+ boolean isExistsTable(Connection con, JDBCStorage.TableScope scope, String tableName) {
+ final String taking = tableToTakeAway;
+ if (taking != null && taking.equalsIgnoreCase(tableName)
+ && lookupsToLetPass.getAndDecrement() <= 0) {
+ tableToTakeAway = null; // once: every later lookup is answered by the database alone
+ try (final PreparedStatement statement = con.prepareStatement("drop table " + taking)) {
+ statement.execute();
+ } catch (SQLException e) {
+ throw new IllegalStateException("the table this case takes away could not be dropped", e);
+ }
+ }
+ return super.isExistsTable(con, scope, tableName);
+ }
+
+ /** Every line reported so far, in the order the clears that reported them ran. */
+ List