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 92c909832c..7a66c6919c 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(); @@ -763,14 +772,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() @@ -817,14 +822,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. @@ -1277,15 +1279,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()); } /** @@ -1370,6 +1376,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 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; }