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/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java | 41 ++++++++++++++++++++++++++++++++++++++++-
1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java
index 3e834f3..5d22954 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java
@@ -15,8 +15,10 @@
*/
package org.opends.server.backends.jdbc;
+import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.server.config.server.JDBCBackendCfg;
import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.backends.pluggable.spi.TreeName;
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.Importer;
import org.testng.annotations.AfterClass;
@@ -36,6 +38,7 @@
import java.sql.Driver;
import java.sql.DriverManager;
import java.sql.DriverPropertyInfo;
+import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.SQLTimeoutException;
import java.util.ArrayDeque;
@@ -61,6 +64,7 @@
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
+import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
@@ -685,6 +689,10 @@
final String url = StubDriver.PREFIX + "import-unchecked-commit";
final Connection parent = mock(Connection.class);
when(parent.isValid(anyInt())).thenReturn(true);
+ // the statement of the write below, which is what gives the commit of close() something to do:
+ // a connection an import wrote nothing through is not committed at all (#891), so an import
+ // that writes nothing would reach neither the Error this test injects nor the return it is about
+ when(parent.prepareStatement(anyString())).thenReturn(mock(PreparedStatement.class));
doThrow(new Error("out of memory while importing")).when(parent).commit();
stub.answerWith(parent);
final JDBCBackendCfg cfg = mock(JDBCBackendCfg.class);
@@ -692,12 +700,17 @@
final JDBCStorage storage = new JDBCStorage(cfg, null);
storage.open(AccessMode.READ_WRITE);
final Importer importer = storage.startImport();
+ importer.put(new TreeName("dc=example,dc=com", "id2entry"),
+ ByteString.valueOfUtf8("key"), ByteString.valueOfUtf8("value"));
try {
importer.close();
fail("the failure of the commit was not reported");
} catch (Error expected) {
- // reported to the caller, which is what an Error out of an import has to be
+ // reported to the caller, which is what an Error out of an import has to be. Asserted
+ // rather than accepted whole: an AssertionError of the fail() above is an Error too, and
+ // would otherwise be caught here and read as the injected one
+ assertEquals(expected.getMessage(), "out of memory while importing");
}
assertEquals(CachedConnection.poolOf(url).idleCount(), 1, "the import kept the connection of the pool");
@@ -1695,6 +1708,32 @@
assertEquals(CachedConnection.deadlineOf(startedAt, 60), startedAt + 60_000);
}
+ /**
+ * A borrow that carries a bound of its own waits for the shorter of the two, whichever way the
+ * deployment spelled a wait with no bound at all.
+ * <p>
+ * Only an import carries one: it holds a connection per tree it writes until it ends (#891), so
+ * the pool it waits at may be full of nothing but its own connections and the wait would be a
+ * deadlock rather than a queue. It has somewhere to go when the wait runs out - the tree is
+ * written through a connection the import already holds - so a long wait of the deployment is
+ * capped rather than merely replaced: paid over again for every tree the pool has nothing to
+ * spare for, it would be the duration of an import rather than a bound on it.
+ */
+ @Test
+ public void testABorrowWithABoundOfItsOwnWaitsForTheShorterOfTheTwo() {
+ assertEquals(CachedConnection.boundedWait(30, 60), 30, "the wait of the deployment was inside the bound");
+ assertEquals(CachedConnection.boundedWait(600, 60), 60, "a long finite wait was not capped");
+ assertEquals(CachedConnection.boundedWait(0, 60), 60, "0 stands for a wait with no bound");
+ // the other spelling of a wait with no bound: seconds enough that the milliseconds they
+ // stand for do not fit in a long, which the deadline of the borrow reads as forever
+ assertEquals(CachedConnection.boundedWait(Long.MAX_VALUE / 1000, 60), 60,
+ "a wait whose milliseconds overflow a long is unbounded too");
+ assertEquals(CachedConnection.boundedWait(Long.MAX_VALUE, 60), 60);
+ // ... and a borrow that carries no bound of its own takes the wait of the deployment whole
+ assertEquals(CachedConnection.boundedWait(600, 0), 600);
+ assertEquals(CachedConnection.boundedWait(0, 0), 0);
+ }
+
/** The connection string holds the credentials of the backend: a stall report must not carry them. */
@Test
public void testLoggedConnectionStringCarriesNoCredentials() throws Exception {
--
Gitblit v1.10.0