From 2b8612f7fa2f5e00dff4ff2d8ac912c833e1f7ab Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Wed, 09 Sep 2026 07:01:15 +0000
Subject: [PATCH] [#888] Name the trees of a JDBC backend from a catalog in the database (#893)
---
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java | 128 ++++++++++++++++++++++++++++++++++++++----
1 files changed, 116 insertions(+), 12 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java
index d59e1f4..acb9e86 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;
@@ -235,6 +236,54 @@
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.
+ * <p>
+ * 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.
+ * <p>
+ * 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.
+ * <p>
+ * 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.
* <p>
@@ -399,7 +448,10 @@
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 @@
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 @@
// 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.
+ * <p>
+ * 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<String> message) {
final long now = System.currentTimeMillis();
+ if (stallWarningDue(connectionString, throttleKeySuffix, startedAt, now)) {
+ logger.warn(LocalizableMessage.raw("%s", message.apply(now - startedAt)));
+ }
+ }
+
+ /**
+ * Whether a stall of this wait is to be reported now: a wait shorter than
+ * {@link #STALL_WARNING_AFTER_MS} is no stall yet, and one already reported for this url within
+ * {@link #STALL_WARNING_INTERVAL_MS} is not reported again - every worker thread of a server
+ * meets a database taking no connection at the same moment, and one line an interval is what
+ * an operator can read.
+ * <p>
+ * 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.
+ * <p>
+ * 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));
}
/**
--
Gitblit v1.10.0