From f54783ec0e5edc9e9e42eed01640e5ad64ae7fe6 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 3a0c76e963..28f94d69ef 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 @@ -728,6 +728,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(); @@ -827,14 +836,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() @@ -881,14 +886,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. @@ -1457,15 +1459,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()); } /** @@ -1550,6 +1556,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 f4aa8439e1..43b4b7f432 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1170,3 +1170,6 @@ WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES_628=Index %s of backend base DN '%s' was ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED_629=Attribute %s of backend base DN '%s' is already indexed by %s. \ An attribute type is indexed once, whichever of its names or its OID the index is declared by, so change that \ index instead of adding another one for the same attribute +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 a3494c3da1..8611d5d81d 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 @@ -23,18 +23,25 @@ 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.CollectionUtils.newTreeSet; +import static org.opends.server.util.StaticUtils.MB; import java.io.File; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +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; @@ -89,6 +96,8 @@ public class JEStorageTest extends DirectoryServerTestCase private static final String LOCK_TIMEOUT_LONGER_THAN_SHORT_WINDOW = "300 ms"; /** How long a test waits for a thread it started, in seconds; well past any bound the tests configure. */ private static final long WAIT_SECONDS = 60; + /** 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; @@ -245,6 +254,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 { @@ -756,13 +888,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 c52dd078f2..6269845870 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 { @@ -550,6 +559,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() @@ -575,12 +707,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 cba9d511b50ceda5c973b394ebea95d868660657 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 23 Sep 2026 16:01:28 +0300 Subject: [PATCH 2/2] [#1063] Admit a change which does not grow the cache past the configured size whatever the storage holds After an open the quota refused, a storage holds nothing of the quota, and the admission of the last commit measured every change against that nothing: newSize <= reservedCacheSize failed for any cache, and isMemoryAvailable(newSize) asked for the very amount the quota had just refused. Every change listener of the backend entry is asked about every change, whatever property it moves, so a change of db-txn-no-sync, a disable, and the disable TaskUtils.disableBackend makes for an online import-ldif, rebuild-index or restore were all refused with UNWILLING_TO_PERFORM and no reason. The state needs no change of configuration to reach: the server does not check the backends it opens at startup against the quota. A size which does not grow past the one configured now asks the quota for nothing again, as it did before this PR; a growth is still measured against what the storage holds, so the case of a change admitted but not applied keeps its outcome. NOTE 630 no longer calls the size the backend was opened with reserved, which after a refused open it is not. PDBStorageTest and JEStorageTest, five more cases each and one extended, each killing a mutant which survived both classes: a change which leaves the cache alone is admitted after a refused reservation, a growth after it is measured against nothing held, a shrink is admitted with the quota exhausted, a cache sized by percent asks for a restart only when the percent moves, a storage which is not open asks for none, and a change back to the size the storage opened with asks for nothing. --- .../opends/server/backends/jeb/JEStorage.java | 10 +- .../server/backends/pdb/PDBStorage.java | 10 +- .../org/opends/messages/backend.properties | 4 +- .../server/backends/jeb/JEStorageTest.java | 119 +++++++++++++++++- .../server/backends/pdb/PDBStorageTest.java | 119 +++++++++++++++++- 5 files changed, 252 insertions(+), 10 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 28f94d69ef..5bcc833f52 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 @@ -1459,11 +1459,15 @@ public Set listTrees() public boolean isConfigurationChangeAcceptable(JEBackendCfg newCfg, List unacceptableReasons) { - // 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. + // A size which does not grow past the one configured asks the quota for nothing, as before: every + // change of the backend entry comes here, the disable of an online import included, and after an + // open the quota refused this storage holds nothing to measure such a change against. A growth is + // measured against what this storage holds, which is what the next open adds 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)) + return (newSize <= Math.max(reservedCacheSize, computeSize(config)) + || quota.isMemoryAvailable(newSize - reservedCacheSize)) && checkConfigurationDirectories(newCfg, unacceptableReasons); } 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..916e5a0fb8 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 @@ -1549,11 +1549,15 @@ private static ByteString valueToBytes(final Value value) public boolean isConfigurationChangeAcceptable(PDBBackendCfg newCfg, List unacceptableReasons) { - // 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. + // A size which does not grow past the one configured asks the quota for nothing, as before: every + // change of the backend entry comes here, the disable of an online import included, and after an + // open the quota refused this storage holds nothing to measure such a change against. A growth is + // measured against what this storage holds, which is what the next open adds 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)) + return (newSize <= Math.max(reservedCacheSize, computeSize(config)) + || quota.isMemoryAvailable(newSize - reservedCacheSize)) && checkConfigurationDirectories(newCfg, unacceptableReasons); } 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 43b4b7f432..5a90a1e194 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1171,5 +1171,5 @@ ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED_629=Attribute %s of backend base DN ' An attribute type is indexed once, whichever of its names or its OID the index is declared by, so change that \ index instead of adding another one for the same attribute 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 + until the backend is restarted: the cache the backend runs with stays at the %d bytes it 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 8611d5d81d..86a241bc43 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 @@ -312,6 +312,61 @@ public void aCacheSizeChangedWhileOpenAsksForARestart() throws Exception 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()); + + // The cache still runs at the size it was opened with, whatever the change before said: back to + // that size, there is nothing left to restart for. + final ConfigChangeResult back = storage.applyConfigurationChange(createBackendCfg(SMALL_CACHE)); + assertThat(back.adminActionRequired()).isFalse(); + assertThat(back.getMessages()).isEmpty(); + } + + /** + * The default cache is sized by db-cache-percent, db-cache-size left at 0: the restart is asked + * for by the size the percentage comes to, not by db-cache-size, which does not move. + */ + @Test + public void aCacheSizedByPercentAsksForARestartOnlyWhenThePercentChanges() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(0L, 10), serverContext); + storage.open(AccessMode.READ_WRITE); + final JEBackendCfg unchangedCache = createBackendCfg(0L, 10); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + final ConfigChangeResult unchanged = storage.applyConfigurationChange(unchangedCache); + assertThat(unchanged.adminActionRequired()).isFalse(); + assertThat(unchanged.getMessages()).isEmpty(); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg(0L, 20)); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get( + BACKEND_ID, quota.memPercentToBytes(10), quota.memPercentToBytes(20)).toString()); + } + + /** + * A storage which has not opened runs no cache to restart: a change of the cache size is picked + * up by the open, and asks for nothing. The listener is registered by the constructor already. + */ + @Test + public void aStorageWhichIsNotOpenAsksForNoRestart() throws Exception + { + final JEStorage unopened = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + try + { + final ConfigChangeResult ccr = unopened.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + + assertThat(ccr.adminActionRequired()).isFalse(); + for (LocalizableMessage message : ccr.getMessages()) + { + assertThat(message.ordinal()).isNotEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.ordinal()); + } + } + finally + { + unopened.close(); + } } /** A change which leaves the cache size alone asks for nothing, as before. */ @@ -377,6 +432,62 @@ public void aReservationTheQuotaRefusedIsNotGivenBackOnClose() throws Exception assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); } + /** + * After an open the quota refused, the storage holds nothing of the quota, and a change which + * leaves the cache size alone - any other property, the disable an online import makes - still + * asks the quota for nothing: every change of the backend entry is put to this storage. + */ + @Test + public void aChangeWhichLeavesTheCacheSizeAloneIsAdmittedAfterARefusedReservation() throws Exception + { + openWithTheReservationRefused(); + final JEBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + assertThat(storage.isConfigurationChangeAcceptable(unchangedCache, new ArrayList())) + .isTrue(); + } + + /** + * A growth after an open the quota refused is measured against what the storage holds, which is + * nothing: a quarter of a cache more than configured is a cache and a quarter more than held. + */ + @Test + public void aGrowthAfterARefusedReservationIsMeasuredAgainstNothingHeld() throws Exception + { + openWithTheReservationRefused(); + + assertThat(storage.isConfigurationChangeAcceptable( + createBackendCfg(SMALL_CACHE + SMALL_CACHE / 4), new ArrayList())) + .as("a cache and a quarter, with nothing held and half a cache free").isFalse(); + } + + /** A shrink asks the quota for nothing, even with none of it left. */ + @Test + public void aShrinkIsAdmittedWithTheQuotaExhausted() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new JEStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.acquireMemory(quota.getAvailableMemory())).isTrue(); + + assertThat(storage.isConfigurationChangeAcceptable( + createBackendCfg(SMALL_CACHE / 2), new ArrayList())).isTrue(); + } + + /** Opens a storage of one cache with half a cache left in the quota, so that its reservation is refused. */ + private void openWithTheReservationRefused() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + 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); + } + /** A storage whose directory is a regular file, which no open of it can use. */ private JEStorage blockedStorage(JEBackendCfg cfg) throws Exception { @@ -894,6 +1005,12 @@ private static JEBackendCfg createBackendCfg() /** 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) + { + return createBackendCfg(cacheSize, 20); + } + + /** A configuration whose cache is the given size in bytes, or the given percent of the quota when it is zero. */ + private static JEBackendCfg createBackendCfg(long cacheSize, int cachePercent) { final JEBackendCfg backendCfg = mockCfg(JEBackendCfg.class); when(backendCfg.dn()).thenReturn(DN.valueOf("ds-cfg-backend-id=" + BACKEND_ID + ",cn=Backends,cn=config")); @@ -901,7 +1018,7 @@ private static JEBackendCfg createBackendCfg(long cacheSize) when(backendCfg.getDBDirectory()).thenReturn(BACKEND_ID); when(backendCfg.getDBDirectoryPermissions()).thenReturn("755"); when(backendCfg.getDBCacheSize()).thenReturn(cacheSize); - when(backendCfg.getDBCachePercent()).thenReturn(20); + when(backendCfg.getDBCachePercent()).thenReturn(cachePercent); 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 6269845870..b7920e4b4d 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 @@ -617,6 +617,61 @@ public void aCacheSizeChangedWhileOpenAsksForARestart() throws Exception 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()); + + // The pool still runs at the size it was opened with, whatever the change before said: back to + // that size, there is nothing left to restart for. + final ConfigChangeResult back = storage.applyConfigurationChange(createBackendCfg(SMALL_CACHE)); + assertThat(back.adminActionRequired()).isFalse(); + assertThat(back.getMessages()).isEmpty(); + } + + /** + * The default cache is sized by db-cache-percent, db-cache-size left at 0: the restart is asked + * for by the size the percentage comes to, not by db-cache-size, which does not move. + */ + @Test + public void aCacheSizedByPercentAsksForARestartOnlyWhenThePercentChanges() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new PDBStorage(createBackendCfg(0L, 10), serverContext); + storage.open(AccessMode.READ_WRITE); + final PDBBackendCfg unchangedCache = createBackendCfg(0L, 10); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + final ConfigChangeResult unchanged = storage.applyConfigurationChange(unchangedCache); + assertThat(unchanged.adminActionRequired()).isFalse(); + assertThat(unchanged.getMessages()).isEmpty(); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg(0L, 20)); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.get( + "PDBStorageTest", quota.memPercentToBytes(10), quota.memPercentToBytes(20)).toString()); + } + + /** + * A storage which has not opened runs no cache to restart: a change of the cache size is picked + * up by the open, and asks for nothing. The listener is registered by the constructor already. + */ + @Test + public void aStorageWhichIsNotOpenAsksForNoRestart() throws Exception + { + final PDBStorage unopened = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + try + { + final ConfigChangeResult ccr = unopened.applyConfigurationChange(createBackendCfg(2 * SMALL_CACHE)); + + assertThat(ccr.adminActionRequired()).isFalse(); + for (LocalizableMessage message : ccr.getMessages()) + { + assertThat(message.ordinal()).isNotEqualTo(NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART.ordinal()); + } + } + finally + { + unopened.close(); + } } /** A change which leaves the cache size alone asks for nothing, as before. */ @@ -682,6 +737,62 @@ public void aReservationTheQuotaRefusedIsNotGivenBackOnClose() throws Exception assertThat(quota.getAvailableMemory()).isEqualTo(availableBefore); } + /** + * After an open the quota refused, the storage holds nothing of the quota, and a change which + * leaves the cache size alone - any other property, the disable an online import makes - still + * asks the quota for nothing: every change of the backend entry is put to this storage. + */ + @Test + public void aChangeWhichLeavesTheCacheSizeAloneIsAdmittedAfterARefusedReservation() throws Exception + { + openWithTheReservationRefused(); + final PDBBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); + when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + + assertThat(storage.isConfigurationChangeAcceptable(unchangedCache, new ArrayList())) + .isTrue(); + } + + /** + * A growth after an open the quota refused is measured against what the storage holds, which is + * nothing: a quarter of a cache more than configured is a cache and a quarter more than held. + */ + @Test + public void aGrowthAfterARefusedReservationIsMeasuredAgainstNothingHeld() throws Exception + { + openWithTheReservationRefused(); + + assertThat(storage.isConfigurationChangeAcceptable( + createBackendCfg(SMALL_CACHE + SMALL_CACHE / 4), new ArrayList())) + .as("a cache and a quarter, with nothing held and half a cache free").isFalse(); + } + + /** A shrink asks the quota for nothing, even with none of it left. */ + @Test + public void aShrinkIsAdmittedWithTheQuotaExhausted() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + storage = new PDBStorage(createBackendCfg(SMALL_CACHE), serverContext); + storage.open(AccessMode.READ_WRITE); + assertThat(quota.acquireMemory(quota.getAvailableMemory())).isTrue(); + + assertThat(storage.isConfigurationChangeAcceptable( + createBackendCfg(SMALL_CACHE / 2), new ArrayList())).isTrue(); + } + + /** Opens a storage of one cache with half a cache left in the quota, so that its reservation is refused. */ + private void openWithTheReservationRefused() throws Exception + { + final MemoryQuota quota = serverContext.getMemoryQuota(); + closeAndRemove(storage); + 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); + } + private void createTree() throws Exception { storage.write(new WriteOperation() @@ -713,13 +824,19 @@ protected PDBBackendCfg createBackendCfg() /** 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) + { + return createBackendCfg(cacheSize, 20); + } + + /** A configuration whose cache is the given size in bytes, or the given percent of the quota when it is zero. */ + private static PDBBackendCfg createBackendCfg(long cacheSize, int cachePercent) { PDBBackendCfg backendCfg = mockCfg(PDBBackendCfg.class); when(backendCfg.getBackendId()).thenReturn("PDBStorageTest"); when(backendCfg.getDBDirectory()).thenReturn("PDBStorageTest"); when(backendCfg.getDBDirectoryPermissions()).thenReturn("755"); when(backendCfg.getDBCacheSize()).thenReturn(cacheSize); - when(backendCfg.getDBCachePercent()).thenReturn(20); + when(backendCfg.getDBCachePercent()).thenReturn(cachePercent); return backendCfg; }