From 64f3cffd001a0bd2f0ac7874e7ad61f663bbeb11 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 17 Sep 2026 22:35:16 +0300 Subject: [PATCH 1/2] [#1063] Give back what the open reserved rather than what the configuration says by then, and ask for a restart when the cache size changes PDBStorage and JEStorage reserved their cache size from the memory quota by reading config in buildConfiguration and released it by reading config again in close(). applyConfigurationChange swapped config in between without touching the quota or the cache, and neither db-cache-size nor db-cache-percent was marked as needing a restart, so a cache grown from 64 MB to 128 MB while the backend ran released 128 against 64 taken at the next disable - the one an online import makes included - and the quota believed 64 MB free that the server did not have, for the life of the JVM; a shrink left the difference reserved by nobody. The running cache was the old size throughout. Both storages now keep two numbers of their own: the cache size of the configuration they opened with, and of it what the quota granted - a tryAcquire it refused, which an open at startup is not checked against, reserved nothing and used to be released all the same. close() gives back the granted size. isConfigurationChangeAcceptable admits the difference to what is held rather than to config, which a change admitted but not yet applied has already moved to the new size. applyConfigurationChange on an open storage whose cache size the change moves sets adminActionRequired and says so (NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART): PersistIt cannot resize a buffer pool once the database is open, and JEStorage has never resized its environment. The two properties are marked component-restart in both configuration XMLs, as db-directory is. PDBStorageTest and JEStorageTest, six cases each: the grow and the shrink give back what was taken, the change asks for a restart and names both sizes, a change which leaves the cache alone asks for nothing, admission is against what is held, and a reservation the quota refused is not given back. --- .../server/config/JEBackendConfiguration.xml | 7 + .../server/config/PDBBackendConfiguration.xml | 7 + .../opends/server/backends/jeb/JEStorage.java | 56 ++++--- .../server/backends/pdb/PDBStorage.java | 60 +++++--- .../org/opends/messages/backend.properties | 3 + .../server/backends/jeb/JEStorageTest.java | 140 +++++++++++++++++- .../server/backends/pdb/PDBStorageTest.java | 140 +++++++++++++++++- 7 files changed, 369 insertions(+), 44 deletions(-) diff --git a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml index c54aa7a797..3c539d4863 100644 --- a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml +++ b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml @@ -14,6 +14,7 @@ Copyright 2007-2010 Sun Microsystems, Inc. Portions Copyright 2010-2015 ForgeRock AS. + Portions Copyright 2026 3A Systems, LLC. ! --> + + + 50 @@ -172,6 +176,9 @@ db-cache-percent property should be used instead to specify the cache size. + + + 0 MB diff --git a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml index b94b20c518..123d7811fd 100644 --- a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml +++ b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml @@ -13,6 +13,7 @@ information: "Portions Copyright [year] [name of copyright owner]". Copyright 2014-2015 ForgeRock AS. + Portions Copyright 2026 3A Systems, LLC. ! --> + + + 50 @@ -146,6 +150,9 @@ db-cache-percent property should be used instead to specify the cache size. + + + 0 MB 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 11fa660077..044527a2e2 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 @@ -696,6 +696,15 @@ private WriteableTransaction newWriteableTransaction(Transaction txn) private Environment env; private EnvironmentConfig envConfig; private MemoryQuota memQuota; + /** + * The cache size of the configuration this storage opened with, in bytes - what the memory quota + * was asked for - and of it, what the quota granted, which is what {@link #close()} gives back. + * Both are zero while the storage is closed. Neither is read from {@link #config} again: a + * configuration change replaces that while the environment and the reservation stay as the open + * made them, so a release computed from it would give back a size that was never taken. + */ + private long configuredCacheSize; + private long reservedCacheSize; private JEMonitor monitor; private DiskSpaceMonitor diskMonitor; private StorageStatus storageStatus = StorageStatus.working(); @@ -762,14 +771,10 @@ private void buildConfiguration(AccessMode accessMode, boolean isImport) throws diskMonitor = serverContext.getDiskSpaceMonitor(); memQuota = serverContext.getMemoryQuota(); - if (config.getDBCacheSize() > 0) - { - memQuota.acquireMemory(config.getDBCacheSize()); - } - else - { - memQuota.acquireMemory(memQuota.memPercentToBytes(config.getDBCachePercent())); - } + configuredCacheSize = computeSize(config); + // A reservation the quota refuses - its budget spent by the other backends, which an open at + // startup is not checked against - is nothing to give back: the open goes ahead without it. + reservedCacheSize = memQuota.acquireMemory(configuredCacheSize) ? configuredCacheSize : 0; } private DatabaseConfig dbConfig() @@ -816,14 +821,11 @@ public void close() // 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())); - } + // What the open reserved, not what the configuration says by now: a cache size changed + // while the storage was open is applied by the next open, which reserves it then. + memQuota.releaseMemory(reservedCacheSize); + reservedCacheSize = 0; + configuredCacheSize = 0; // 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. @@ -1276,15 +1278,19 @@ public Set listTrees() public boolean isConfigurationChangeAcceptable(JEBackendCfg newCfg, List unacceptableReasons) { - long newSize = computeSize(newCfg); - long oldSize = computeSize(config); - return (newSize <= oldSize || memQuota.isMemoryAvailable(newSize - oldSize)) + // Against what this storage holds of the quota, which is what the next open has to add to - not + // against config, which a change admitted but not yet applied has already moved to the new size. + final long newSize = computeSize(newCfg); + final MemoryQuota quota = serverContext.getMemoryQuota(); + return (newSize <= reservedCacheSize || quota.isMemoryAvailable(newSize - reservedCacheSize)) && checkConfigurationDirectories(newCfg, unacceptableReasons); } private long computeSize(JEBackendCfg cfg) { - return cfg.getDBCacheSize() > 0 ? cfg.getDBCacheSize() : memQuota.memPercentToBytes(cfg.getDBCachePercent()); + return cfg.getDBCacheSize() > 0 + ? cfg.getDBCacheSize() + : serverContext.getMemoryQuota().memPercentToBytes(cfg.getDBCachePercent()); } /** @@ -1369,6 +1375,16 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) return ccr; } } + final long newCacheSize = computeSize(cfg); + if (env != null && newCacheSize != configuredCacheSize) + { + // The cache is sized when the environment opens and this storage never resizes it: the next + // open of the backend builds it to the new size and reserves that, and until then the + // reservation stays with the cache it was made for. + ccr.setAdminActionRequired(true); + ccr.addMessage( + NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get(cfg.getBackendId(), configuredCacheSize, newCacheSize)); + } registerMonitoredDirectory(cfg); config = cfg; } 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 3ae64699ca..3fa27a58a2 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 @@ -1009,6 +1009,16 @@ private StorageImpl newStorageImpl() { private DiskSpaceMonitor diskMonitor; private PDBMonitor monitor; private MemoryQuota memQuota; + /** + * The cache size of the configuration this storage opened with, in bytes - what the buffer pool was + * built to and the memory quota was asked for - and of it, what the quota granted, which is what + * {@link #close()} gives back. Both are zero while the storage is closed. Neither is read from + * {@link #config} again: a configuration change replaces that while the pool and the reservation + * stay as the open made them, so a release computed from it would give back a size that was never + * taken. + */ + private long configuredCacheSize; + private long reservedCacheSize; private StorageStatus storageStatus = StorageStatus.working(); /** Attempt bound of a {@link WriteableStorageImpl#write}, {@link #MAX_RETRIES} outside the tests. */ private final int maxRetries; @@ -1084,16 +1094,11 @@ private Configuration buildConfiguration(AccessMode accessMode) diskMonitor = serverContext.getDiskSpaceMonitor(); memQuota = serverContext.getMemoryQuota(); - if (config.getDBCacheSize() > 0) - { - bufferPoolCfg.setMaximumMemory(config.getDBCacheSize()); - memQuota.acquireMemory(config.getDBCacheSize()); - } - else - { - bufferPoolCfg.setMaximumMemory(memQuota.memPercentToBytes(config.getDBCachePercent())); - memQuota.acquireMemory(memQuota.memPercentToBytes(config.getDBCachePercent())); - } + configuredCacheSize = computeSize(config); + bufferPoolCfg.setMaximumMemory(configuredCacheSize); + // A reservation the quota refuses - its budget spent by the other backends, which an open at + // startup is not checked against - is nothing to give back: the open goes ahead without it. + reservedCacheSize = memQuota.acquireMemory(configuredCacheSize) ? configuredCacheSize : 0; commitPolicy = config.isDBTxnNoSync() ? SOFT : GROUP; dbCfg.setJmxEnabled(false); return dbCfg; @@ -1127,14 +1132,11 @@ public void close() // 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())); - } + // What the open reserved, not what the configuration says by now: a cache size changed + // while the storage was open is applied by the next open, which reserves it then. + memQuota.releaseMemory(reservedCacheSize); + reservedCacheSize = 0; + configuredCacheSize = 0; // 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. @@ -1547,15 +1549,19 @@ private static ByteString valueToBytes(final Value value) public boolean isConfigurationChangeAcceptable(PDBBackendCfg newCfg, List unacceptableReasons) { - long newSize = computeSize(newCfg); - long oldSize = computeSize(config); - return (newSize <= oldSize || memQuota.isMemoryAvailable(newSize - oldSize)) + // Against what this storage holds of the quota, which is what the next open has to add to - not + // against config, which a change admitted but not yet applied has already moved to the new size. + final long newSize = computeSize(newCfg); + final MemoryQuota quota = serverContext.getMemoryQuota(); + return (newSize <= reservedCacheSize || quota.isMemoryAvailable(newSize - reservedCacheSize)) && checkConfigurationDirectories(newCfg, unacceptableReasons); } private long computeSize(PDBBackendCfg cfg) { - return cfg.getDBCacheSize() > 0 ? cfg.getDBCacheSize() : memQuota.memPercentToBytes(cfg.getDBCachePercent()); + return cfg.getDBCacheSize() > 0 + ? cfg.getDBCacheSize() + : serverContext.getMemoryQuota().memPercentToBytes(cfg.getDBCachePercent()); } /** @@ -1640,6 +1646,16 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) return ccr; } } + final long newCacheSize = computeSize(cfg); + if (db != null && newCacheSize != configuredCacheSize) + { + // The buffer pool is sized when the database opens and PersistIt has no way to resize it: the + // next open of the backend builds it to the new size and reserves that, and until then the + // reservation stays with the pool it was made for. + ccr.setAdminActionRequired(true); + ccr.addMessage( + NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get(cfg.getBackendId(), configuredCacheSize, newCacheSize)); + } registerMonitoredDirectory(cfg); config = cfg; commitPolicy = config.isDBTxnNoSync() ? SOFT : GROUP; diff --git a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties index a897dc4cb0..6590d4c3d1 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1156,3 +1156,6 @@ ERR_CONFIG_BACKEND_DATA_CHANGE_FAILED_627=The compression, encoding and encrypti backend base DN '%s' could not be applied in full: %s. Its entries and its indexes may no longer \ be encoded under the same settings; disabling and enabling this backend builds both from the \ stored configuration again +NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART_630=The change to the database cache of backend %s will not take effect \ + until the backend is restarted: the cache the backend runs with, and the memory reserved for it, stay at the \ + %d bytes the backend was opened with until then, and the %d bytes now configured are reserved by the next open 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 index ef1e762f86..bd32ed6da8 100644 --- 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 @@ -22,12 +22,19 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART; +import static org.opends.server.util.StaticUtils.MB; import java.io.File; +import java.util.ArrayList; +import java.util.List; +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.server.ConfigChangeResult; import org.forgerock.opendj.config.server.ConfigException; import org.forgerock.opendj.ldap.ByteString; import org.forgerock.opendj.ldap.DN; +import org.forgerock.opendj.ldap.ResultCode; import org.forgerock.opendj.server.config.server.JEBackendCfg; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; @@ -59,6 +66,8 @@ public class JEStorageTest extends DirectoryServerTestCase * what a backend whose directory the server cannot use meets. */ private static final String BLOCKED_DB_DIRECTORY = BACKEND_ID + "-blocked"; + /** A cache size the quota of the test JVM grants several times over, in bytes. */ + private static final long SMALL_CACHE = 64L * MB; private final TreeName treeName = new TreeName("dc=test", "test"); private ServerContext serverContext; @@ -215,6 +224,129 @@ public void openingAnOpenStorageIsRefusedAndTakesNothing() throws Exception assertThat(read("missing")).isNull(); } + /** + * A cache size changed while the storage is open is given back as it was taken: the close + * releases what the open reserved, not what the configuration says by then. Read from the + * configuration at both ends, a change in between drifts the quota by the difference for the + * life of the JVM - the open which follows reserves the new size and pays nothing back. + */ + @Test + public void aCacheGrownWhileOpenIsGivenBackAsItWasTaken() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + final long availableBefore = quota.getAvailableMemory(); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore - SMALL_CACHE); + + storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + + /** The shrink is the same drift the other way: the difference stays reserved by nobody. */ + @Test + public void aCacheShrunkWhileOpenIsGivenBackAsItWasTaken() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + final long availableBefore = quota.getAvailableMemory(); + storage = new JEStorage(createBackendCfg(2 * SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + + storage.applyConfigurationChange(createBackendCfg(SMALL_CACHE)); + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + + /** + * The cache is sized when the environment opens and this storage never resizes it, so a change + * of the cache size is applied by the next open of the backend - and the operator is told so, + * rather than that the change applied. + */ + @Test + public void aCacheSizeChangedWhileOpenAsksForARestart() throws Exception + { + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.ordinal()); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo( + NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get(BACKEND_ID, SMALL_CACHE, 2 * SMALL_CACHE).toString()); + } + + /** A change which leaves the cache size alone asks for nothing, as before. */ + @Test + public void aChangeWhichLeavesTheCacheSizeAloneAsksForNothing() throws Exception + { + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + final JEBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(unchangedCache); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + } + + /** + * A change of the cache size is admitted against what the storage holds of the quota, which is + * what the next open has to add to. Once a change has been admitted but not applied, the + * configuration says the new size while the reservation is still the old one, and a check + * against the configuration would admit a second change the server has no memory for. + */ + @Test + public void aCacheSizeChangeIsAdmittedAgainstWhatTheStorageHolds() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + // Room for two caches and a bit: the difference to the configured size, not to the reserved one. + assertThat(quota.acquireMemory(quota.getAvailableMemory() - 2 * SMALL_CACHE - MB)).isTrue(); + + final List reasons = new ArrayList<>(); + assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(4 * SMALL_CACHE), reasons)) + .as("four caches, with one reserved and two and a bit free").isFalse(); + assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(3 * SMALL_CACHE), reasons)) + .as("three caches, with one reserved and two and a bit free").isTrue(); + } + + /** + * A reservation the quota refused is not given back on close. The open goes ahead without it - + * the quota is a budget, not a lock - but a close which released what was never taken would + * hand the quota memory the server does not have. + */ + @Test + public void aReservationTheQuotaRefusedIsNotGivenBackOnClose() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + // Half a cache left in the quota: the reservation of a whole one is refused. + assertThat(quota.acquireMemory(quota.getAvailableMemory() - SMALL_CACHE / 2)).isTrue(); + final long availableBefore = quota.getAvailableMemory(); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + /** A storage whose directory is a regular file, which no open of it can use. */ private JEStorage blockedStorage(JEBackendCfg cfg) throws Exception { @@ -262,13 +394,19 @@ public ByteString run(ReadableTransaction txn) throws Exception } private static JEBackendCfg createBackendCfg() + { + return createBackendCfg(0L); + } + + /** A configuration whose cache is the given size in bytes, or a fifth of the quota when it is zero. */ + private static JEBackendCfg createBackendCfg(long cacheSize) { 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.getDBCacheSize()).thenReturn(cacheSize); when(backendCfg.getDBCachePercent()).thenReturn(20); when(backendCfg.getDBNumCleanerThreads()).thenReturn(2); when(backendCfg.getDBNumLockTables()).thenReturn(63); 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 9bcbbb2fe9..b40670b206 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 @@ -21,12 +21,18 @@ import static org.forgerock.opendj.config.ConfigurationMock.*; import static org.opends.server.util.StaticUtils.*; import static org.forgerock.opendj.ldap.ByteString.*; +import static org.opends.messages.BackendMessages.*; import java.io.File; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.server.ConfigChangeResult; import org.forgerock.opendj.config.server.ConfigException; import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.ResultCode; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; import org.forgerock.opendj.server.config.server.PDBBackendCfg; @@ -63,6 +69,9 @@ public class PDBStorageTest extends DirectoryServerTestCase private ServerContext serverContext; private PDBStorage storage; + /** A cache size the quota of the test JVM grants several times over, in bytes. */ + private static final long SMALL_CACHE = 64L * MB; + @BeforeClass public static void startServer() throws Exception { @@ -542,6 +551,129 @@ public void aStorageWhoseOpenFailedAfterItsDatabaseOpenedGivesTheDatabaseBack() storage.open(AccessMode.READ_WRITE); } + /** + * A cache size changed while the storage is open is given back as it was taken: the close + * releases what the open reserved, not what the configuration says by then. Read from the + * configuration at both ends, a change in between drifts the quota by the difference for the + * life of the JVM - the open which follows reserves the new size and pays nothing back. + */ + @Test + public void aCacheGrownWhileOpenIsGivenBackAsItWasTaken() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + final long availableBefore = quota.getAvailableMemory(); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore - SMALL_CACHE); + + storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + + /** The shrink is the same drift the other way: the difference stays reserved by nobody. */ + @Test + public void aCacheShrunkWhileOpenIsGivenBackAsItWasTaken() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + final long availableBefore = quota.getAvailableMemory(); + storage = new PDBStorage(createBackendCfg(2 * SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + + storage.applyConfigurationChange(createBackendCfg(SMALL_CACHE)); + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + + /** + * The buffer pool is sized when the database opens and PersistIt has no way to resize it, so a + * change of the cache size is applied by the next open of the backend - and the operator is told + * so, rather than that the change applied. + */ + @Test + public void aCacheSizeChangedWhileOpenAsksForARestart() throws Exception + { + closeAndRemove(storage); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.ordinal()); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo( + NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get("PDBStorageTest", SMALL_CACHE, 2 * SMALL_CACHE).toString()); + } + + /** A change which leaves the cache size alone asks for nothing, as before. */ + @Test + public void aChangeWhichLeavesTheCacheSizeAloneAsksForNothing() throws Exception + { + closeAndRemove(storage); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + final PDBBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(unchangedCache); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + } + + /** + * A change of the cache size is admitted against what the storage holds of the quota, which is + * what the next open has to add to. Once a change has been admitted but not applied, the + * configuration says the new size while the reservation is still the old one, and a check + * against the configuration would admit a second change the server has no memory for. + */ + @Test + public void aCacheSizeChangeIsAdmittedAgainstWhatTheStorageHolds() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + storage.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + // Room for two caches and a bit: the difference to the configured size, not to the reserved one. + assertThat(quota.acquireMemory(quota.getAvailableMemory() - 2 * SMALL_CACHE - MB)).isTrue(); + + final List reasons = new ArrayList<>(); + assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(4 * SMALL_CACHE), reasons)) + .as("four caches, with one reserved and two and a bit free").isFalse(); + assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(3 * SMALL_CACHE), reasons)) + .as("three caches, with one reserved and two and a bit free").isTrue(); + } + + /** + * A reservation the quota refused is not given back on close. The open goes ahead without it - + * the quota is a budget, not a lock - but a close which released what was never taken would + * hand the quota memory the server does not have. + */ + @Test + public void aReservationTheQuotaRefusedIsNotGivenBackOnClose() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + // Half a cache left in the quota: the reservation of a whole one is refused. + assertThat(quota.acquireMemory(quota.getAvailableMemory() - SMALL_CACHE / 2)).isTrue(); + final long availableBefore = quota.getAvailableMemory(); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + + storage.close(); + + assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); + } + private void createTree() throws Exception { storage.write(new WriteOperation() @@ -567,12 +699,18 @@ public ByteString run(ReadableTransaction txn) throws Exception } protected PDBBackendCfg createBackendCfg() + { + return createBackendCfg(0L); + } + + /** A configuration whose cache is the given size in bytes, or a fifth of the quota when it is zero. */ + private static PDBBackendCfg createBackendCfg(long cacheSize) { PDBBackendCfg backendCfg = mockCfg(PDBBackendCfg.class); when(backendCfg.getBackendId()).thenReturn("PDBStorageTest"); when(backendCfg.getDBDirectory()).thenReturn("PDBStorageTest"); when(backendCfg.getDBDirectoryPermissions()).thenReturn("755"); - when(backendCfg.getDBCacheSize()).thenReturn(0L); + when(backendCfg.getDBCacheSize()).thenReturn(cacheSize); when(backendCfg.getDBCachePercent()).thenReturn(20); return backendCfg; } From 4ea1d188930805911dd3892d4627055438abd600 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 06:54:30 +0300 Subject: [PATCH 2/2] [#1068] Apply what a running backend takes of a configuration change, and ask for a restart for what it does not Nine properties of the JE and PDB backends were neither applied to a running backend nor marked as requiring a restart. JEStorage.applyConfigurationChange handled the directory, its permissions and the disk thresholds and left the environment - configured once, at the open - as it was, while the XML kept db-cleaner-min-utilization, db-run-cleaner, db-evictor-core-threads, db-evictor-max-threads, db-evictor-keep-alive, db-num-cleaner-threads, db-txn-no-sync and db-txn-write-no-sync (JE) and db-checkpointer-wakeup-interval (PDB) as live properties, which they had been in the local-db backend OPENDJ-1719 replaced. A change of any of them was reported as applied while the backend ran on unchanged until it was next opened; so was a native property changed through je-property. JEStorage now builds the environment configuration the changed configuration describes and hands it to Environment.setMutableConfig, which takes of it what JE accepts while it runs: the properties above, the durability, and a mutable native property - all but the cache, which stays with the memory reserved for it until the restart #1063 asks for. Every immutable JE parameter whose value differs from the running environment's is reported with the new NOTE 631, which names the property, the value the environment runs with and the one configured, and reaches the error log as a warning - where the change result of a property marked in the XML alone never did. An import's environment is left alone: it runs on a configuration of its own, and the backend opens again on the changed one once the import is over. The build of the environment configuration is split from the checks of the open (ConfigurableEnvironment.toEnvironmentConfig): no cache size probe against the memory quota, no level set on the JE loggers, so that a configuration change can be checked against it as well - and it is: isConfigurationChangeAcceptable and isConfigurationAcceptable refuse a durability which sets both flags (db-txn-write-no-sync is on by default, so setting db-txn-no-sync alone is one) and a native property JE does not know before the change is written. Nothing checked either before, and the backend failed to open on them at its next restart. A configuration which sets neither durability flag now sets COMMIT_SYNC explicitly: what JE falls back on, but set, since JE leaves the durability an environment has in place when a configuration hands it none. PDBStorage reports a change of db-checkpointer-wakeup-interval with the same note, holding the configured interval against the one the database opened with - PersistIt takes no configuration once one is set - and the property is marked component-restart in PDBBackendConfiguration.xml. The definition of je-property says which of its changes wait for a restart. --- .../server/config/JEBackendConfiguration.xml | 9 + .../server/config/PDBBackendConfiguration.xml | 3 + .../backends/jeb/ConfigurableEnvironment.java | 52 +++- .../opends/server/backends/jeb/JEStorage.java | 73 +++++- .../server/backends/pdb/PDBStorage.java | 10 + .../org/opends/messages/backend.properties | 2 + .../server/backends/jeb/JEStorageTest.java | 247 +++++++++++++++++- .../server/backends/pdb/PDBStorageTest.java | 52 ++++ 8 files changed, 440 insertions(+), 8 deletions(-) diff --git a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml index 3c539d4863..120f2e27f8 100644 --- a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml +++ b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/JEBackendConfiguration.xml @@ -726,6 +726,15 @@ all the property parameters is available in the example.properties file of Berkeley DB Java Edition distribution. + + + + A change of a property which Berkeley DB Java Edition does not + accept while the environment runs takes effect when the backend + is restarted, and the change result says so. + + + diff --git a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml index 123d7811fd..9f8b1b57c2 100644 --- a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml +++ b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PDBBackendConfiguration.xml @@ -254,6 +254,9 @@ disk, but also potentially causes recovery from an abrupt termination (crash) to take more time. + + + 15s diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ConfigurableEnvironment.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ConfigurableEnvironment.java index 896d0910ff..ac9b63b643 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ConfigurableEnvironment.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ConfigurableEnvironment.java @@ -13,6 +13,7 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2010-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.jeb; @@ -375,7 +376,25 @@ private static EnvironmentConfig defaultConfig() static EnvironmentConfig parseConfigEntry(JEBackendCfg cfg) throws ConfigException { validateDbCacheSize(cfg.getDBCacheSize()); + final EnvironmentConfig envConfig = toEnvironmentConfig(cfg); + // The JE loggers are shared by every environment of the JVM: their level is set by the open, not + // built into the configuration of one environment. + Logger.getLogger("com.sleepycat.je").setLevel(parseLoggingLevel(cfg.getDBLoggingLevel(), cfg.dn())); + return envConfig; + } + /** + * Build the environment configuration the given configuration describes, and nothing else: no + * check of the cache size against the memory quota, no level set on the JE loggers. What a + * configuration change is checked as, applied to a running environment and held against, is + * built here. + * + * @param cfg The configuration to be parsed. + * @return An environment config instance corresponding to the configuration. + * @throws ConfigException If there is an error in the provided configuration. + */ + static EnvironmentConfig toEnvironmentConfig(JEBackendCfg cfg) throws ConfigException + { EnvironmentConfig envConfig = defaultConfig(); setDurability(envConfig, cfg.isDBTxnNoSync(), cfg.isDBTxnWriteNoSync()); setJEProperties(cfg, envConfig, cfg.dn().rdn().getFirstAVA().getAttributeValue()); @@ -386,6 +405,19 @@ static EnvironmentConfig parseConfigEntry(JEBackendCfg cfg) throws ConfigExcepti return setJEProperties(envConfig, cfg.getJEProperty(), attrMap); } + /** + * Get the name a JE property is configured under: the property of the backend configuration + * which is mapped to it, or the JE property's own name when it is set through je-property alone. + * + * @param jeProperty The JE property name. + * @return The name the operator changes it by. + */ + static String configuredNameOf(String jeProperty) + { + final String attrName = attrMap.get(jeProperty); + return attrName != null ? attrName.substring(ConfigConstants.NAME_PREFIX_CFG.length()) : jeProperty; + } + private static void validateDbCacheSize(long dbCacheSize) throws ConfigException { if (dbCacheSize != 0) @@ -424,6 +456,12 @@ else if (dbTxnWriteNoSync) { envConfig.setDurability(Durability.COMMIT_WRITE_NO_SYNC); } + else + { + // What JE falls back on when a configuration sets none - but set, so that a change back from + // either flag replaces the durability the environment runs with rather than leaving it be. + envConfig.setDurability(Durability.COMMIT_SYNC); + } } private static void setJEProperties(BackendCfg cfg, EnvironmentConfig envConfig, ByteString backendId) @@ -441,18 +479,22 @@ private static void setJEProperties(BackendCfg cfg, EnvironmentConfig envConfig, private static void setDBLoggingLevel(EnvironmentConfig envConfig, String loggingLevel, DN dn, boolean loggingFileHandlerOn) throws ConfigException { - Logger parent = Logger.getLogger("com.sleepycat.je"); + // Refused as a whole here; the level itself is set on the JE loggers by the open. + parseLoggingLevel(loggingLevel, dn); + final Level level = loggingFileHandlerOn ? Level.ALL : Level.OFF; + envConfig.setConfigParam(FILE_LOGGING_LEVEL, level.getName()); + } + + private static Level parseLoggingLevel(String loggingLevel, DN dn) throws ConfigException + { try { - parent.setLevel(Level.parse(loggingLevel)); + return Level.parse(loggingLevel); } catch (Exception e) { throw new ConfigException(ERR_JEB_INVALID_LOGGING_LEVEL.get(loggingLevel, dn)); } - - final Level level = loggingFileHandlerOn ? Level.ALL : Level.OFF; - envConfig.setConfigParam(FILE_LOGGING_LEVEL, level.getName()); } /** 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 044527a2e2..16ec72ebd1 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 @@ -42,6 +42,7 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; @@ -94,6 +95,8 @@ import com.sleepycat.je.OperationStatus; import com.sleepycat.je.Transaction; import com.sleepycat.je.TransactionConfig; +import com.sleepycat.je.config.ConfigParam; +import com.sleepycat.je.config.EnvironmentParams; /** Berkeley DB Java Edition (JE for short) database implementation of the {@link Storage} engine. */ public final class JEStorage implements Storage, Backupable, ConfigurationChangeListener, @@ -1283,7 +1286,8 @@ public boolean isConfigurationChangeAcceptable(JEBackendCfg newCfg, final long newSize = computeSize(newCfg); final MemoryQuota quota = serverContext.getMemoryQuota(); return (newSize <= reservedCacheSize || quota.isMemoryAvailable(newSize - reservedCacheSize)) - && checkConfigurationDirectories(newCfg, unacceptableReasons); + && checkConfigurationDirectories(newCfg, unacceptableReasons) + && checkEnvironmentConfiguration(newCfg, unacceptableReasons); } private long computeSize(JEBackendCfg cfg) @@ -1319,7 +1323,28 @@ else if (!memQuota.isMemoryAvailable(memQuota.memPercentToBytes(cfg.getDBCachePe return false; } } - return checkConfigurationDirectories(cfg, unacceptableReasons); + return checkConfigurationDirectories(cfg, unacceptableReasons) + && checkEnvironmentConfiguration(cfg, unacceptableReasons); + } + + /** + * Whether an environment can be configured from the given configuration. A durability which + * sets both flags, or a native property JE does not know, is refused here, before the change + * is written - rather than by the next open of the backend, which is where a configuration + * nothing checked used to fail. + */ + private static boolean checkEnvironmentConfiguration(JEBackendCfg cfg, List unacceptableReasons) + { + try + { + ConfigurableEnvironment.toEnvironmentConfig(cfg); + return true; + } + catch (ConfigException e) + { + unacceptableReasons.add(e.getMessageObject()); + return false; + } } private static boolean checkConfigurationDirectories(JEBackendCfg cfg, @@ -1385,6 +1410,12 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) ccr.addMessage( NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get(cfg.getBackendId(), configuredCacheSize, newCacheSize)); } + // An import runs the environment on a configuration of its own, which goes with it: the backend + // opens again on the configuration as changed once the import is over. + if (env != null && envConfig.getTransactional()) + { + applyToEnvironment(cfg, ccr); + } registerMonitoredDirectory(cfg); config = cfg; } @@ -1395,6 +1426,44 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) return ccr; } + /** + * Applies to the running environment what JE takes while it runs, and asks for a restart for what + * it takes at the open alone. The environment is configured when it opens, from the configuration + * as it is then: a change of a property JE accepts as mutable is handed to the environment here, + * and a change of one it does not is reported - the change result reaches the error log, where a + * change reported as applied while the environment ran on unchanged until its next open did not. + *

+ * The cache is the one mutable setting left where the open put it: it is sized with the memory + * reserved for it, and a change of its size asks for a restart above, at which the next open + * reserves the new size. + */ + private void applyToEnvironment(JEBackendCfg cfg, ConfigChangeResult ccr) throws ConfigException + { + final EnvironmentConfig next = ConfigurableEnvironment.toEnvironmentConfig(cfg); + final EnvironmentConfig running = env.getConfig(); + for (ConfigParam param : new TreeMap<>(EnvironmentParams.SUPPORTED_PARAMS).values()) + { + // Replication parameters are not set through an environment configuration; a multi-value + // parameter is not read as one value. Neither is set by this storage. + if (param.isMutable() || param.isForReplication() || param.isMultiValueParam()) + { + continue; + } + final String runningValue = running.getConfigParam(param.getName()); + final String nextValue = next.getConfigParam(param.getName()); + if (!Objects.equals(runningValue, nextValue)) + { + ccr.setAdminActionRequired(true); + ccr.addMessage(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.get( + ConfigurableEnvironment.configuredNameOf(param.getName()), cfg.getBackendId(), runningValue, nextValue)); + } + } + next.setConfigParam(MAX_MEMORY, running.getConfigParam(MAX_MEMORY)); + next.setConfigParam(MAX_MEMORY_PERCENT, running.getConfigParam(MAX_MEMORY_PERCENT)); + // What JE takes while it runs, of the properties the configuration sets; the rest it ignores. + env.setMutableConfig(next); + } + private void registerMonitoredDirectory(JEBackendCfg cfg) { diskMonitor.registerMonitoredDirectory( 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 3fa27a58a2..bd45b1c688 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 @@ -51,6 +51,7 @@ import org.forgerock.opendj.config.server.ConfigurationChangeListener; import org.forgerock.opendj.ldap.ByteSequence; import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.server.config.meta.PDBBackendCfgDefn; import org.forgerock.opendj.server.config.server.PDBBackendCfg; import org.forgerock.util.Reject; import org.opends.server.api.Backupable; @@ -1656,6 +1657,15 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) ccr.addMessage( NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get(cfg.getBackendId(), configuredCacheSize, newCacheSize)); } + if (db != null && cfg.getDBCheckpointerWakeupInterval() != db.getConfiguration().getCheckpointInterval()) + { + // The checkpoint interval is set on the PersistIt configuration when the database opens, and + // PersistIt takes no configuration once one is set: the next open of the backend applies it. + ccr.setAdminActionRequired(true); + ccr.addMessage(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.get( + PDBBackendCfgDefn.getInstance().getDBCheckpointerWakeupIntervalPropertyDefinition().getName(), + cfg.getBackendId(), db.getConfiguration().getCheckpointInterval(), cfg.getDBCheckpointerWakeupInterval())); + } registerMonitoredDirectory(cfg); config = cfg; commitPolicy = config.isDBTxnNoSync() ? SOFT : GROUP; diff --git a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties index 6590d4c3d1..41cb59a652 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1159,3 +1159,5 @@ ERR_CONFIG_BACKEND_DATA_CHANGE_FAILED_627=The compression, encoding and encrypti NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART_630=The change to the database cache of backend %s will not take effect \ until the backend is restarted: the cache the backend runs with, and the memory reserved for it, stay at the \ %d bytes the backend was opened with until then, and the %d bytes now configured are reserved by the next open +NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART_631=The change to %s of backend %s from %s to %s will not take \ + effect until the backend is restarted: the database takes that setting when it opens, not while it runs 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 index bd32ed6da8..a5bcdfc5b9 100644 --- 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 @@ -15,6 +15,15 @@ */ package org.opends.server.backends.jeb; +import static com.sleepycat.je.EnvironmentConfig.CLEANER_MIN_AGE; +import static com.sleepycat.je.EnvironmentConfig.CLEANER_MIN_UTILIZATION; +import static com.sleepycat.je.EnvironmentConfig.CLEANER_THREADS; +import static com.sleepycat.je.EnvironmentConfig.ENV_RUN_CLEANER; +import static com.sleepycat.je.EnvironmentConfig.EVICTOR_CORE_THREADS; +import static com.sleepycat.je.EnvironmentConfig.EVICTOR_KEEP_ALIVE; +import static com.sleepycat.je.EnvironmentConfig.EVICTOR_MAX_THREADS; +import static com.sleepycat.je.EnvironmentConfig.LOG_FILE_MAX; +import static com.sleepycat.je.EnvironmentConfig.LOG_ITERATOR_READ_SIZE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; @@ -22,12 +31,18 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.opends.messages.BackendMessages.ERR_CONFIG_JEB_DURABILITY_CONFLICT; import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART; +import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART; +import static org.opends.messages.ConfigMessages.ERR_CONFIG_JE_PROPERTY_INVALID; import static org.opends.server.util.StaticUtils.MB; import java.io.File; +import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.TreeSet; import org.forgerock.i18n.LocalizableMessage; import org.forgerock.opendj.config.server.ConfigChangeResult; @@ -39,6 +54,7 @@ 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.Importer; import org.opends.server.backends.pluggable.spi.ReadOperation; import org.opends.server.backends.pluggable.spi.ReadableTransaction; import org.opends.server.backends.pluggable.spi.TreeName; @@ -52,9 +68,14 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import com.sleepycat.je.Durability; +import com.sleepycat.je.Environment; +import com.sleepycat.je.EnvironmentMutableConfig; + /** * 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}. + * of the same cases on {@code PDBStorageTest} - and what a configuration change reaches of the + * environment it runs. */ @SuppressWarnings("javadoc") public class JEStorageTest extends DirectoryServerTestCase @@ -293,6 +314,7 @@ public void aChangeWhichLeavesTheCacheSizeAloneAsksForNothing() throws Exception storage.open(AccessMode.READ_WRITE); final JEBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + when(unchangedCache.isDBTxnWriteNoSync()).thenReturn(false); final ConfigChangeResult ccr = storage.applyConfigurationChange(unchangedCache); @@ -347,6 +369,229 @@ public void aReservationTheQuotaRefusedIsNotGivenBackOnClose() throws Exception assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); } + /** + * What JE takes while it runs - the cleaner, the evictor pool and the durability - is applied + * to the running environment by a configuration change, and the operator is asked for nothing. + * Built at the open and never touched again, the environment ran on unchanged until the next + * open while the change was reported as applied. + */ + @Test + public void aChangeOfWhatJETakesWhileItRunsReachesTheEnvironment() throws Exception + { + final Environment env = environmentOf(storage); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_MIN_UTILIZATION)).isEqualTo("50"); + + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCleanerMinUtilization()).thenReturn(60); + when(cfg.isDBRunCleaner()).thenReturn(false); + when(cfg.getDBEvictorCoreThreads()).thenReturn(2); + when(cfg.getDBEvictorMaxThreads()).thenReturn(4); + when(cfg.getDBEvictorKeepAlive()).thenReturn(120L); + when(cfg.getDBNumCleanerThreads()).thenReturn(3); + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + final EnvironmentMutableConfig running = env.getMutableConfig(); + assertThat(running.getConfigParam(CLEANER_MIN_UTILIZATION)).isEqualTo("60"); + assertThat(running.getConfigParam(ENV_RUN_CLEANER)).isEqualTo("false"); + assertThat(running.getConfigParam(EVICTOR_CORE_THREADS)).isEqualTo("2"); + assertThat(running.getConfigParam(EVICTOR_MAX_THREADS)).isEqualTo("4"); + // a JE duration, in microseconds + assertThat(running.getConfigParam(EVICTOR_KEEP_ALIVE)).isEqualTo("120000000"); + assertThat(running.getConfigParam(CLEANER_THREADS)).isEqualTo("3"); + } + + /** + * The durability follows the change every way. It is what the transactions of this storage + * commit with, taken from the environment handle: a change set on the handle reaches the next + * transaction - a change which sets neither flag included, which commits synchronously, set as + * such rather than left to what JE falls back on, since JE leaves a durability it has in place + * when it is handed none. + */ + @Test + public void aChangeOfTheDurabilityReachesTheEnvironmentEveryWay() throws Exception + { + final Environment env = environmentOf(storage); + // db-txn-write-no-sync is on by default + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_WRITE_NO_SYNC); + + final JEBackendCfg noSync = createBackendCfg(); + when(noSync.isDBTxnNoSync()).thenReturn(true); + when(noSync.isDBTxnWriteNoSync()).thenReturn(false); + assertThat(storage.applyConfigurationChange(noSync).getMessages()).isEmpty(); + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_NO_SYNC); + + final JEBackendCfg sync = createBackendCfg(); + when(sync.isDBTxnWriteNoSync()).thenReturn(false); + assertThat(storage.applyConfigurationChange(sync).getMessages()).isEmpty(); + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_SYNC); + + assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_WRITE_NO_SYNC); + } + + /** + * A native property set through je-property is applied when JE takes it while it runs, and + * asks for a restart when JE takes it at the open alone: the change result names the property, + * what the environment runs with and what is now configured, and the environment keeps the + * former. + */ + @Test + public void aNativePropertyIsAppliedOrAsksForARestartAsJETakesIt() throws Exception + { + final Environment env = environmentOf(storage); + final String readSizeAtOpen = env.getConfig().getConfigParam(LOG_ITERATOR_READ_SIZE); + assertThat(readSizeAtOpen).isNotEqualTo("16384"); + + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getJEProperty()).thenReturn( + new TreeSet<>(Arrays.asList(CLEANER_MIN_AGE + "=5", LOG_ITERATOR_READ_SIZE + "=16384"))); + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART + .get(LOG_ITERATOR_READ_SIZE, BACKEND_ID, readSizeAtOpen, "16384").toString()); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_MIN_AGE)).isEqualTo("5"); + assertThat(env.getConfig().getConfigParam(LOG_ITERATOR_READ_SIZE)).isEqualTo(readSizeAtOpen); + } + + /** + * A property JE takes at the open alone asks for a restart in the change result as well, not + * only in the property's definition: the definition reaches the reference documentation, the + * change result reaches the error log of the server which took the change. + */ + @Test + public void aChangeOfWhatJETakesAtTheOpenAloneAsksForARestart() throws Exception + { + final Environment env = environmentOf(storage); + final String fileMaxAtOpen = env.getConfig().getConfigParam(LOG_FILE_MAX); + + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBLogFileMax()).thenReturn(2 * Long.parseLong(fileMaxAtOpen)); + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART + .get("db-log-file-max", BACKEND_ID, fileMaxAtOpen, String.valueOf(2 * Long.parseLong(fileMaxAtOpen))) + .toString()); + assertThat(env.getConfig().getConfigParam(LOG_FILE_MAX)).isEqualTo(fileMaxAtOpen); + } + + /** + * The cache is the one thing JE takes while it runs which a change leaves alone: it stays at + * the size the open reserved, with the reservation, until the next open - the restart the + * change result asks for. What is applied to the environment is applied around it. + */ + @Test + public void aChangeWhileOpenLeavesTheCacheWhereTheOpenReservedIt() throws Exception + { + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + final Environment env = environmentOf(storage); + assertThat(env.getMutableConfig().getCacheSize()).isEqualTo(SMALL_CACHE); + + final JEBackendCfg cfg = createBackendCfg(2 * SMALL_CACHE); + when(cfg.getDBCleanerMinUtilization()).thenReturn(60); + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.ordinal()); + final EnvironmentMutableConfig running = env.getMutableConfig(); + assertThat(running.getCacheSize()).isEqualTo(SMALL_CACHE); + assertThat(running.getConfigParam(CLEANER_MIN_UTILIZATION)).isEqualTo("60"); + } + + /** + * An import runs the environment on a configuration of its own, which is thrown away with it: + * the backend opens again on the configuration as changed once the import is over, so a change + * which lands during one is neither applied to the import's environment nor reported against + * it - held to the import's configuration, every property the import sets differently would + * ask for a restart. + */ + @Test + public void aChangeDuringAnImportLeavesTheImportsEnvironmentAlone() throws Exception + { + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(), serverContext); + final Importer importer = storage.startImport(); + try + { + final Environment env = environmentOf(storage); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCleanerMinUtilization()).thenReturn(60); + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_MIN_UTILIZATION)).isEqualTo("50"); + } + finally + { + importer.close(); + } + } + + /** + * A configuration no environment can be built from is refused before it is written: a + * durability which sets both flags - db-txn-write-no-sync is on by default, so setting + * db-txn-no-sync alone is one - and a native property JE does not know. Nothing checked either + * before, and the backend failed to open on them at its next restart. + */ + @Test + public void aConfigurationNoEnvironmentCanBeBuiltFromIsRefused() throws Exception + { + final JEBackendCfg bothFlags = createBackendCfg(); + when(bothFlags.isDBTxnNoSync()).thenReturn(true); + final List reasons = new ArrayList<>(); + assertThat(storage.isConfigurationChangeAcceptable(bothFlags, reasons)).isFalse(); + assertThat(reasons).hasSize(1); + assertThat(reasons.get(0).toString()).isEqualTo(ERR_CONFIG_JEB_DURABILITY_CONFLICT.get().toString()); + + final JEBackendCfg unknownProperty = createBackendCfg(); + when(unknownProperty.getJEProperty()).thenReturn(new TreeSet<>(Arrays.asList("je.no.such.property=1"))); + reasons.clear(); + assertThat(storage.isConfigurationChangeAcceptable(unknownProperty, reasons)).isFalse(); + assertThat(reasons).hasSize(1); + assertThat(reasons.get(0).ordinal()).isEqualTo(ERR_CONFIG_JE_PROPERTY_INVALID.get("", "").ordinal()); + + reasons.clear(); + assertThat(JEStorage.isConfigurationAcceptable(bothFlags, reasons, serverContext)).isFalse(); + assertThat(reasons).hasSize(1); + assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(), new ArrayList())).isTrue(); + } + + /** A storage which is closed has no environment to apply a change to: the next open takes it. */ + @Test + public void aChangeWhileClosedTouchesNoEnvironment() throws Exception + { + storage.close(); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCleanerMinUtilization()).thenReturn(60); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + } + + /** The environment of the given storage, which it keeps to itself. */ + private static Environment environmentOf(JEStorage storage) throws Exception + { + final Field env = JEStorage.class.getDeclaredField("env"); + env.setAccessible(true); + return (Environment) env.get(storage); + } + /** A storage whose directory is a regular file, which no open of it can use. */ private JEStorage blockedStorage(JEBackendCfg cfg) throws Exception { 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 b40670b206..13beca062a 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 @@ -24,6 +24,7 @@ import static org.opends.messages.BackendMessages.*; import java.io.File; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -54,6 +55,7 @@ import org.testng.annotations.Test; import com.persistit.Exchange; +import com.persistit.Persistit; import com.persistit.exception.RollbackException; public class PDBStorageTest extends DirectoryServerTestCase @@ -703,6 +705,56 @@ protected PDBBackendCfg createBackendCfg() return createBackendCfg(0L); } + /** + * The checkpoint interval is set on the PersistIt configuration when the database opens, and + * PersistIt takes no configuration once one is set: a change of it asks for a restart, naming + * the interval the database runs with and the one now configured, and the database keeps the + * former. The property's definition says so as well now, which reaches the reference + * documentation; the change result reaches the error log of the server which took the change. + */ + @Test + public void aCheckpointIntervalChangedWhileOpenAsksForARestart() throws Exception + { + final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); + assertThat(checkpointIntervalOf(storage)).isEqualTo(intervalAtOpen); + final PDBBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCheckpointerWakeupInterval()).thenReturn(4 * intervalAtOpen); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART + .get("db-checkpointer-wakeup-interval", "PDBStorageTest", intervalAtOpen, 4 * intervalAtOpen) + .toString()); + assertThat(checkpointIntervalOf(storage)).isEqualTo(intervalAtOpen); + } + + /** A storage which is closed has no database to hold a change against: the next open takes it. */ + @Test + public void aCheckpointIntervalChangedWhileClosedAsksForNothing() throws Exception + { + storage.close(); + final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); + final PDBBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCheckpointerWakeupInterval()).thenReturn(4 * intervalAtOpen); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + } + + /** The checkpoint interval of the database the given storage runs, in seconds. */ + private static long checkpointIntervalOf(PDBStorage storage) throws Exception + { + final Field db = PDBStorage.class.getDeclaredField("db"); + db.setAccessible(true); + return ((Persistit) db.get(storage)).getConfiguration().getCheckpointInterval(); + } + /** A configuration whose cache is the given size in bytes, or a fifth of the quota when it is zero. */ private static PDBBackendCfg createBackendCfg(long cacheSize) {