From 1af0a1247d3e2d503a53723dd1d844c90c43ccc6 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Sat, 19 Sep 2026 08:56:47 +0000
Subject: [PATCH] [#993] Register an entry container's configuration listeners only once it has opened (#999)
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java | 69 ++
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java | 41 +
opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java | 109 +++-
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java | 112 +++-
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/FailedBackendOpenTest.java | 789 ++++++++++++++++++++++++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/jeb/JEStorageTest.java | 277 ++++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java | 120 ++++
7 files changed, 1,446 insertions(+), 71 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java
index 4db4d9f..11fa660 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java
@@ -791,37 +791,50 @@
trees.clear();
}
- if (env != null)
+ try
{
- DirectoryServer.deregisterMonitorProvider(monitor);
- monitor = null;
- try
+ if (env != null)
{
+ // Not yet registered when a failed open got no further than the environment itself.
+ if (monitor != null)
+ {
+ DirectoryServer.deregisterMonitorProvider(monitor);
+ monitor = null;
+ }
env.close();
env = null;
}
- catch (DatabaseException e)
- {
- throw new IllegalStateException(e);
- }
}
-
- if (memQuota != null)
+ catch (DatabaseException e)
{
- if (config.getDBCacheSize() > 0)
- {
- memQuota.releaseMemory(config.getDBCacheSize());
- }
- else
- {
- memQuota.releaseMemory(memQuota.memPercentToBytes(config.getDBCachePercent()));
- }
+ throw new IllegalStateException(e);
}
- config.removeJEChangeListener(this);
- envConfig = null;
- if (diskMonitor != null)
+ finally
{
- diskMonitor.deregisterMonitoredDirectory(getDirectory(), this);
+ // Given back after the environment, and whether or not its close threw: reporting the
+ // memory free before the environment has actually freed it would let a racing enable of
+ // another backend be admitted while this one's cache is still resident.
+ if (memQuota != null)
+ {
+ if (config.getDBCacheSize() > 0)
+ {
+ memQuota.releaseMemory(config.getDBCacheSize());
+ }
+ else
+ {
+ memQuota.releaseMemory(memQuota.memPercentToBytes(config.getDBCachePercent()));
+ }
+ // Released once: what an open takes, the next open takes again, and a close which follows
+ // a close - BackendImpl.importLDIF closes the storage of its root container however the
+ // import ended, on top of the close the import itself made - releases nothing more.
+ memQuota = null;
+ }
+ config.removeJEChangeListener(this);
+ envConfig = null;
+ if (diskMonitor != null)
+ {
+ diskMonitor.deregisterMonitoredDirectory(getDirectory(), this);
+ }
}
}
@@ -836,8 +849,53 @@
// Do not open files on disk
return;
}
+ rejectIfOpen();
buildConfiguration(accessMode, false);
- open0();
+ openOrGiveBack();
+ }
+
+ /**
+ * Refuses to open an environment which is open, before anything is taken for the attempt: the
+ * refusal guards against a programming error, and what this storage holds is left as it is.
+ */
+ private void rejectIfOpen()
+ {
+ if (env != null)
+ {
+ throw new IllegalStateException(
+ "Database is already open, either the backend is enabled or an import is currently running.");
+ }
+ }
+
+ /**
+ * Opens the environment, or gives back what the attempt took before it failed. Nothing else will:
+ * a root container does not close a storage whose {@code open()} threw, and a backend whose open
+ * failed is thrown away with the storage still registered as a listener of its configuration and
+ * the cache size it reserved still drawn from the memory quota - once per attempt to enable it.
+ */
+ private void openOrGiveBack() throws ConfigException
+ {
+ boolean opened = false;
+ try
+ {
+ open0();
+ opened = true;
+ }
+ finally
+ {
+ if (!opened)
+ {
+ try
+ {
+ close();
+ }
+ catch (RuntimeException e)
+ {
+ // The failure being given up after is the one worth reporting, and this must not replace it.
+ logger.traceException(e);
+ }
+ }
+ }
}
private boolean isBackendIncomplete(AccessMode accessMode)
@@ -863,11 +921,6 @@
setupStorageFiles(backendDirectory, config.getDBDirectoryPermissions(), config.dn());
try
{
- if (env != null)
- {
- throw new IllegalStateException(
- "Database is already open, either the backend is enabled or an import is currently running.");
- }
env = new Environment(backendDirectory, envConfig);
monitor = new JEMonitor(config.getBackendId() + " JE Database", env);
DirectoryServer.registerMonitorProvider(monitor);
@@ -899,8 +952,9 @@
@Override
public Importer startImport() throws ConfigException, StorageRuntimeException
{
+ rejectIfOpen();
buildConfiguration(AccessMode.READ_WRITE, true);
- open0();
+ openOrGiveBack();
return new ImporterImpl();
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
index 7ad8cc7..392f25b 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
@@ -1102,35 +1102,49 @@
@Override
public void close()
{
- if (db != null)
+ try
{
- DirectoryServer.deregisterMonitorProvider(monitor);
- monitor = null;
- try
+ if (db != null)
{
+ // Not yet registered when a failed open got no further than the database itself.
+ if (monitor != null)
+ {
+ DirectoryServer.deregisterMonitorProvider(monitor);
+ monitor = null;
+ }
db.close();
db = null;
}
- catch (final PersistitException e)
- {
- throw new IllegalStateException(e);
- }
}
- if (memQuota != null)
+ catch (final PersistitException e)
{
- if (config.getDBCacheSize() > 0)
- {
- memQuota.releaseMemory(config.getDBCacheSize());
- }
- else
- {
- memQuota.releaseMemory(memQuota.memPercentToBytes(config.getDBCachePercent()));
- }
+ throw new IllegalStateException(e);
}
- config.removePDBChangeListener(this);
- if (diskMonitor != null)
+ finally
{
- diskMonitor.deregisterMonitoredDirectory(getDirectory(), this);
+ // Given back after the database, and whether or not its close threw: reporting the memory
+ // free before the database has actually freed it would let a racing enable of another
+ // backend be admitted while this one's cache is still resident.
+ if (memQuota != null)
+ {
+ if (config.getDBCacheSize() > 0)
+ {
+ memQuota.releaseMemory(config.getDBCacheSize());
+ }
+ else
+ {
+ memQuota.releaseMemory(memQuota.memPercentToBytes(config.getDBCachePercent()));
+ }
+ // Released once: what an open takes, the next open takes again, and a close which follows
+ // a close - BackendImpl.importLDIF closes the storage of its root container however the
+ // import ended, on top of the close the import itself made - releases nothing more.
+ memQuota = null;
+ }
+ config.removePDBChangeListener(this);
+ if (diskMonitor != null)
+ {
+ diskMonitor.deregisterMonitoredDirectory(getDirectory(), this);
+ }
}
}
@@ -1148,7 +1162,52 @@
// Do not open volume on disk
return;
}
- open0(buildConfiguration(accessMode));
+ rejectIfOpen();
+ openOrGiveBack(buildConfiguration(accessMode));
+ }
+
+ /**
+ * Refuses to open a database which is open, before anything is taken for the attempt: the
+ * refusal guards against a programming error, and what this storage holds is left as it is.
+ */
+ private void rejectIfOpen()
+ {
+ if (db != null)
+ {
+ throw new IllegalStateException(
+ "Database is already open, either the backend is enabled or an import is currently running.");
+ }
+ }
+
+ /**
+ * Opens the database, or gives back what the attempt took before it failed. Nothing else will: a
+ * root container does not close a storage whose {@code open()} threw, and a backend whose open
+ * failed is thrown away with the storage still registered as a listener of its configuration and
+ * the cache size it reserved still drawn from the memory quota - once per attempt to enable it.
+ */
+ private void openOrGiveBack(final Configuration dbCfg) throws ConfigException
+ {
+ boolean opened = false;
+ try
+ {
+ open0(dbCfg);
+ opened = true;
+ }
+ finally
+ {
+ if (!opened)
+ {
+ try
+ {
+ close();
+ }
+ catch (RuntimeException e)
+ {
+ // The failure being given up after is the one worth reporting, and this must not replace it.
+ logger.traceException(e);
+ }
+ }
+ }
}
private boolean isBackendIncomplete(AccessMode accessMode)
@@ -1174,11 +1233,6 @@
setupStorageFiles(backendDirectory, config.getDBDirectoryPermissions(), config.dn());
try
{
- if (db != null)
- {
- throw new IllegalStateException(
- "Database is already open, either the backend is enabled or an import is currently running.");
- }
db = new Persistit(dbCfg);
final long bufferCount = getBufferPoolCfg(dbCfg).computeBufferCount(db.getAvailableHeap());
@@ -1217,7 +1271,8 @@
@Override
public Importer startImport() throws ConfigException, StorageRuntimeException
{
- open0(buildImportConfiguration());
+ rejectIfOpen();
+ openOrGiveBack(buildImportConfiguration());
return new ImporterImpl();
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
index 917b230..d47e2db 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
@@ -467,15 +467,8 @@
this.dn2uri = new DN2URI(getIndexName(REFERRAL_TREE_NAME), this);
this.state = new State(getIndexName(STATE_TREE_NAME));
- config.addPluggableChangeListener(this);
-
attributeIndexCfgManager = new AttributeIndexCfgManager();
- config.addBackendIndexAddListener(attributeIndexCfgManager);
- config.addBackendIndexDeleteListener(attributeIndexCfgManager);
-
vlvIndexCfgManager = new VLVIndexCfgManager();
- config.addBackendVLVIndexAddListener(vlvIndexCfgManager);
- config.addBackendVLVIndexDeleteListener(vlvIndexCfgManager);
}
private CryptoSuite newCryptoSuite(boolean confidentiality)
@@ -533,13 +526,16 @@
CryptoSuite cryptoSuite = newCryptoSuite(indexCfg.isConfidentialityEnabled());
final AttributeIndex index = newAttributeIndex(indexCfg, cryptoSuite);
+ // Held before it is opened, because open() is what registers it as a listener of its own
+ // configuration and close() is what takes that off again: an index which fails while
+ // opening is one this container must still be able to close.
+ attrIndexMap.put(indexCfg.getAttribute(), index);
+ attrCryptoMap.put(indexCfg.getAttribute(), cryptoSuite);
index.open(txn, shouldCreate);
if(!index.isTrusted() && isNotEmpty)
{
logger.info(NOTE_INDEX_ADD_REQUIRES_REBUILD, index.getName());
}
- attrIndexMap.put(indexCfg.getAttribute(), index);
- attrCryptoMap.put(indexCfg.getAttribute(), cryptoSuite);
}
for (String idx : config.listBackendVLVIndexes())
@@ -547,20 +543,37 @@
BackendVLVIndexCfg vlvIndexCfg = config.getBackendVLVIndex(idx);
VLVIndex vlvIndex = new VLVIndex(vlvIndexCfg, state, storage, this, txn);
+ // Held before it is opened, for the reason given above, and here the window is wider still:
+ // a VLV index registers itself as a listener of its configuration from its constructor.
+ vlvIndexMap.put(vlvIndexCfg.getName().toLowerCase(), vlvIndex);
vlvIndex.open(txn, shouldCreate);
if(!vlvIndex.isTrusted() && isNotEmpty)
{
logger.info(NOTE_INDEX_ADD_REQUIRES_REBUILD, vlvIndex.getName());
}
-
- vlvIndexMap.put(vlvIndexCfg.getName().toLowerCase(), vlvIndex);
}
+
+ // Registered once everything they answer for is open, and never from the constructor: an
+ // entry container which fails to open is registered nowhere - RootContainer.openEntryContainer
+ // and BackendImpl.changeBaseDNTrees both let the failure through before anything holds it -
+ // so nothing would ever call the close() which takes these off again, and they would go on
+ // answering configuration changes for a backend which is not running. Nothing can reach this
+ // container in between either: open() is called before it is registered anywhere.
+ config.addPluggableChangeListener(this);
+ config.addBackendIndexAddListener(attributeIndexCfgManager);
+ config.addBackendIndexDeleteListener(attributeIndexCfgManager);
+ config.addBackendVLVIndexAddListener(vlvIndexCfgManager);
+ config.addBackendVLVIndexDeleteListener(vlvIndexCfgManager);
}
- catch (StorageRuntimeException de)
+ catch (Exception e)
{
- logger.traceException(de);
+ // Every failure, not the storage ones alone: open() is declared to throw ConfigException and
+ // does - an index type the attribute has no matching rule for, an index protecting both its
+ // keys and its values, a VLV filter or sort order which does not parse - and the indexes
+ // opened before it registered listeners of their own, which only close() takes back.
+ logger.traceException(e);
close();
- throw de;
+ throw e;
}
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
index ad1dcd6..e1dbac5 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
@@ -17,6 +17,7 @@
*/
package org.opends.server.backends.pluggable;
+import static org.forgerock.util.Utils.closeSilently;
import static org.opends.messages.BackendMessages.*;
import static org.opends.server.util.StaticUtils.*;
@@ -129,9 +130,12 @@
*/
void open(final AccessMode accessMode) throws StorageRuntimeException, ConfigException
{
+ boolean opened = false;
+ boolean storageOpened = false;
try
{
storage.open(accessMode);
+ storageOpened = true;
storage.write(new WriteOperation()
{
@Override
@@ -144,6 +148,7 @@
// after the write, never inside it: a compressed schema migration is only worth reporting
// once the transaction that copied it has committed, and a replayed operation runs twice
compressedSchema.reportMigration();
+ opened = true;
}
catch(StorageRuntimeException e)
{
@@ -153,6 +158,52 @@
{
throw new StorageRuntimeException(e);
}
+ finally
+ {
+ if (!opened)
+ {
+ giveUpAfterFailedOpen(storageOpened);
+ }
+ }
+ }
+
+ /**
+ * Gives back what a root container which failed to open took. Nothing else will: the caller
+ * throws it away - {@link BackendImpl#newRootContainer} lets every failure through without a
+ * reference to it left anywhere - and {@code BackendConfigManager} releases the backend's shared
+ * lock without calling {@code closeBackend()} for a backend which never opened. What is left here
+ * is left for the life of the JVM: entry containers and this root container go on answering the
+ * configuration changes of a backend which is not running, and every later attempt to enable that
+ * backend adds another set of them.
+ * <p>
+ * The failure being given up after is the one worth reporting, so nothing here is allowed to
+ * replace it.
+ *
+ * @param storageOpened whether the storage opened, which is false on one road only: its
+ * {@code open()} threw. A storage whose open failed is not one this container can
+ * close - what that open took before it failed is the storage's own to give back, as
+ * {@code PDBStorage}, {@code JEStorage} and {@code JDBCStorage} do - and there is no other: every
+ * root container is opened over a storage no root container holds, since
+ * {@code BackendImpl} opens one, read only or not, only while it has none.
+ */
+ private void giveUpAfterFailedOpen(boolean storageOpened)
+ {
+ try
+ {
+ for (DN baseDN : entryContainers.keySet())
+ {
+ closeSilently(unregisterEntryContainer(baseDN));
+ }
+ config.removePluggableChangeListener(this);
+ if (storageOpened)
+ {
+ storage.close();
+ }
+ }
+ catch (Exception e)
+ {
+ logger.traceException(e);
+ }
}
/**
@@ -221,12 +272,28 @@
private void openAndRegisterEntryContainers(WriteableTransaction txn, Set<DN> baseDNs, AccessMode accessMode)
throws StorageRuntimeException, InitializationException, ConfigException
{
+ // Give up what a previous, rolled back attempt registered: this runs inside the write
+ // Storage.write may replay, and an entry container left registered fails the attempt which
+ // replaces it with ERR_ENTRY_CONTAINER_ALREADY_REGISTERED - so a write-write conflict, which
+ // the replay is there to absorb, would leave the backend unopened instead - while keeping the
+ // configuration listeners it opened with, which only its close() takes back. The same shape as
+ // BackendImpl.changeBaseDNTrees, which opens its entry containers inside a write for the same
+ // reason. Nothing else can reach these containers: the backend registers its base DNs only once
+ // this has returned, so they are closed without being locked.
+ for (DN baseDN : baseDNs)
+ {
+ closeSilently(unregisterEntryContainer(baseDN));
+ }
+
EntryID highestID = null;
for (DN baseDN : baseDNs)
{
EntryContainer ec = openEntryContainer(baseDN, txn, accessMode);
- EntryID id = ec.getHighestEntryID(txn);
+ // Registered before anything else here can throw: a container which opened has registered
+ // every listener it ever will, and only what this map holds is given back when the open of
+ // the root container fails.
registerEntryContainer(baseDN, ec);
+ EntryID id = ec.getHighestEntryID(txn);
if (highestID == null || id.compareTo(highestID) > 0)
{
highestID = id;
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jeb/JEStorageTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jeb/JEStorageTest.java
new file mode 100644
index 0000000..ef1e762
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jeb/JEStorageTest.java
@@ -0,0 +1,277 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package org.opends.server.backends.jeb;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
+import static org.forgerock.opendj.ldap.ByteString.valueOfUtf8;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+
+import org.forgerock.opendj.config.server.ConfigException;
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.DN;
+import org.forgerock.opendj.server.config.server.JEBackendCfg;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.backends.pluggable.spi.AccessMode;
+import org.opends.server.backends.pluggable.spi.ReadOperation;
+import org.opends.server.backends.pluggable.spi.ReadableTransaction;
+import org.opends.server.backends.pluggable.spi.TreeName;
+import org.opends.server.backends.pluggable.spi.WriteOperation;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.opends.server.core.MemoryQuota;
+import org.opends.server.core.ServerContext;
+import org.opends.server.extensions.DiskSpaceMonitor;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+/**
+ * Tests what a {@link JEStorage} takes as it opens and gives back when the open fails - the twin
+ * of the same cases on {@code PDBStorageTest}.
+ */
+@SuppressWarnings("javadoc")
+public class JEStorageTest extends DirectoryServerTestCase
+{
+ private static final String BACKEND_ID = "JEStorageTest";
+ /**
+ * A parent directory under which the storage's own directory is a regular file, so that the open
+ * fails once its configuration is built - the memory reserved - and before the environment is:
+ * what a backend whose directory the server cannot use meets.
+ */
+ private static final String BLOCKED_DB_DIRECTORY = BACKEND_ID + "-blocked";
+
+ private final TreeName treeName = new TreeName("dc=test", "test");
+ private ServerContext serverContext;
+ private JEStorage storage;
+
+ @BeforeClass
+ public static void startServer() throws Exception
+ {
+ TestCaseUtils.startServer();
+ }
+
+ @BeforeMethod
+ public void setUp() throws Exception
+ {
+ serverContext = mock(ServerContext.class);
+ when(serverContext.getMemoryQuota()).thenReturn(new MemoryQuota());
+ when(serverContext.getDiskSpaceMonitor()).thenReturn(mock(DiskSpaceMonitor.class));
+
+ storage = new JEStorage(createBackendCfg(), serverContext);
+ // the environment is removed on the way in as well as on the way out: a build whose JVM died never ran
+ // tearDown(), and this class shares a fixed db-directory across methods and across builds
+ storage.removeStorageFiles();
+ storage.open(AccessMode.READ_WRITE);
+ }
+
+ @AfterMethod
+ public void tearDown()
+ {
+ closeAndRemove(storage);
+ }
+
+ /**
+ * Closes the storage and removes its environment, keeping whichever of the two failed first. Removing it
+ * from a finally would let a removal failure replace the close() failure (JLS 14.20.2) - and a close() that
+ * throws is exactly the case the removal is here for.
+ */
+ private static void closeAndRemove(JEStorage storage)
+ {
+ RuntimeException failure = null;
+ try
+ {
+ storage.close();
+ }
+ catch (RuntimeException e)
+ {
+ failure = e;
+ }
+ try
+ {
+ storage.removeStorageFiles();
+ }
+ catch (RuntimeException e)
+ {
+ if (failure == null)
+ {
+ failure = e;
+ }
+ else
+ {
+ failure.addSuppressed(e);
+ }
+ }
+ if (failure != null)
+ {
+ throw failure;
+ }
+ }
+
+ /**
+ * An open which fails gives back what it took before it failed: the memory it reserved for the
+ * cache, and the listener the constructor registered on the backend configuration. Nothing else
+ * will - a root container does not close a storage which did not open - and a backend whose
+ * directory the server cannot use is enabled again and again, each attempt draining one cache
+ * size.
+ */
+ @Test
+ public void aStorageWhoseOpenFailedGivesBackWhatItTook() throws Exception
+ {
+ final JEBackendCfg cfg = createBackendCfg();
+ final JEStorage second = blockedStorage(cfg);
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ second.open(AccessMode.READ_WRITE);
+ fail("the storage was expected not to open over a directory which is a file");
+ }
+ catch (ConfigException expected)
+ {
+ // What a backend directory the server cannot use does.
+ }
+ finally
+ {
+ unblock(second);
+ }
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ verify(cfg).removeJEChangeListener(second);
+ }
+
+ /**
+ * A storage whose open failed has given everything back already, so closing it afterwards takes
+ * nothing more - {@code BackendImpl.importLDIF} closes the storage of its root container however
+ * the import ended - and does not fail on what the open never got to.
+ */
+ @Test
+ public void closingAStorageWhoseOpenFailedTakesNothingMore() throws Exception
+ {
+ final JEStorage second = blockedStorage(createBackendCfg());
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ second.open(AccessMode.READ_WRITE);
+ fail("the storage was expected not to open over a directory which is a file");
+ }
+ catch (ConfigException expected)
+ {
+ // What a backend directory the server cannot use does.
+ }
+ finally
+ {
+ unblock(second);
+ }
+
+ second.close();
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ }
+
+ /**
+ * A storage which is open refuses to open again before it takes anything, and what it holds is
+ * left as it is: the refusal is a guard against a programming error, not a failed open with
+ * something to give back.
+ */
+ @Test
+ public void openingAnOpenStorageIsRefusedAndTakesNothing() throws Exception
+ {
+ createTree();
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ storage.open(AccessMode.READ_WRITE);
+ fail("a storage which is open was expected to refuse to open again");
+ }
+ catch (IllegalStateException expected)
+ {
+ // The guard against a double open.
+ }
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ // Still open: a read reaches the environment.
+ assertThat(read("missing")).isNull();
+ }
+
+ /** A storage whose directory is a regular file, which no open of it can use. */
+ private JEStorage blockedStorage(JEBackendCfg cfg) throws Exception
+ {
+ when(cfg.getDBDirectory()).thenReturn(BLOCKED_DB_DIRECTORY);
+ final JEStorage blocked = new JEStorage(cfg, serverContext);
+ final File directory = blocked.getDirectory();
+ directory.getParentFile().mkdirs();
+ if (!directory.isFile())
+ {
+ assertThat(directory.createNewFile()).as("the file in the way of %s", directory).isTrue();
+ }
+ return blocked;
+ }
+
+ /** Removes the file in the way of the given storage's directory, and the directory it was made in. */
+ private static void unblock(JEStorage blocked)
+ {
+ final File directory = blocked.getDirectory();
+ directory.delete();
+ directory.getParentFile().delete();
+ }
+
+ private void createTree() throws Exception
+ {
+ storage.write(new WriteOperation()
+ {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception
+ {
+ txn.openTree(treeName, true);
+ }
+ });
+ }
+
+ private ByteString read(final String key) throws Exception
+ {
+ return storage.read(new ReadOperation<ByteString>()
+ {
+ @Override
+ public ByteString run(ReadableTransaction txn) throws Exception
+ {
+ return txn.read(treeName, valueOfUtf8(key));
+ }
+ });
+ }
+
+ private static JEBackendCfg createBackendCfg()
+ {
+ final JEBackendCfg backendCfg = mockCfg(JEBackendCfg.class);
+ when(backendCfg.dn()).thenReturn(DN.valueOf("ds-cfg-backend-id=" + BACKEND_ID + ",cn=Backends,cn=config"));
+ when(backendCfg.getBackendId()).thenReturn(BACKEND_ID);
+ when(backendCfg.getDBDirectory()).thenReturn(BACKEND_ID);
+ when(backendCfg.getDBDirectoryPermissions()).thenReturn("755");
+ when(backendCfg.getDBCacheSize()).thenReturn(0L);
+ when(backendCfg.getDBCachePercent()).thenReturn(20);
+ when(backendCfg.getDBNumCleanerThreads()).thenReturn(2);
+ when(backendCfg.getDBNumLockTables()).thenReturn(63);
+ return backendCfg;
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java
index f633849..9bcbbb2 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java
@@ -22,6 +22,7 @@
import static org.opends.server.util.StaticUtils.*;
import static org.forgerock.opendj.ldap.ByteString.*;
+import java.io.File;
import java.util.concurrent.atomic.AtomicInteger;
import org.forgerock.opendj.config.server.ConfigException;
@@ -32,10 +33,12 @@
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.ReadOperation;
import org.opends.server.backends.pluggable.spi.ReadableTransaction;
+import org.opends.server.backends.pluggable.spi.StorageInUseException;
import org.opends.server.backends.pluggable.spi.StorageRuntimeException;
import org.opends.server.backends.pluggable.spi.TreeName;
import org.opends.server.backends.pluggable.spi.WriteOperation;
import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.opends.server.core.DirectoryServer;
import org.opends.server.core.MemoryQuota;
import org.opends.server.core.ServerContext;
import org.opends.server.extensions.DiskSpaceMonitor;
@@ -422,6 +425,123 @@
assertThat(grown).as("the last attempts still sleep within the first attempt's bound").isGreaterThan(500);
}
+ /**
+ * An open which fails gives back what it took before it failed: the memory it reserved for the
+ * cache, and the listener the constructor registered on the backend configuration. Nothing else
+ * will - a root container does not close a storage which did not open - and a backend whose
+ * volume another storage holds is enabled again and again, each attempt draining one cache size.
+ */
+ @Test
+ public void aStorageWhoseOpenFailedGivesBackWhatItTook() throws Exception
+ {
+ final PDBBackendCfg cfg = createBackendCfg();
+ // Over the volume the storage of setUp() holds: what a second attempt to enable the backend meets.
+ final PDBStorage second = new PDBStorage(cfg, serverContext);
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ second.open(AccessMode.READ_WRITE);
+ fail("the storage was expected not to open over a volume another storage holds");
+ }
+ catch (StorageInUseException expected)
+ {
+ // What the lock on the volume file does.
+ }
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ verify(cfg).removePDBChangeListener(second);
+ }
+
+ /**
+ * A storage whose open failed has given everything back already, so closing it afterwards takes
+ * nothing more - {@code BackendImpl.importLDIF} closes the storage of its root container however
+ * the import ended - and does not fail on what the open never got to.
+ */
+ @Test
+ public void closingAStorageWhoseOpenFailedTakesNothingMore() throws Exception
+ {
+ final PDBStorage second = new PDBStorage(createBackendCfg(), serverContext);
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ second.open(AccessMode.READ_WRITE);
+ fail("the storage was expected not to open over a volume another storage holds");
+ }
+ catch (StorageInUseException expected)
+ {
+ // What the lock on the volume file does.
+ }
+
+ second.close();
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ }
+
+ /**
+ * A storage which is open refuses to open again before it takes anything, and what it holds is
+ * left as it is: the refusal is a guard against a programming error, not a failed open with
+ * something to give back.
+ */
+ @Test
+ public void openingAnOpenStorageIsRefusedAndTakesNothing() throws Exception
+ {
+ createTree();
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ storage.open(AccessMode.READ_WRITE);
+ fail("a storage which is open was expected to refuse to open again");
+ }
+ catch (IllegalStateException expected)
+ {
+ // The guard against a double open.
+ }
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ // Still open: a read reaches the database.
+ assertThat(read("missing")).isNull();
+ }
+
+ /**
+ * An open which fails once the database is open gives the database back with the rest: the
+ * volume, or no later open of the backend can take it, and the monitor the open registered. The
+ * disk monitor is the one thing past the database open that a test can refuse.
+ */
+ @Test
+ public void aStorageWhoseOpenFailedAfterItsDatabaseOpenedGivesTheDatabaseBack() throws Exception
+ {
+ // The volume of setUp() is given up first: held, it fails the open before the database is built.
+ closeAndRemove(storage);
+ final DiskSpaceMonitor refusing = mock(DiskSpaceMonitor.class);
+ doThrow(new IllegalStateException("the directory cannot be monitored"))
+ .when(refusing).registerMonitoredDirectory(anyString(), any(File.class), anyLong(), anyLong(), any());
+ when(serverContext.getDiskSpaceMonitor()).thenReturn(refusing);
+ final PDBBackendCfg cfg = createBackendCfg();
+ final PDBStorage second = new PDBStorage(cfg, serverContext);
+ final MemoryQuota quota = serverContext.getMemoryQuota();
+ final long availableBefore = quota.getAvailableMemory();
+ try
+ {
+ second.open(AccessMode.READ_WRITE);
+ fail("the storage was expected not to open when its directory cannot be monitored");
+ }
+ catch (IllegalStateException expected)
+ {
+ // What the failure past the database open does.
+ }
+
+ assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore);
+ verify(cfg).removePDBChangeListener(second);
+ assertThat(DirectoryServer.getMonitorProviders()).doesNotContainKey("pdbstoragetest pdb database");
+ // The volume was given back: a storage over the same directory opens.
+ when(serverContext.getDiskSpaceMonitor()).thenReturn(mock(DiskSpaceMonitor.class));
+ storage = new PDBStorage(createBackendCfg(), serverContext);
+ storage.open(AccessMode.READ_WRITE);
+ }
+
private void createTree() throws Exception
{
storage.write(new WriteOperation()
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/FailedBackendOpenTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/FailedBackendOpenTest.java
new file mode 100644
index 0000000..ccc1601
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/FailedBackendOpenTest.java
@@ -0,0 +1,789 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package org.opends.server.backends.pluggable;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
+import static org.mockito.Mockito.atLeast;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.opends.server.util.CollectionUtils.newTreeSet;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.function.BooleanSupplier;
+
+import org.forgerock.opendj.config.server.ConfigException;
+import org.forgerock.opendj.config.server.ConfigurationAddListener;
+import org.forgerock.opendj.config.server.ConfigurationChangeListener;
+import org.forgerock.opendj.config.server.ConfigurationDeleteListener;
+import org.forgerock.opendj.ldap.ByteSequence;
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.DN;
+import org.forgerock.opendj.ldap.schema.AttributeType;
+import org.forgerock.opendj.server.config.meta.BackendIndexCfgDefn.IndexType;
+import org.forgerock.opendj.server.config.meta.BackendVLVIndexCfgDefn.Scope;
+import org.forgerock.opendj.server.config.server.BackendIndexCfg;
+import org.forgerock.opendj.server.config.server.BackendVLVIndexCfg;
+import org.forgerock.opendj.server.config.server.PDBBackendCfg;
+import org.forgerock.opendj.server.config.server.PluggableBackendCfg;
+import org.mockito.ArgumentCaptor;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.backends.pdb.PDBStorage;
+import org.opends.server.backends.pluggable.spi.AccessMode;
+import org.opends.server.backends.pluggable.spi.Cursor;
+import org.opends.server.backends.pluggable.spi.Importer;
+import org.opends.server.backends.pluggable.spi.ReadOperation;
+import org.opends.server.backends.pluggable.spi.Storage;
+import org.opends.server.backends.pluggable.spi.StorageInUseException;
+import org.opends.server.backends.pluggable.spi.StorageRuntimeException;
+import org.opends.server.backends.pluggable.spi.StorageStatus;
+import org.opends.server.backends.pluggable.spi.TreeName;
+import org.opends.server.backends.pluggable.spi.UpdateFunction;
+import org.opends.server.backends.pluggable.spi.WriteOperation;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.opends.server.core.ServerContext;
+import org.opends.server.types.BackupConfig;
+import org.opends.server.types.BackupDirectory;
+import org.opends.server.types.DirectoryException;
+import org.opends.server.types.InitializationException;
+import org.opends.server.types.RestoreConfig;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.persistit.exception.RollbackException;
+
+/**
+ * Tests that a backend which fails to open gives back everything its opening took - see OpenDJ
+ * issue #993.
+ * <p>
+ * {@link EntryContainer} registers itself and its two configuration managers as listeners of the
+ * backend configuration, and every index it opens registers one of its own. Only
+ * {@link EntryContainer#close()} takes them off again, and an entry container whose
+ * {@link EntryContainer#open} failed is registered nowhere, so nothing will ever call it: the
+ * listeners of a backend which is not running answer configuration changes for the life of the JVM.
+ */
+@SuppressWarnings("javadoc")
+@Test(groups = { "precommit", "pluggablebackend" }, sequential = true)
+public class FailedBackendOpenTest extends DirectoryServerTestCase
+{
+ private static final String BACKEND_ID = "FailedBackendOpenTest";
+ private static final DN BASE_DN = DN.valueOf("dc=b993,dc=com");
+ /** A second base DN of the same backend, sorting after {@link #BASE_DN}. */
+ private static final DN SECOND_BASE_DN = DN.valueOf("dc=b993b,dc=com");
+
+ private ServerContext serverContext;
+ private AttributeType cnType;
+ /** The index configuration the entry container's attribute index registers with. */
+ private BackendIndexCfg indexCfg;
+ /** The VLV index configuration the entry container's VLV index registers with. */
+ private BackendVLVIndexCfg vlvIndexCfg;
+
+ @BeforeClass
+ public void startServer() throws Exception
+ {
+ TestCaseUtils.startServer();
+ serverContext = TestCaseUtils.getServerContext();
+ cnType = serverContext.getSchema().getAttributeType("cn");
+ }
+
+ /**
+ * A VLV index whose filter does not parse fails {@code EntryContainer.open()} with a
+ * {@link ConfigException}, after the attribute indexes ahead of it have opened and registered
+ * their own listeners. The backend does not open, and nothing it registered may be left behind.
+ */
+ @Test
+ public void aBackendWhichFailsToOpenLeavesNothingRegistered() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ when(vlvIndexCfg.getFilter()).thenReturn("(&(objectClass=*)");
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ // What an index the stored configuration names, and the schema no longer supports, does.
+ openExpectingFailure(backend, "the backend was expected not to open with a VLV index whose filter does not parse");
+
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ assertThat(stillRegisteredOn(indexCfg)).isEmpty();
+ }
+
+ /**
+ * A VLV index registers itself as a listener of its configuration from its constructor, and only
+ * reaches the entry container's map once it has opened. A failure in between is the one the
+ * container has always caught, and the index it is closing is still not one it holds.
+ */
+ @Test
+ public void anIndexWhichFailsToOpenLeavesNoListenerBehind() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ backend.storage.failOpeningTree("/dc=com,dc=b993/vlv.vlv1");
+ // What a storage which cannot give the index its tree does.
+ openExpectingFailure(backend, "the backend was expected not to open with a VLV index whose tree cannot be opened");
+
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ assertThat(stillRegisteredOn(vlvIndexCfg)).isEmpty();
+ }
+
+ /**
+ * The storage a failed open opened is given back along with the listeners.
+ * {@code BackendConfigManager} releases the backend's shared lock and never calls
+ * {@code closeBackend()} for a backend which did not open, so a volume left open here is one no
+ * later attempt to enable that backend can take.
+ */
+ @Test
+ public void aBackendWhichFailsToOpenGivesBackTheStorageItOpened() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ when(vlvIndexCfg.getFilter()).thenReturn("(&(objectClass=*)");
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ // What an index the stored configuration names, and the schema no longer supports, does.
+ final int closesByTheFailedOpen = openExpectingFailure(backend,
+ "the backend was expected not to open with a VLV index whose filter does not parse");
+
+ assertThat(closesByTheFailedOpen).isEqualTo(1);
+ }
+
+ /**
+ * A root container whose storage would not open has nothing of the storage's to give back: a
+ * {@code Storage.open()} which threw returns what it took itself, and a storage which never
+ * opened is not one to close. What is the root container's own - the listener it registered from
+ * its constructor - it gives back all the same.
+ */
+ @Test
+ public void aRootContainerWhichCouldNotOpenTheStorageDoesNotCloseIt() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ backend.storage.refuseToOpen();
+ // What a storage whose volume another process holds does.
+ final int closesByTheFailedOpen = openExpectingFailure(backend,
+ "the backend was expected not to open over a storage whose volume is locked");
+
+ assertThat(closesByTheFailedOpen).isEqualTo(0);
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ }
+
+ /**
+ * The give-back of a failed open walks the entry containers the attempt had registered, which
+ * every base DN but the last one leaves behind when a later one fails: the first base DN's
+ * container is open, registered and answering configuration changes by the time the second one
+ * cannot open its trees.
+ */
+ @Test
+ public void aSecondBaseDNWhichFailsToOpenGivesBackTheFirst() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ // A tree set, as the configuration's own is: dc=b993 sorts before dc=b993b, so it is the one
+ // opened - and registered - first.
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN, SECOND_BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ backend.storage.failOpeningTree("/dc=com,dc=b993b/id2entry");
+ // What a storage which cannot give the second container its trees does.
+ openExpectingFailure(backend,
+ "the backend was expected not to open with a second base DN whose trees cannot be opened");
+
+ // The road this test is about: the first container had opened before the second one failed.
+ assertThat(backend.storage.openedTrees()).contains("/dc=com,dc=b993/id2entry");
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ assertThat(stillRegisteredOn(indexCfg)).isEmpty();
+ assertThat(stillRegisteredOn(vlvIndexCfg)).isEmpty();
+ }
+
+ /**
+ * An entry container which opened has registered everything it ever will, and it is registered
+ * with the root container only after its highest entry ID has been read. A failure of that read
+ * leaves a container which nothing holds, unless it is registered before anything else can
+ * throw.
+ */
+ @Test
+ public void anEntryContainerWhichOpenedButWasNotRegisteredIsGivenBack() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ // The read of the highest entry ID is the first cursor over id2entry once the container has
+ // opened, which is when it registers itself: every cursor before that - the emptiness check of
+ // EntryContainer.open() and the one each untrusted index makes as it opens - fails the open
+ // itself, which the container catches.
+ backend.storage.failOpeningCursor("/dc=com,dc=b993/id2entry", () -> anEntryContainerIsRegisteredOn(cfg));
+ // What a storage which cannot position a cursor on the last entry does.
+ openExpectingFailure(backend, "the backend was expected not to open when the highest entry ID cannot be read");
+
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ assertThat(stillRegisteredOn(indexCfg)).isEmpty();
+ assertThat(stillRegisteredOn(vlvIndexCfg)).isEmpty();
+ }
+
+ /**
+ * The positive twin of the tests above: an entry container which opened is registered, once, as
+ * a listener of the backend configuration, and so are its two configuration managers and each
+ * index it opened. Without it, the registrations could be dropped and every test of a failed
+ * open would stay green.
+ */
+ @Test
+ public void anEntryContainerWhichOpenedIsRegisteredOnce() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ openExpectingSuccess(backend);
+ final List<Object> registered;
+ final List<Object> registeredOnIndex;
+ final List<Object> registeredOnVLVIndex;
+ try
+ {
+ registered = stillRegisteredOn(cfg);
+ registeredOnIndex = stillRegisteredOn(indexCfg);
+ registeredOnVLVIndex = stillRegisteredOn(vlvIndexCfg);
+ }
+ finally
+ {
+ backend.finalizeBackend();
+ }
+
+ assertThat(registered).filteredOn(listener -> listener instanceof EntryContainer).hasSize(1);
+ // The two configuration managers, each once as an add listener and once as a delete listener.
+ assertThat(registered)
+ .filteredOn(listener -> listener.getClass().getSimpleName().endsWith("IndexCfgManager"))
+ .hasSize(4);
+ assertThat(registeredOnIndex).hasSize(1);
+ assertThat(registeredOnVLVIndex).hasSize(1);
+ }
+
+ /**
+ * {@code RootContainer.open} opens and registers its entry containers inside the write a storage
+ * may replay after a transaction conflict. The attempt which replaces a rolled back one must find
+ * the registry as the first one found it: an entry container left registered fails it with
+ * {@code ERR_ENTRY_CONTAINER_ALREADY_REGISTERED}, so an ordinary write-write conflict becomes a
+ * backend which does not start, and the container it left behind keeps the listeners it opened
+ * with.
+ */
+ @Test
+ public void aReplayedOpenLeavesOneSetOfEntryContainers() throws Exception
+ {
+ final TrackedBackend backend = new TrackedBackend();
+ backend.setBackendID(BACKEND_ID);
+ final PDBBackendCfg cfg = backendCfg(newTreeSet(BASE_DN));
+ backend.configureBackend(cfg, serverContext);
+ backend.storage.removeStorageFiles();
+ backend.storage.conflictAtCommit(1);
+ openExpectingSuccess(backend);
+ try
+ {
+ assertThat(backend.storage.writeAttempts()).isEqualTo(2);
+ assertThat(backend.getRootContainer().getBaseDNs()).containsOnly(BASE_DN);
+ }
+ finally
+ {
+ backend.finalizeBackend();
+ }
+
+ // Closing the backend takes back what the backend which is running registered, so anything
+ // still registered here belongs to the attempt which was rolled back.
+ assertThat(stillRegisteredOn(cfg)).isEmpty();
+ assertThat(stillRegisteredOn(indexCfg)).isEmpty();
+ assertThat(stillRegisteredOn(vlvIndexCfg)).isEmpty();
+ }
+
+ /**
+ * Opens a backend which is not expected to open, and gives back what the attempt left behind:
+ * the storage a failed open may have left open, or the backend itself when it opened after all -
+ * its base DNs stay registered with the server otherwise, and every test which follows fails in
+ * {@code openBackend()} on them rather than on what it is about.
+ *
+ * @return how many times the failed open closed the storage, before this method closed it
+ */
+ private static int openExpectingFailure(TrackedBackend backend, String expectation) throws Exception
+ {
+ try
+ {
+ backend.openBackend();
+ }
+ catch (InitializationException expected)
+ {
+ final int closesByTheFailedOpen = backend.storage.closeCalls();
+ // A no-op once the failed open has given the storage back, and what keeps the tests which
+ // follow runnable if it has not.
+ backend.storage.close();
+ return closesByTheFailedOpen;
+ }
+ backend.finalizeBackend();
+ throw new AssertionError(expectation);
+ }
+
+ /**
+ * Opens a backend which is expected to open. One which does not is left with its volume closed,
+ * or every test which follows fails in {@code openBackend()} too and the one which actually
+ * broke is lost among them.
+ */
+ private static void openExpectingSuccess(TrackedBackend backend) throws Exception
+ {
+ try
+ {
+ backend.openBackend();
+ }
+ catch (Exception failedToOpen)
+ {
+ backend.storage.close();
+ throw failedToOpen;
+ }
+ }
+
+ /** Every listener added to the backend configuration and not taken off it again. */
+ private static List<Object> stillRegisteredOn(PluggableBackendCfg cfg) throws ConfigException
+ {
+ final List<Object> registered = new ArrayList<>();
+
+ final ArgumentCaptor<ConfigurationChangeListener<PluggableBackendCfg>> changeAdded =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).addPluggableChangeListener(changeAdded.capture());
+ registered.addAll(changeAdded.getAllValues());
+ final ArgumentCaptor<ConfigurationChangeListener<PluggableBackendCfg>> changeRemoved =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).removePluggableChangeListener(changeRemoved.capture());
+ removeEach(registered, changeRemoved.getAllValues());
+
+ final ArgumentCaptor<ConfigurationAddListener<BackendIndexCfg>> indexAdded =
+ captorFor(ConfigurationAddListener.class);
+ verify(cfg, atLeast(0)).addBackendIndexAddListener(indexAdded.capture());
+ registered.addAll(indexAdded.getAllValues());
+ final ArgumentCaptor<ConfigurationAddListener<BackendIndexCfg>> indexAddRemoved =
+ captorFor(ConfigurationAddListener.class);
+ verify(cfg, atLeast(0)).removeBackendIndexAddListener(indexAddRemoved.capture());
+ removeEach(registered, indexAddRemoved.getAllValues());
+
+ final ArgumentCaptor<ConfigurationDeleteListener<BackendIndexCfg>> indexDeleteAdded =
+ captorFor(ConfigurationDeleteListener.class);
+ verify(cfg, atLeast(0)).addBackendIndexDeleteListener(indexDeleteAdded.capture());
+ registered.addAll(indexDeleteAdded.getAllValues());
+ final ArgumentCaptor<ConfigurationDeleteListener<BackendIndexCfg>> indexDeleteRemoved =
+ captorFor(ConfigurationDeleteListener.class);
+ verify(cfg, atLeast(0)).removeBackendIndexDeleteListener(indexDeleteRemoved.capture());
+ removeEach(registered, indexDeleteRemoved.getAllValues());
+
+ final ArgumentCaptor<ConfigurationAddListener<BackendVLVIndexCfg>> vlvAdded =
+ captorFor(ConfigurationAddListener.class);
+ verify(cfg, atLeast(0)).addBackendVLVIndexAddListener(vlvAdded.capture());
+ registered.addAll(vlvAdded.getAllValues());
+ final ArgumentCaptor<ConfigurationAddListener<BackendVLVIndexCfg>> vlvAddRemoved =
+ captorFor(ConfigurationAddListener.class);
+ verify(cfg, atLeast(0)).removeBackendVLVIndexAddListener(vlvAddRemoved.capture());
+ removeEach(registered, vlvAddRemoved.getAllValues());
+
+ final ArgumentCaptor<ConfigurationDeleteListener<BackendVLVIndexCfg>> vlvDeleteAdded =
+ captorFor(ConfigurationDeleteListener.class);
+ verify(cfg, atLeast(0)).addBackendVLVIndexDeleteListener(vlvDeleteAdded.capture());
+ registered.addAll(vlvDeleteAdded.getAllValues());
+ final ArgumentCaptor<ConfigurationDeleteListener<BackendVLVIndexCfg>> vlvDeleteRemoved =
+ captorFor(ConfigurationDeleteListener.class);
+ verify(cfg, atLeast(0)).removeBackendVLVIndexDeleteListener(vlvDeleteRemoved.capture());
+ removeEach(registered, vlvDeleteRemoved.getAllValues());
+
+ return registered;
+ }
+
+ /** Every listener added to an index configuration and not taken off it again. */
+ private static List<Object> stillRegisteredOn(BackendIndexCfg cfg)
+ {
+ final List<Object> registered = new ArrayList<>();
+ final ArgumentCaptor<ConfigurationChangeListener<BackendIndexCfg>> added =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).addChangeListener(added.capture());
+ registered.addAll(added.getAllValues());
+ final ArgumentCaptor<ConfigurationChangeListener<BackendIndexCfg>> removed =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).removeChangeListener(removed.capture());
+ removeEach(registered, removed.getAllValues());
+ return registered;
+ }
+
+ /** Every listener added to a VLV index configuration and not taken off it again. */
+ private static List<Object> stillRegisteredOn(BackendVLVIndexCfg cfg)
+ {
+ final List<Object> registered = new ArrayList<>();
+ final ArgumentCaptor<ConfigurationChangeListener<BackendVLVIndexCfg>> added =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).addChangeListener(added.capture());
+ registered.addAll(added.getAllValues());
+ final ArgumentCaptor<ConfigurationChangeListener<BackendVLVIndexCfg>> removed =
+ captorFor(ConfigurationChangeListener.class);
+ verify(cfg, atLeast(0)).removeChangeListener(removed.capture());
+ removeEach(registered, removed.getAllValues());
+ return registered;
+ }
+
+ /**
+ * Whether an entry container is registered as a listener of the backend configuration, which is
+ * the last thing {@code EntryContainer.open()} does.
+ */
+ private static boolean anEntryContainerIsRegisteredOn(PluggableBackendCfg cfg)
+ {
+ try
+ {
+ for (Object listener : stillRegisteredOn(cfg))
+ {
+ if (listener instanceof EntryContainer)
+ {
+ return true;
+ }
+ }
+ return false;
+ }
+ catch (ConfigException declaredButNeverThrownByAMock)
+ {
+ throw new AssertionError(declaredButNeverThrownByAMock);
+ }
+ }
+
+ /**
+ * Takes one occurrence off the registrations for every removal, rather than every occurrence
+ * for any: a listener registered twice and taken off once is still registered.
+ */
+ private static void removeEach(List<Object> registered, List<?> removed)
+ {
+ for (Object listener : removed)
+ {
+ registered.remove(listener);
+ }
+ }
+
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ private static <T> ArgumentCaptor<T> captorFor(Class<?> listenerClass)
+ {
+ return (ArgumentCaptor<T>) ArgumentCaptor.forClass((Class) listenerClass);
+ }
+
+ private PDBBackendCfg backendCfg(SortedSet<DN> baseDNs) throws ConfigException
+ {
+ final PDBBackendCfg cfg = mockCfg(PDBBackendCfg.class);
+ when(cfg.dn()).thenReturn(DN.valueOf("ds-cfg-backend-id=" + BACKEND_ID + ",cn=Backends,cn=config"));
+ when(cfg.getBackendId()).thenReturn(BACKEND_ID);
+ when(cfg.getDBDirectory()).thenReturn(BACKEND_ID);
+ when(cfg.getDBDirectoryPermissions()).thenReturn("755");
+ when(cfg.getDBCacheSize()).thenReturn(0L);
+ when(cfg.getDBCachePercent()).thenReturn(20);
+ when(cfg.getBaseDN()).thenReturn(baseDNs);
+ when(cfg.listBackendIndexes()).thenReturn(new String[] { "cn" });
+ when(cfg.listBackendVLVIndexes()).thenReturn(new String[] { "vlv1" });
+
+ indexCfg = mock(BackendIndexCfg.class);
+ when(indexCfg.getIndexType()).thenReturn(newTreeSet(IndexType.EQUALITY));
+ when(indexCfg.getAttribute()).thenReturn(cnType);
+ when(indexCfg.getIndexEntryLimit()).thenReturn(4000);
+ when(indexCfg.getSubstringLength()).thenReturn(6);
+ when(cfg.getBackendIndex("cn")).thenReturn(indexCfg);
+
+ vlvIndexCfg = mock(BackendVLVIndexCfg.class);
+ when(vlvIndexCfg.getName()).thenReturn("vlv1");
+ when(vlvIndexCfg.getBaseDN()).thenReturn(baseDNs.first());
+ when(vlvIndexCfg.getScope()).thenReturn(Scope.WHOLE_SUBTREE);
+ when(vlvIndexCfg.getFilter()).thenReturn("(objectClass=*)");
+ when(vlvIndexCfg.getSortOrder()).thenReturn("+cn");
+ when(cfg.getBackendVLVIndex("vlv1")).thenReturn(vlvIndexCfg);
+ return cfg;
+ }
+
+ /** A backend whose storage the test can reach, and which counts what closes it. */
+ private static final class TrackedBackend extends BackendImpl<PDBBackendCfg>
+ {
+ private TrackingStorage storage;
+
+ @Override
+ protected Storage configureStorage(PDBBackendCfg cfg, ServerContext serverContext) throws ConfigException
+ {
+ storage = new TrackingStorage(new PDBStorage(cfg, serverContext));
+ return storage;
+ }
+ }
+
+ /**
+ * Decorates a {@link Storage} so that the test can tell whether what opened it also gave it back.
+ * {@link #close()} is answered once for each time the storage was opened, so that a test can
+ * close what a failure left open without hiding whether it had already been closed.
+ */
+ private static final class TrackingStorage implements Storage
+ {
+ private final Storage delegate;
+ private boolean open;
+ private int closeCalls;
+ /** The full name of the tree no transaction of this storage will open, if any. */
+ private String failingTree;
+ /** The full name of the tree one cursor of which no transaction of this storage will open, if any. */
+ private String failingCursorTree;
+ /** Once this holds, the next cursor over {@link #failingCursorTree} is the one refused. */
+ private BooleanSupplier cursorRefusalDue;
+ /** The full names of every tree a transaction of this storage opened. */
+ private final Set<String> openedTrees = new HashSet<>();
+ /** Whether this storage refuses to open at all, as one whose volume another process holds does. */
+ private boolean refuseToOpen;
+ /** How many write operations are still to be conflicted once they have run. */
+ private int conflictsLeft;
+ private int writeAttempts;
+
+ TrackingStorage(Storage delegate)
+ {
+ this.delegate = delegate;
+ }
+
+ /** How many times this storage was asked to close, whether it was open or not. */
+ int closeCalls()
+ {
+ return closeCalls;
+ }
+
+ /** Makes every transaction of this storage refuse to open the tree of the given full name. */
+ void failOpeningTree(String treeName)
+ {
+ failingTree = treeName;
+ }
+
+ /**
+ * Makes the transactions of this storage refuse to open one cursor over the tree of the given
+ * full name: the first one asked for once the given condition holds.
+ */
+ void failOpeningCursor(String treeName, BooleanSupplier once)
+ {
+ failingCursorTree = treeName;
+ cursorRefusalDue = once;
+ }
+
+ /** The full names of every tree a transaction of this storage opened. */
+ Set<String> openedTrees()
+ {
+ return openedTrees;
+ }
+
+ /** Makes this storage refuse to open, as one whose volume another process holds does. */
+ void refuseToOpen()
+ {
+ refuseToOpen = true;
+ }
+
+ /**
+ * Makes the next write operations conflict once they have run, as PersistIt reports a
+ * write-write conflict at commit time. The conflict is raised from within the single write the
+ * delegate is asked for, so the replay is the delegate's own retry loop.
+ */
+ void conflictAtCommit(int conflicts)
+ {
+ conflictsLeft = conflicts;
+ }
+
+ /** How many times a write operation was run, the replays included. */
+ int writeAttempts()
+ {
+ return writeAttempts;
+ }
+
+ @Override
+ public void open(AccessMode accessMode) throws Exception
+ {
+ if (refuseToOpen)
+ {
+ throw new StorageInUseException("the volume is locked by another process");
+ }
+ delegate.open(accessMode);
+ open = true;
+ }
+
+ @Override
+ public void close()
+ {
+ closeCalls++;
+ // Only what was opened is given back, so that a test can close what a failure left open
+ // without closing the delegate twice.
+ if (open)
+ {
+ open = false;
+ delegate.close();
+ }
+ }
+
+ @Override
+ public void write(final WriteOperation writeOperation) throws Exception
+ {
+ delegate.write(new WriteOperation()
+ {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception
+ {
+ writeAttempts++;
+ writeOperation.run(new TrackingTransaction(txn));
+ if (conflictsLeft > 0)
+ {
+ conflictsLeft--;
+ throw new RollbackException();
+ }
+ }
+ });
+ }
+
+ @Override
+ public <T> T read(ReadOperation<T> readOperation) throws Exception
+ {
+ return delegate.read(readOperation);
+ }
+
+ @Override
+ public Importer startImport() throws ConfigException
+ {
+ return delegate.startImport();
+ }
+
+ @Override
+ public void removeStorageFiles()
+ {
+ delegate.removeStorageFiles();
+ }
+
+ @Override
+ public StorageStatus getStorageStatus()
+ {
+ return delegate.getStorageStatus();
+ }
+
+ @Override
+ public Set<TreeName> listTrees()
+ {
+ return delegate.listTrees();
+ }
+
+ @Override
+ public boolean supportsBackupAndRestore()
+ {
+ return delegate.supportsBackupAndRestore();
+ }
+
+ @Override
+ public void createBackup(BackupConfig backupConfig) throws DirectoryException
+ {
+ delegate.createBackup(backupConfig);
+ }
+
+ @Override
+ public void removeBackup(BackupDirectory backupDirectory, String backupID) throws DirectoryException
+ {
+ delegate.removeBackup(backupDirectory, backupID);
+ }
+
+ @Override
+ public void restoreBackup(RestoreConfig restoreConfig) throws DirectoryException
+ {
+ delegate.restoreBackup(restoreConfig);
+ }
+
+ /**
+ * A transaction which is every bit the one it decorates, except that it records the trees it
+ * opens and refuses what the storage was told to refuse: one named tree - what a storage which
+ * cannot give an index the tree it asks for does - or one cursor over one.
+ */
+ private final class TrackingTransaction implements WriteableTransaction
+ {
+ private final WriteableTransaction delegate;
+
+ TrackingTransaction(WriteableTransaction delegate)
+ {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void openTree(TreeName name, boolean createOnDemand)
+ {
+ if (name.toString().equals(failingTree))
+ {
+ throw new StorageRuntimeException("cannot open " + name);
+ }
+ delegate.openTree(name, createOnDemand);
+ openedTrees.add(name.toString());
+ }
+
+ @Override
+ public Cursor<ByteString, ByteString> openCursor(TreeName treeName)
+ {
+ if (treeName.toString().equals(failingCursorTree) && cursorRefusalDue.getAsBoolean())
+ {
+ failingCursorTree = null;
+ throw new StorageRuntimeException("cannot open a cursor over " + treeName);
+ }
+ return delegate.openCursor(treeName);
+ }
+
+ @Override
+ public void deleteTree(TreeName name)
+ {
+ delegate.deleteTree(name);
+ }
+
+ @Override
+ public void put(TreeName treeName, ByteSequence key, ByteSequence value)
+ {
+ delegate.put(treeName, key, value);
+ }
+
+ @Override
+ public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f)
+ {
+ return delegate.update(treeName, key, f);
+ }
+
+ @Override
+ public boolean delete(TreeName treeName, ByteSequence key)
+ {
+ return delegate.delete(treeName, key);
+ }
+
+ @Override
+ public ByteString read(TreeName treeName, ByteSequence key)
+ {
+ return delegate.read(treeName, key);
+ }
+
+ @Override
+ public long getRecordCount(TreeName treeName)
+ {
+ return delegate.getRecordCount(treeName);
+ }
+
+ @Override
+ public boolean treeExists(TreeName treeName)
+ {
+ return delegate.treeExists(treeName);
+ }
+ }
+ }
+}
--
Gitblit v1.10.0