From a1b8537e1e0cd10e4d614dd9c6bdda541a614572 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Thu, 10 Sep 2026 11:56:05 +0000
Subject: [PATCH] [#891] Give the trees of an import connections of their own (#940)

---
 opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java |   71 +++++++++++++++++++++++++++++++----
 1 files changed, 63 insertions(+), 8 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 c03b8f6..fc3881b 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
@@ -359,16 +359,28 @@
     }
 
     /**
-     * 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.
+     * Whether a wait of this many seconds is one with no bound at all.
      * <p>
-     * The sum is guarded and not only the product: a value under the clamp above but large enough
+     * Two spellings of it, and one predicate for both so that they cannot drift: zero, which is how
+     * {@value #POOL_TIMEOUT_PROPERTY} says "wait for as long as it takes", and a number so large
+     * that the milliseconds it stands for do not fit in a {@code long} - a deadline computed from
+     * one of those overflows into the past, which is the opposite of what it asked for.
+     */
+    static boolean isUnboundedWait(long seconds) {
+        return seconds == 0 || seconds >= Long.MAX_VALUE / 1000;
+    }
+
+    /**
+     * The moment a borrow of this length gives up, or {@link Long#MAX_VALUE} where it gives up
+     * never - the two readings {@link #isUnboundedWait} names.
+     * <p>
+     * The sum is guarded and not only the product: a value that predicate lets through 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) {
+        if (isUnboundedWait(poolTimeoutSeconds)) {
             return Long.MAX_VALUE;
         }
         final long deadline = startedAt + poolTimeoutSeconds * 1000;
@@ -522,8 +534,10 @@
      * <p>
      * A lower bound than that is what is reported, not every way past it: the replay threads of
      * replication default to the same count again and borrow on top of the workers, and an import or
-     * a rebuild borrows besides. So this names one difference the operator can act on rather than
-     * standing for the whole demand on the pool.
+     * a rebuild borrows besides - one connection per tree it writes at a time, up to the bound of
+     * {@code JDBCStorage.IMPORT_CONNECTIONS_PROPERTY}, and it holds them for its whole duration
+     * (#891). So this names one difference the operator can act on rather than standing for the
+     * whole demand on the pool.
      * <p>
      * Nothing fails for the difference alone: the surplus waits for a connection to be returned,
      * which is what the bound is there for. But every one of those waits is paid on an operation,
@@ -809,8 +823,15 @@
      * Returns the value of a numeric system property, ignoring a value that is not a non-negative
      * number in favor of the default. The unit is the one the property is read in, so that the
      * value the message names is not mistaken for another.
+     * <p>
+     * Package private rather than private so that the bound on the connections of an import
+     * ({@code JDBCStorage.IMPORT_CONNECTIONS_PROPERTY}) can be read through it too, the way the
+     * bounds of the pool are: an operator who mistypes one of those is told rather than left with a
+     * default. Not every number this backend takes from a property comes through here - the
+     * statistics timeout and the fetch sizes of {@code JDBCStorage} are read with
+     * {@code Integer.getInteger}, which replaces a value it cannot parse in silence.
      */
-    private static long getNonNegativeProperty(String name, long defaultValue, String unit) {
+    static long getNonNegativeProperty(String name, long defaultValue, String unit) {
         final String value = System.getProperty(name);
         if (value != null) {
             try {
@@ -1260,11 +1281,45 @@
      * them is one borrow of a cold path, where the round trip the window saves is worth nothing.
      */
     static Connection getConnection(String connectionString, boolean trusted) throws Exception {
+        return getConnection(connectionString, trusted, 0);
+    }
+
+    /**
+     * The wait a borrow that carries a bound of its own actually gets: the shorter of what the
+     * deployment asked for and what the caller can afford, and the caller's where the deployment
+     * asked for no bound at all.
+     *
+     * @param maxWaitSeconds 0 for a caller that has no bound of its own, which takes the wait of
+     * the deployment whatever it is
+     */
+    static long boundedWait(long poolTimeoutSeconds, long maxWaitSeconds) {
+        if (isUnboundedWait(maxWaitSeconds)) {
+            return poolTimeoutSeconds;
+        }
+        return isUnboundedWait(poolTimeoutSeconds)
+            ? maxWaitSeconds : Math.min(poolTimeoutSeconds, maxWaitSeconds);
+    }
+
+    /**
+     * Borrows a connection, waiting at the bound of the pool no longer than the given number of
+     * seconds however long {@value #POOL_TIMEOUT_PROPERTY} says to wait.
+     *
+     * @param maxWaitSeconds the longest this borrow may wait at the bound of the pool, or 0 to wait
+     * as the property says. For a caller whose own connections are what the pool is full of - an
+     * import holds one per tree it writes until it ends (#891) - a wait with no bound is a deadlock
+     * rather than a queue: nothing is going to return the connection it is waiting for but itself.
+     * A caller that names one has somewhere to go when it runs out, so the wait of the deployment
+     * is capped rather than merely replaced where it is unbounded: an import that has to ask the
+     * pool once per tree would otherwise pay a long {@value #POOL_TIMEOUT_PROPERTY} over again for
+     * every tree the pool has nothing to spare for.
+     */
+    static Connection getConnection(String connectionString, boolean trusted, long maxWaitSeconds)
+            throws Exception {
         final Pool pool = poolOf(connectionString);
         final ConnectDialect dialect = ConnectDialect.of(connectionString);
         reportUnknownDialect(connectionString, dialect);
         final long connectTimeoutSeconds = getConnectTimeoutSeconds();
-        final long poolTimeoutSeconds = getPoolTimeoutSeconds();
+        final long poolTimeoutSeconds = boundedWait(getPoolTimeoutSeconds(), maxWaitSeconds);
         final long ttlMillis = getCacheTtlMillis();
         final long startedAt = System.currentTimeMillis();
         final long deadline = deadlineOf(startedAt, poolTimeoutSeconds);

--
Gitblit v1.10.0