From f68630e5f54bdf8a4e377aadbd217b927338c8ee Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 06:54:30 +0300 Subject: [PATCH 1/4] [#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 | 51 +++- .../opends/server/backends/jeb/JEStorage.java | 73 ++++- .../server/backends/pdb/PDBStorage.java | 10 + .../org/opends/messages/backend.properties | 2 + .../server/backends/jeb/JEStorageTest.java | 250 +++++++++++++++++- .../server/backends/pdb/PDBStorageTest.java | 52 ++++ 8 files changed, 441 insertions(+), 9 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 c364a198a3..3b51122150 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 @@ -378,7 +378,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()); @@ -389,6 +407,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) @@ -430,6 +461,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) @@ -447,18 +484,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 852943b2e2..8624be03fa 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; @@ -95,6 +96,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, @@ -1470,7 +1473,8 @@ public boolean isConfigurationChangeAcceptable(JEBackendCfg newCfg, final MemoryQuota quota = serverContext.getMemoryQuota(); return (newSize <= Math.max(reservedCacheSize, computeSize(config)) || quota.isMemoryAvailable(newSize - reservedCacheSize)) - && checkConfigurationDirectories(newCfg, unacceptableReasons); + && checkConfigurationDirectories(newCfg, unacceptableReasons) + && checkEnvironmentConfiguration(newCfg, unacceptableReasons); } private long computeSize(JEBackendCfg cfg) @@ -1506,7 +1510,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, @@ -1572,6 +1597,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; } @@ -1582,6 +1613,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 916e5a0fb8..a3b1b6f564 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; @@ -1660,6 +1661,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 58d6a7b740..a23a6c819b 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1173,3 +1173,5 @@ ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED_629=Attribute %s of backend base DN ' 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: until then the cache keeps the size the backend was opened with, which the \ memory quota counts as %d bytes, and the next open reserves the %d bytes the quota counts for the new configuration +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 6150a5addf..f8fd4eaca2 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.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; @@ -23,18 +32,24 @@ 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.CollectionUtils.newTreeSet; 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.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 java.util.TreeSet; import org.forgerock.i18n.LocalizableMessage; import org.forgerock.opendj.config.server.ConfigChangeResult; @@ -46,6 +61,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.StorageRuntimeException; @@ -60,13 +76,17 @@ 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; import com.sleepycat.je.LockConflictException; import com.sleepycat.je.LockTimeoutException; /** * 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} - and the replay of a {@link JEStorage#write} whose - * transaction JE ends with a {@link LockConflictException}. + * of the same cases on {@code PDBStorageTest} - what a configuration change reaches of the + * environment it runs, and the replay of a {@link JEStorage#write} whose transaction JE ends with a + * {@link LockConflictException}. *

* The conflicts are the engine's own. A deadlock is made by two writers locking two records in opposite order, * which JE resolves by throwing at a random victim; a conflict on every attempt is made by a transaction which @@ -338,6 +358,7 @@ public void aCacheSizedByPercentAsksForARestartOnlyWhenThePercentChanges() throw storage.open(AccessMode.READ_WRITE); final JEBackendCfg unchangedCache = createBackendCfg(0L, 10); when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + when(unchangedCache.isDBTxnWriteNoSync()).thenReturn(false); final ConfigChangeResult unchanged = storage.applyConfigurationChange(unchangedCache); assertThat(unchanged.adminActionRequired()).isFalse(); @@ -383,6 +404,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); @@ -448,6 +470,7 @@ public void aChangeWhichLeavesTheCacheSizeAloneIsAdmittedAfterARefusedReservatio openWithTheReservationRefused(); final JEBackendCfg unchangedCache = createBackendCfg(SMALL_CACHE); when(unchangedCache.isDBTxnNoSync()).thenReturn(true); + when(unchangedCache.isDBTxnWriteNoSync()).thenReturn(false); assertThat(storage.isConfigurationChangeAcceptable(unchangedCache, new ArrayList())) .isTrue(); @@ -532,6 +555,229 @@ private void openWithTheReservationRefused() 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 49f3929a66..4979675398 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 @@ -866,6 +868,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) { From eebd25910dd3310774afb88c35f675b2ded3483a Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 23 Sep 2026 16:41:00 +0300 Subject: [PATCH 2/4] [#1068] Put a removed native property back to JE's default, and apply the rest of a change which moves the directory A mutable je-property removed from the configuration stayed in the running environment: setMutableConfig copies only what the configuration handed to it sets, onto a clone of the running one. applyToEnvironment now hands JE's default explicitly for a mutable parameter the configuration no longer sets, and asks for a restart (631) where JE does not take its own default as a value - je.cleaner.readSize, whose 0 means "computed at the open", is refused below 128. The two checks after the directory permissions returned on any message, and the note of a moved db-directory is one: a change which moved the directory as well skipped the environment, the checkpoint interval, the cache note and config = cfg. Both storages now return on the result code StorageUtils.addErrorMessage sets. Tests: the removal applied, the removal JE cannot take while it runs, a moved directory with the rest of the change (JE and PDB), a percentage change which leaves the live cache alone, and the checkpoint interval held against the database across two changes. --- .../opends/server/backends/jeb/JEStorage.java | 48 +++++++-- .../server/backends/pdb/PDBStorage.java | 7 +- .../server/backends/jeb/JEStorageTest.java | 101 ++++++++++++++++++ .../server/backends/pdb/PDBStorageTest.java | 33 ++++++ 4 files changed, 180 insertions(+), 9 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 8624be03fa..7e877f7527 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 @@ -53,6 +53,7 @@ import org.forgerock.opendj.config.server.ConfigException; import org.forgerock.opendj.ldap.ByteSequence; import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.ResultCode; import org.forgerock.util.Reject; import org.forgerock.opendj.config.server.ConfigurationChangeListener; import org.forgerock.opendj.server.config.server.JEBackendCfg; @@ -1576,13 +1577,15 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) || !cfg.getDBDirectory().equals(config.getDBDirectory())) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); - if (!ccr.getMessages().isEmpty()) + // By its result code: the note of a moved directory is in the result already, and the rest of + // the change is still applied and reported alongside it. + if (ccr.getResultCode() != ResultCode.SUCCESS) { return ccr; } setDBDirPermissions(newBackendDirectory, cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); - if (!ccr.getMessages().isEmpty()) + if (ccr.getResultCode() != ResultCode.SUCCESS) { return ccr; } @@ -1632,18 +1635,21 @@ private void applyToEnvironment(JEBackendCfg cfg, ConfigChangeResult ccr) throws { // 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()) + if (param.isForReplication() || param.isMultiValueParam()) { continue; } final String runningValue = running.getConfigParam(param.getName()); final String nextValue = next.getConfigParam(param.getName()); - if (!Objects.equals(runningValue, nextValue)) + if (Objects.equals(runningValue, nextValue) + || (param.isMutable() && (next.isConfigParamSet(param.getName()) + || resetsToDefault(next, param.getName(), nextValue)))) { - ccr.setAdminActionRequired(true); - ccr.addMessage(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.get( - ConfigurableEnvironment.configuredNameOf(param.getName()), cfg.getBackendId(), runningValue, nextValue)); + continue; } + 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)); @@ -1651,6 +1657,34 @@ private void applyToEnvironment(JEBackendCfg cfg, ConfigChangeResult ccr) throws env.setMutableConfig(next); } + /** + * Sets a mutable parameter the configuration no longer sets - a je-property removed - to JE's + * default, since the environment keeps the value it runs with of every parameter it is not handed. + * A default JE does not take as a value, such as the 0 of je.cleaner.readSize, which JE reads as + * "computed at the open", leaves the parameter to the next open. + * + * @param next the environment configuration handed to the running environment + * @param name the name of the parameter + * @param defaultValue JE's default of the parameter, as the configuration reads it + * @return whether the default is handed to the environment along with the rest + */ + private static boolean resetsToDefault(EnvironmentConfig next, String name, String defaultValue) + { + if (defaultValue == null) + { + return false; + } + try + { + next.setConfigParam(name, defaultValue); + return true; + } + catch (IllegalArgumentException e) + { + return false; + } + } + 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 a3b1b6f564..00d1c040f9 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.ldap.ResultCode; import org.forgerock.opendj.server.config.meta.PDBBackendCfgDefn; import org.forgerock.opendj.server.config.server.PDBBackendCfg; import org.forgerock.util.Reject; @@ -1640,13 +1641,15 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) || !cfg.getDBDirectory().equals(config.getDBDirectory())) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); - if (!ccr.getMessages().isEmpty()) + // By its result code: the note of a moved directory is in the result already, and the rest of + // the change is still applied and reported alongside it. + if (ccr.getResultCode() != ResultCode.SUCCESS) { return ccr; } setDBDirPermissions(newBackendDirectory, cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); - if (!ccr.getMessages().isEmpty()) + if (ccr.getResultCode() != ResultCode.SUCCESS) { return ccr; } 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 f8fd4eaca2..2ea53c3b26 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 @@ -17,6 +17,7 @@ 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_READ_SIZE; 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; @@ -34,10 +35,13 @@ 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_DIR_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.CollectionUtils.newTreeSet; import static org.opends.server.util.StaticUtils.MB; +import static org.opends.server.util.StaticUtils.getFileForPath; +import static org.opends.server.util.StaticUtils.recursiveDelete; import java.io.File; import java.lang.reflect.Field; @@ -106,6 +110,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"; + /** Where a change moves db-directory to; the environment stays where it opened until a restart. */ + private static final String MOVED_DB_DIRECTORY = BACKEND_ID + "-moved"; /** A window no run of replays can spend, so that a test of the attempt cap is only ever ended by the cap. */ private static final long UNREACHABLE_RETRY_WINDOW_NANOS = 300L * 1000L * 1000L * 1000L; //5 min /** A window a single attempt outlasts, so that a test of the window reaches it without seconds of build time. */ @@ -645,6 +651,101 @@ public void aNativePropertyIsAppliedOrAsksForARestartAsJETakesIt() throws Except assertThat(env.getConfig().getConfigParam(LOG_ITERATOR_READ_SIZE)).isEqualTo(readSizeAtOpen); } + /** + * A native property JE takes while it runs goes back to JE's default when it is removed from + * je-property: the environment keeps the value it runs with of every parameter it is not handed, + * so the removal hands it the default explicitly. + */ + @Test + public void aRemovedNativePropertyGoesBackToJEsDefault() throws Exception + { + final Environment env = environmentOf(storage); + final String minAgeAtOpen = env.getMutableConfig().getConfigParam(CLEANER_MIN_AGE); + assertThat(minAgeAtOpen).isNotEqualTo("5"); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getJEProperty()).thenReturn(new TreeSet<>(Arrays.asList(CLEANER_MIN_AGE + "=5"))); + assertThat(storage.applyConfigurationChange(cfg).getMessages()).isEmpty(); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_MIN_AGE)).isEqualTo("5"); + + final ConfigChangeResult removed = storage.applyConfigurationChange(createBackendCfg()); + + assertThat(removed.adminActionRequired()).isFalse(); + assertThat(removed.getMessages()).isEmpty(); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_MIN_AGE)).isEqualTo(minAgeAtOpen); + } + + /** + * A native property whose default JE does not take as a value - the 0 of je.cleaner.readSize + * stands for "computed at the open" - cannot be put back while the environment runs: its removal + * asks for a restart instead of failing the change, and the environment keeps what it runs with. + */ + @Test + public void aRemovedNativePropertyJECannotPutBackWhileItRunsAsksForARestart() throws Exception + { + final Environment env = environmentOf(storage); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getJEProperty()).thenReturn(new TreeSet<>(Arrays.asList(CLEANER_READ_SIZE + "=16384"))); + assertThat(storage.applyConfigurationChange(cfg).getMessages()).isEmpty(); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_READ_SIZE)).isEqualTo("16384"); + + final ConfigChangeResult removed = storage.applyConfigurationChange(createBackendCfg()); + + assertThat(removed.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(removed.adminActionRequired()).isTrue(); + assertThat(removed.getMessages()).hasSize(1); + assertThat(removed.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART + .get(CLEANER_READ_SIZE, BACKEND_ID, "16384", "0").toString()); + assertThat(env.getMutableConfig().getConfigParam(CLEANER_READ_SIZE)).isEqualTo("16384"); + } + + /** + * A change which moves db-directory as well is still applied and reported: the note of the moved + * directory, which asks for a restart of its own, does not end the change before the rest of it. + */ + @Test + public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws Exception + { + final Environment env = environmentOf(storage); + final String fileMaxAtOpen = env.getConfig().getConfigParam(LOG_FILE_MAX); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBDirectory()).thenReturn(MOVED_DB_DIRECTORY); + when(cfg.isDBTxnNoSync()).thenReturn(true); + when(cfg.isDBTxnWriteNoSync()).thenReturn(false); + when(cfg.getDBLogFileMax()).thenReturn(2 * Long.parseLong(fileMaxAtOpen)); + try + { + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(2); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); + assertThat(ccr.getMessages().get(1).ordinal()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.ordinal()); + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_NO_SYNC); + } + finally + { + recursiveDelete(getFileForPath(MOVED_DB_DIRECTORY)); + } + } + + /** + * The cache a percentage sizes stays where the open put it as well: the percentage the environment + * runs with is handed back to it along with its size, or a change of db-cache-percent would resize + * the live cache while the quota still holds what the open reserved. + */ + @Test + public void aCachePercentChangedWhileOpenLeavesTheCacheWhereTheOpenReservedIt() throws Exception + { + final Environment env = environmentOf(storage); + final long cacheAtOpen = env.getMutableConfig().getCacheSize(); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBCachePercent()).thenReturn(30); + + assertThat(storage.applyConfigurationChange(cfg).adminActionRequired()).isTrue(); + assertThat(env.getMutableConfig().getCacheSize()).isEqualTo(cacheAtOpen); + } + /** * 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 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 4979675398..18f55dac9d 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 @@ -892,6 +892,39 @@ public void aCheckpointIntervalChangedWhileOpenAsksForARestart() throws Exceptio .get("db-checkpointer-wakeup-interval", "PDBStorageTest", intervalAtOpen, 4 * intervalAtOpen) .toString()); assertThat(checkpointIntervalOf(storage)).isEqualTo(intervalAtOpen); + + // held against the interval the database runs with, not against the configuration the last change left: + // a later change which leaves the interval where the first one put it still asks for the restart, + assertThat(storage.applyConfigurationChange(cfg).adminActionRequired()).isTrue(); + // and one which puts it back to what the database runs with asks for nothing + assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); + } + + /** + * A change which moves db-directory as well is still reported whole: the note of the moved + * directory, which asks for a restart of its own, does not end the change before the rest of it. + */ + @Test + public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception + { + final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); + final PDBBackendCfg cfg = createBackendCfg(); + when(cfg.getDBDirectory()).thenReturn("PDBStorageTest-moved"); + when(cfg.getDBCheckpointerWakeupInterval()).thenReturn(4 * intervalAtOpen); + try + { + final ConfigChangeResult ccr = storage.applyConfigurationChange(cfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ccr.getMessages()).hasSize(2); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); + assertThat(ccr.getMessages().get(1).ordinal()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.ordinal()); + } + finally + { + recursiveDelete(getFileForPath("PDBStorageTest-moved")); + } } /** A storage which is closed has no database to hold a change against: the next open takes it. */ From 09aef5e188e7d6cde765d7a3923cf38ec1224852 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 24 Sep 2026 17:48:20 +0300 Subject: [PATCH 3/4] [#1068] Name the directory the storage runs on until the restart, and hold a moved directory against it Once a change which moves db-directory went on to config = cfg, getDirectory() - read from the configuration - named the new, empty directory while the environment ran on the one it was opened on: an online backup listed no file and reported success, and close() deregistered the new directory from the disk monitor instead of the one registered. Both storages now return the directory they were built on, which a new storage - the next open of the backend - builds from the moved configuration. The move was also compared with the configuration as last changed, so only the first change asked for the restart. Both storages now compare the new directory with the one they run on: a later change asks for the restart again, one which moves back asks for nothing, and the note names the backend's directories rather than the db-directory values. Tests: a moved directory keeps the directory the storage runs on and a non-empty backup list, and asks again on a later change and for nothing when moved back (JE and PDB); a directory mode the server could not use refuses the change whole (JE and PDB); the static acceptability check refuses an unknown native property for that reason, and the durability conflict for its own. --- .../opends/server/backends/jeb/JEStorage.java | 13 ++++-- .../server/backends/pdb/PDBStorage.java | 13 ++++-- .../server/backends/jeb/JEStorageTest.java | 42 +++++++++++++++++++ .../server/backends/pdb/PDBStorageTest.java | 32 ++++++++++++++ 4 files changed, 92 insertions(+), 8 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 7e877f7527..4ecdc894a9 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 @@ -1213,7 +1213,9 @@ public boolean supportsBackupAndRestore() @Override public File getDirectory() { - return getBackendDirectory(config); + // The directory the storage runs on: a db-directory moved while it runs is used from the next open, + // which a new storage makes. + return backendDirectory; } private static File getBackendDirectory(JEBackendCfg cfg) @@ -1559,9 +1561,12 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) try { File newBackendDirectory = getBackendDirectory(cfg); + // Against the directory the storage runs on rather than the configuration as last changed, so + // that a later change still asks for the restart a move is waiting for. + final boolean moved = !newBackendDirectory.equals(backendDirectory); // Create the directory if it doesn't exist. - if (!cfg.getDBDirectory().equals(config.getDBDirectory())) + if (moved) { checkDBDirExistsOrCanCreate(newBackendDirectory, ccr, false); if (!ccr.getMessages().isEmpty()) @@ -1570,11 +1575,11 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) } ccr.setAdminActionRequired(true); - ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(config.getDBDirectory(), cfg.getDBDirectory())); + ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(backendDirectory, newBackendDirectory)); } if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(config.getDBDirectoryPermissions()) - || !cfg.getDBDirectory().equals(config.getDBDirectory())) + || moved) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); // By its result code: the note of a moved directory is in the result already, and the rest of 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 00d1c040f9..842610fcdd 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 @@ -1337,7 +1337,9 @@ public boolean supportsBackupAndRestore() @Override public File getDirectory() { - return getBackendDirectory(config); + // The directory the storage runs on: a db-directory moved while it runs is used from the next open, + // which a new storage makes. + return backendDirectory; } private static File getBackendDirectory(PDBBackendCfg cfg) @@ -1623,9 +1625,12 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) try { File newBackendDirectory = getBackendDirectory(cfg); + // Against the directory the storage runs on rather than the configuration as last changed, so + // that a later change still asks for the restart a move is waiting for. + final boolean moved = !newBackendDirectory.equals(backendDirectory); // Create the directory if it doesn't exist. - if(!cfg.getDBDirectory().equals(config.getDBDirectory())) + if (moved) { checkDBDirExistsOrCanCreate(newBackendDirectory, ccr, false); if (!ccr.getMessages().isEmpty()) @@ -1634,11 +1639,11 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) } ccr.setAdminActionRequired(true); - ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(config.getDBDirectory(), cfg.getDBDirectory())); + ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(backendDirectory, newBackendDirectory)); } if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(config.getDBDirectoryPermissions()) - || !cfg.getDBDirectory().equals(config.getDBDirectory())) + || moved) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); // By its result code: the note of a moved directory is in the result already, and the rest of 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 2ea53c3b26..6df806874f 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 @@ -37,6 +37,7 @@ import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART; import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_DIR_REQUIRES_RESTART; import static org.opends.messages.BackendMessages.NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART; +import static org.opends.messages.ConfigMessages.ERR_CONFIG_BACKEND_INSANE_MODE; import static org.opends.messages.ConfigMessages.ERR_CONFIG_JE_PROPERTY_INVALID; import static org.opends.server.util.CollectionUtils.newTreeSet; import static org.opends.server.util.StaticUtils.MB; @@ -701,11 +702,15 @@ public void aRemovedNativePropertyJECannotPutBackWhileItRunsAsksForARestart() th /** * A change which moves db-directory as well is still applied and reported: the note of the moved * directory, which asks for a restart of its own, does not end the change before the rest of it. + * The storage keeps naming the directory the environment runs on, which a backup lists, and the + * move is held against that directory: a later change still asks for the restart, and one which + * moves back asks for nothing. */ @Test public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws Exception { final Environment env = environmentOf(storage); + final File directoryAtOpen = storage.getDirectory(); final String fileMaxAtOpen = env.getConfig().getConfigParam(LOG_FILE_MAX); final JEBackendCfg cfg = createBackendCfg(); when(cfg.getDBDirectory()).thenReturn(MOVED_DB_DIRECTORY); @@ -722,6 +727,14 @@ public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); assertThat(ccr.getMessages().get(1).ordinal()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.ordinal()); assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_NO_SYNC); + assertThat(storage.getDirectory()).isEqualTo(directoryAtOpen); + assertThat(storage.getFilesToBackup().hasNext()).isTrue(); + + final ConfigChangeResult again = storage.applyConfigurationChange(cfg); + assertThat(again.getMessages()).hasSize(2); + assertThat(again.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); + + assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); } finally { @@ -729,6 +742,29 @@ public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws } } + /** + * A directory mode the server itself could not use refuses the change whole: nothing of it is + * written to the running directory, and nothing else of it reaches the environment. + */ + @Test + public void aChangeToAnInsaneDirectoryModeIsRefusedWhole() throws Exception + { + final Environment env = environmentOf(storage); + final Durability before = env.getConfig().getDurability(); + final JEBackendCfg insaneMode = createBackendCfg(); + when(insaneMode.getDBDirectoryPermissions()).thenReturn("500"); + when(insaneMode.isDBTxnNoSync()).thenReturn(true); + when(insaneMode.isDBTxnWriteNoSync()).thenReturn(false); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(insaneMode); + + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(ERR_CONFIG_BACKEND_INSANE_MODE.ordinal()); + assertThat(env.getConfig().getDurability()).isEqualTo(before); + assertThat(storage.getDirectory().canWrite()).isTrue(); + } + /** * The cache a percentage sizes stays where the open put it as well: the percentage the environment * runs with is handed back to it along with its size, or a change of db-cache-percent would resize @@ -853,6 +889,12 @@ public void aConfigurationNoEnvironmentCanBeBuiltFromIsRefused() throws Exceptio reasons.clear(); assertThat(JEStorage.isConfigurationAcceptable(bothFlags, reasons, serverContext)).isFalse(); assertThat(reasons).hasSize(1); + assertThat(reasons.get(0).toString()).isEqualTo(ERR_CONFIG_JEB_DURABILITY_CONFLICT.get().toString()); + + reasons.clear(); + assertThat(JEStorage.isConfigurationAcceptable(unknownProperty, reasons, serverContext)).isFalse(); + assertThat(reasons).hasSize(1); + assertThat(reasons.get(0).ordinal()).isEqualTo(ERR_CONFIG_JE_PROPERTY_INVALID.get("", "").ordinal()); assertThat(storage.isConfigurationChangeAcceptable(createBackendCfg(), new ArrayList())).isTrue(); } 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 18f55dac9d..6a316c92e3 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java @@ -22,6 +22,7 @@ import static org.opends.server.util.StaticUtils.*; import static org.forgerock.opendj.ldap.ByteString.*; import static org.opends.messages.BackendMessages.*; +import static org.opends.messages.ConfigMessages.ERR_CONFIG_BACKEND_INSANE_MODE; import java.io.File; import java.lang.reflect.Field; @@ -903,10 +904,14 @@ public void aCheckpointIntervalChangedWhileOpenAsksForARestart() throws Exceptio /** * A change which moves db-directory as well is still reported whole: the note of the moved * directory, which asks for a restart of its own, does not end the change before the rest of it. + * The storage keeps naming the directory it runs on, which a backup lists, and the move is held + * against that directory: a later change still asks for the restart, and one which moves back asks + * for nothing. */ @Test public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception { + final File directoryAtOpen = storage.getDirectory(); final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); final PDBBackendCfg cfg = createBackendCfg(); when(cfg.getDBDirectory()).thenReturn("PDBStorageTest-moved"); @@ -920,6 +925,13 @@ public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception assertThat(ccr.getMessages()).hasSize(2); assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); assertThat(ccr.getMessages().get(1).ordinal()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART.ordinal()); + assertThat(storage.getDirectory()).isEqualTo(directoryAtOpen); + + final ConfigChangeResult again = storage.applyConfigurationChange(cfg); + assertThat(again.getMessages()).hasSize(2); + assertThat(again.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); + + assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); } finally { @@ -927,6 +939,26 @@ public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception } } + /** + * A directory mode the server itself could not use refuses the change whole: nothing of it is + * written to the running directory, and the rest of it is not reported as waiting for a restart. + */ + @Test + public void aChangeToAnInsaneDirectoryModeIsRefusedWhole() throws Exception + { + final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); + final PDBBackendCfg insaneMode = createBackendCfg(); + when(insaneMode.getDBDirectoryPermissions()).thenReturn("500"); + when(insaneMode.getDBCheckpointerWakeupInterval()).thenReturn(4 * intervalAtOpen); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(insaneMode); + + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).hasSize(1); + assertThat(ccr.getMessages().get(0).ordinal()).isEqualTo(ERR_CONFIG_BACKEND_INSANE_MODE.ordinal()); + assertThat(storage.getDirectory().canWrite()).isTrue(); + } + /** A storage which is closed has no database to hold a change against: the next open takes it. */ @Test public void aCheckpointIntervalChangedWhileClosedAsksForNothing() throws Exception From a7406cf008aa35bb0b30a7ccd1af1f1e9c6f2f5b Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 25 Sep 2026 11:27:58 +0300 Subject: [PATCH 4/4] [#1068] Ask for a restart for an off-heap cache switched on or off, and hold the directory mode against the running directory JE takes a change of je.maxOffHeapMemory while it runs, but not one between zero and non-zero: EnvironmentImpl.doSetMutableConfig takes the new value as its own before OffHeapCache refuses it, so the change failed and every later change of the backend entry failed the same way until the restart. Such a change now asks for a restart with 631, and the environment is handed back the off-heap cache size it runs with. A change of the size alone is still applied. The directory mode was compared with the configuration as last changed, while a mode changed along with a move of db-directory is written to the directory moved to alone: a later change which moved back with the same mode left the running directory on its former mode. Both storages now hold the mode against the one last written to the directory they run on, which the open and a change which does not move the directory write. Tests: switching the off-heap cache on asks for a restart and the next change succeeds; switching it off does too, after a change of its size which is applied, and a durability change still reaches the environment; a mode changed along with a move reaches the running directory when moved back, and a mode the open wrote is what a later change is held against (JE and PDB); the disk monitor registers the directory the storage runs on across a move, and a storage closed with the move pending deregisters that directory (JE and PDB). --- .../opends/server/backends/jeb/JEStorage.java | 42 +++- .../server/backends/pdb/PDBStorage.java | 14 +- .../server/backends/jeb/JEStorageTest.java | 186 +++++++++++++++++- .../server/backends/pdb/PDBStorageTest.java | 117 ++++++++++- 4 files changed, 349 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 4ecdc894a9..a70f9a0065 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 @@ -725,6 +725,12 @@ private WriteableTransaction newWriteableTransaction(Transaction txn) private final ServerContext serverContext; private final File backendDirectory; + /** + * The mode last written to the directory the storage runs on. A mode changed along with a move of + * db-directory is written to the directory moved to alone, so the configuration as last changed + * does not say what the running directory has. + */ + private String runningDirectoryPermissions; private JEBackendCfg config; private AccessMode accessMode; @@ -795,6 +801,7 @@ public JEStorage(final JEBackendCfg cfg, ServerContext serverContext) throws Con this.maxRetries = maxRetries; this.retryWindowNanos = retryWindowNanos; backendDirectory = getBackendDirectory(cfg); + runningDirectoryPermissions = cfg.getDBDirectoryPermissions(); config = cfg; cfg.addJEChangeListener(this); } @@ -992,6 +999,7 @@ private boolean backendDirectoryIncomplete() private void open0() throws ConfigException { setupStorageFiles(backendDirectory, config.getDBDirectoryPermissions(), config.dn()); + runningDirectoryPermissions = config.getDBDirectoryPermissions(); try { env = new Environment(backendDirectory, envConfig); @@ -1578,7 +1586,7 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(backendDirectory, newBackendDirectory)); } - if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(config.getDBDirectoryPermissions()) + if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(runningDirectoryPermissions) || moved) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); @@ -1594,6 +1602,10 @@ public ConfigChangeResult applyConfigurationChange(JEBackendCfg cfg) { return ccr; } + if (!moved) + { + runningDirectoryPermissions = cfg.getDBDirectoryPermissions(); + } } final long newCacheSize = computeSize(cfg); if (env != null && newCacheSize != configuredCacheSize) @@ -1647,8 +1659,9 @@ private void applyToEnvironment(JEBackendCfg cfg, ConfigChangeResult ccr) throws final String runningValue = running.getConfigParam(param.getName()); final String nextValue = next.getConfigParam(param.getName()); if (Objects.equals(runningValue, nextValue) - || (param.isMutable() && (next.isConfigParamSet(param.getName()) - || resetsToDefault(next, param.getName(), nextValue)))) + || (param.isMutable() + && !switchesOffHeapCache(param.getName(), runningValue, nextValue) + && (next.isConfigParamSet(param.getName()) || resetsToDefault(next, param.getName(), nextValue)))) { continue; } @@ -1658,10 +1671,33 @@ private void applyToEnvironment(JEBackendCfg cfg, ConfigChangeResult ccr) throws } next.setConfigParam(MAX_MEMORY, running.getConfigParam(MAX_MEMORY)); next.setConfigParam(MAX_MEMORY_PERCENT, running.getConfigParam(MAX_MEMORY_PERCENT)); + // The off-heap cache is switched on or off by the next open alone, which the change asks for above. + final String offHeapRunning = running.getConfigParam(MAX_OFF_HEAP_MEMORY); + if (switchesOffHeapCache(MAX_OFF_HEAP_MEMORY, offHeapRunning, next.getConfigParam(MAX_OFF_HEAP_MEMORY))) + { + next.setConfigParam(MAX_OFF_HEAP_MEMORY, offHeapRunning); + } // What JE takes while it runs, of the properties the configuration sets; the rest it ignores. env.setMutableConfig(next); } + /** + * Tells whether a change of the given parameter switches JE's off-heap cache on or off. JE takes a + * change of the off-heap cache size while it runs, but not one between zero and non-zero: it throws + * once it has already taken the new value as its own, and so throws again on every change which + * follows, until the next open - which does it. + * + * @param name the name of the parameter + * @param runningValue the value the environment runs with + * @param nextValue the value configured + * @return whether the change switches the off-heap cache on or off + */ + private static boolean switchesOffHeapCache(String name, String runningValue, String nextValue) + { + return MAX_OFF_HEAP_MEMORY.equals(name) + && (Long.parseLong(runningValue) > 0) != (Long.parseLong(nextValue) > 0); + } + /** * Sets a mutable parameter the configuration no longer sets - a je-property removed - to JE's * default, since the environment keeps the value it runs with of every parameter it is not handed. 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 842610fcdd..d0ea8de7b0 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 @@ -1001,6 +1001,12 @@ private StorageImpl newStorageImpl() { private final ServerContext serverContext; private final File backendDirectory; + /** + * The mode last written to the directory the storage runs on. A mode changed along with a move of + * db-directory is written to the directory moved to alone, so the configuration as last changed + * does not say what the running directory has. + */ + private String runningDirectoryPermissions; private CommitPolicy commitPolicy; private AccessMode accessMode; /** It is NULL when opening the storage READ-ONLY and no files have been created yet. */ @@ -1068,6 +1074,7 @@ public PDBStorage(final PDBBackendCfg cfg, ServerContext serverContext) throws C this.maxRetries = maxRetries; this.retryWindowNanos = retryWindowNanos; backendDirectory = getBackendDirectory(cfg); + runningDirectoryPermissions = cfg.getDBDirectoryPermissions(); config = cfg; cfg.addPDBChangeListener(this); } @@ -1235,6 +1242,7 @@ private boolean backendDirectoryIncomplete() private void open0(final Configuration dbCfg) throws ConfigException { setupStorageFiles(backendDirectory, config.getDBDirectoryPermissions(), config.dn()); + runningDirectoryPermissions = config.getDBDirectoryPermissions(); try { db = new Persistit(dbCfg); @@ -1642,7 +1650,7 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) ccr.addMessage(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.get(backendDirectory, newBackendDirectory)); } - if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(config.getDBDirectoryPermissions()) + if (!cfg.getDBDirectoryPermissions().equalsIgnoreCase(runningDirectoryPermissions) || moved) { checkDBDirPermissions(cfg.getDBDirectoryPermissions(), cfg.dn(), ccr); @@ -1658,6 +1666,10 @@ public ConfigChangeResult applyConfigurationChange(PDBBackendCfg cfg) { return ccr; } + if (!moved) + { + runningDirectoryPermissions = cfg.getDBDirectoryPermissions(); + } } final long newCacheSize = computeSize(cfg); if (db != null && newCacheSize != configuredCacheSize) 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 6df806874f..dc23fb2286 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 @@ -25,11 +25,16 @@ 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 com.sleepycat.je.EnvironmentConfig.MAX_OFF_HEAP_MEMORY; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; import static org.forgerock.opendj.ldap.ByteString.valueOfUtf8; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -46,6 +51,9 @@ import java.io.File; import java.lang.reflect.Field; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -63,8 +71,10 @@ import org.forgerock.opendj.ldap.DN; import org.forgerock.opendj.ldap.ResultCode; import org.forgerock.opendj.server.config.server.JEBackendCfg; +import org.mockito.ArgumentCaptor; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; +import org.opends.server.api.DiskSpaceMonitorHandler; import org.opends.server.backends.pluggable.spi.AccessMode; import org.opends.server.backends.pluggable.spi.Importer; import org.opends.server.backends.pluggable.spi.ReadOperation; @@ -76,6 +86,7 @@ import org.opends.server.core.MemoryQuota; import org.opends.server.core.ServerContext; import org.opends.server.extensions.DiskSpaceMonitor; +import org.testng.SkipException; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; @@ -699,16 +710,81 @@ public void aRemovedNativePropertyJECannotPutBackWhileItRunsAsksForARestart() th assertThat(env.getMutableConfig().getConfigParam(CLEANER_READ_SIZE)).isEqualTo("16384"); } + /** + * JE takes a change of its off-heap cache size while it runs, but not one which switches the cache + * on: switching it on asks for a restart, and the environment keeps running without it and takes + * the changes which follow. + */ + @Test + public void aNativePropertyWhichSwitchesTheOffHeapCacheOnAsksForARestart() throws Exception + { + final Environment env = environmentOf(storage); + assertThat(env.getConfig().getConfigParam(MAX_OFF_HEAP_MEMORY)).isEqualTo("0"); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getJEProperty()).thenReturn(newTreeSet(MAX_OFF_HEAP_MEMORY + "=" + MB)); + + 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(MAX_OFF_HEAP_MEMORY, BACKEND_ID, "0", String.valueOf(MB)).toString()); + assertThat(env.getConfig().getConfigParam(MAX_OFF_HEAP_MEMORY)).isEqualTo("0"); + + final ConfigChangeResult removed = storage.applyConfigurationChange(createBackendCfg()); + assertThat(removed.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(removed.getMessages()).isEmpty(); + } + + /** + * Nor does JE take a change which switches its off-heap cache off: the removal of the native + * property asks for a restart, and the environment keeps its cache and takes the changes which + * follow - a change of its size among them. + */ + @Test + public void aRemovedNativePropertyWhichSwitchesTheOffHeapCacheOffAsksForARestart() throws Exception + { + final JEBackendCfg withOffHeapCache = createBackendCfg(); + when(withOffHeapCache.getJEProperty()).thenReturn(newTreeSet(MAX_OFF_HEAP_MEMORY + "=" + MB)); + closeAndRemove(storage); + storage = new JEStorage(withOffHeapCache, serverContext); + storage.open(AccessMode.READ_WRITE); + final Environment env = environmentOf(storage); + assertThat(env.getConfig().getConfigParam(MAX_OFF_HEAP_MEMORY)).isEqualTo(String.valueOf(MB)); + + final JEBackendCfg resized = createBackendCfg(); + when(resized.getJEProperty()).thenReturn(newTreeSet(MAX_OFF_HEAP_MEMORY + "=" + 2 * MB)); + assertThat(storage.applyConfigurationChange(resized).getMessages()).isEmpty(); + assertThat(env.getConfig().getConfigParam(MAX_OFF_HEAP_MEMORY)).isEqualTo(String.valueOf(2 * MB)); + + final ConfigChangeResult removed = storage.applyConfigurationChange(createBackendCfg()); + + assertThat(removed.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(removed.adminActionRequired()).isTrue(); + assertThat(removed.getMessages()).hasSize(1); + assertThat(removed.getMessages().get(0).toString()).isEqualTo(NOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART + .get(MAX_OFF_HEAP_MEMORY, BACKEND_ID, String.valueOf(2 * MB), "0").toString()); + assertThat(env.getConfig().getConfigParam(MAX_OFF_HEAP_MEMORY)).isEqualTo(String.valueOf(2 * MB)); + + final JEBackendCfg durabilityChanged = createBackendCfg(); + when(durabilityChanged.isDBTxnNoSync()).thenReturn(true); + when(durabilityChanged.isDBTxnWriteNoSync()).thenReturn(false); + assertThat(storage.applyConfigurationChange(durabilityChanged).getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(env.getConfig().getDurability()).isEqualTo(Durability.COMMIT_NO_SYNC); + } + /** * A change which moves db-directory as well is still applied and reported: the note of the moved * directory, which asks for a restart of its own, does not end the change before the rest of it. - * The storage keeps naming the directory the environment runs on, which a backup lists, and the - * move is held against that directory: a later change still asks for the restart, and one which - * moves back asks for nothing. + * The storage keeps naming the directory the environment runs on, which a backup lists and the + * disk monitor watches, and the move is held against that directory: a later change still asks for + * the restart, and one which moves back asks for nothing. */ @Test public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws Exception { + final DiskSpaceMonitor monitor = serverContext.getDiskSpaceMonitor(); final Environment env = environmentOf(storage); final File directoryAtOpen = storage.getDirectory(); final String fileMaxAtOpen = env.getConfig().getConfigParam(LOG_FILE_MAX); @@ -735,6 +811,110 @@ public void aChangeWhichMovesTheDirectoryIsStillAppliedToTheEnvironment() throws assertThat(again.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); + + final ArgumentCaptor registered = ArgumentCaptor.forClass(File.class); + verify(monitor, atLeastOnce()).registerMonitoredDirectory( + anyString(), registered.capture(), anyLong(), anyLong(), any(DiskSpaceMonitorHandler.class)); + assertThat(registered.getAllValues()).containsOnly(directoryAtOpen); + } + finally + { + recursiveDelete(getFileForPath(MOVED_DB_DIRECTORY)); + } + } + + /** + * A storage closed while a move of its directory waits for the restart deregisters from the disk + * monitor the directory it ran on, the one it registered. + */ + @Test + public void aStorageClosedWithAMovePendingDeregistersTheDirectoryItRanOn() throws Exception + { + final DiskSpaceMonitor monitor = serverContext.getDiskSpaceMonitor(); + final File directoryAtOpen = storage.getDirectory(); + final JEBackendCfg cfg = createBackendCfg(); + when(cfg.getDBDirectory()).thenReturn(MOVED_DB_DIRECTORY); + try + { + assertThat(storage.applyConfigurationChange(cfg).getResultCode()).isEqualTo(ResultCode.SUCCESS); + + storage.close(); + + verify(monitor).deregisterMonitoredDirectory(directoryAtOpen, storage); + } + finally + { + recursiveDelete(getFileForPath(MOVED_DB_DIRECTORY)); + } + } + + /** + * A mode changed along with a move is written to the directory moved to alone, the one the + * environment runs on keeps its own: a later change which moves back with the same mode still + * writes it to the running directory. + */ + @Test + public void aModeChangedAlongWithAMoveReachesTheRunningDirectoryWhenMovedBack() throws Exception + { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) + { + throw new SkipException("the directory mode is POSIX alone"); + } + final File directoryAtOpen = storage.getDirectory(); + final JEBackendCfg movedOut = createBackendCfg(); + when(movedOut.getDBDirectory()).thenReturn(MOVED_DB_DIRECTORY); + when(movedOut.getDBDirectoryPermissions()).thenReturn("700"); + final JEBackendCfg movedBack = createBackendCfg(); + when(movedBack.getDBDirectoryPermissions()).thenReturn("700"); + try + { + assertThat(storage.applyConfigurationChange(movedOut).getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwxr-xr-x")); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(movedBack); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).isEmpty(); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwx------")); + } + finally + { + recursiveDelete(getFileForPath(MOVED_DB_DIRECTORY)); + } + } + + /** + * The open writes the configured mode to the directory it runs on, a mode which came with a move + * made while the storage was closed as well: a later change back to the former mode writes that + * one to the running directory again. + */ + @Test + public void aModeTheOpenWroteIsWhatALaterChangeIsHeldAgainst() throws Exception + { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) + { + throw new SkipException("the directory mode is POSIX alone"); + } + final File directoryAtOpen = storage.getDirectory(); + final JEBackendCfg movedOut = createBackendCfg(); + when(movedOut.getDBDirectory()).thenReturn(MOVED_DB_DIRECTORY); + when(movedOut.getDBDirectoryPermissions()).thenReturn("700"); + storage.close(); + try + { + assertThat(storage.applyConfigurationChange(movedOut).getResultCode()).isEqualTo(ResultCode.SUCCESS); + storage.open(AccessMode.READ_WRITE); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwx------")); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg()); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).isEmpty(); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwxr-xr-x")); } finally { 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 6a316c92e3..06d3d1a506 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 @@ -26,6 +26,9 @@ import java.io.File; import java.lang.reflect.Field; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -35,8 +38,10 @@ import org.forgerock.opendj.config.server.ConfigException; import org.forgerock.opendj.ldap.ByteString; import org.forgerock.opendj.ldap.ResultCode; +import org.mockito.ArgumentCaptor; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; +import org.opends.server.api.DiskSpaceMonitorHandler; import org.forgerock.opendj.server.config.server.PDBBackendCfg; import org.opends.server.backends.pluggable.spi.AccessMode; import org.opends.server.backends.pluggable.spi.ReadOperation; @@ -50,6 +55,7 @@ import org.opends.server.core.MemoryQuota; import org.opends.server.core.ServerContext; import org.opends.server.extensions.DiskSpaceMonitor; +import org.testng.SkipException; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; @@ -904,13 +910,14 @@ public void aCheckpointIntervalChangedWhileOpenAsksForARestart() throws Exceptio /** * A change which moves db-directory as well is still reported whole: the note of the moved * directory, which asks for a restart of its own, does not end the change before the rest of it. - * The storage keeps naming the directory it runs on, which a backup lists, and the move is held - * against that directory: a later change still asks for the restart, and one which moves back asks - * for nothing. + * The storage keeps naming the directory it runs on, which a backup lists and the disk monitor + * watches, and the move is held against that directory: a later change still asks for the restart, + * and one which moves back asks for nothing. */ @Test public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception { + final DiskSpaceMonitor monitor = serverContext.getDiskSpaceMonitor(); final File directoryAtOpen = storage.getDirectory(); final long intervalAtOpen = createBackendCfg().getDBCheckpointerWakeupInterval(); final PDBBackendCfg cfg = createBackendCfg(); @@ -932,6 +939,110 @@ public void aChangeWhichMovesTheDirectoryStillReportsTheRest() throws Exception assertThat(again.getMessages().get(0).ordinal()).isEqualTo(NOTE_CONFIG_DB_DIR_REQUIRES_RESTART.ordinal()); assertThat(storage.applyConfigurationChange(createBackendCfg()).getMessages()).isEmpty(); + + final ArgumentCaptor registered = ArgumentCaptor.forClass(File.class); + verify(monitor, atLeastOnce()).registerMonitoredDirectory( + anyString(), registered.capture(), anyLong(), anyLong(), any(DiskSpaceMonitorHandler.class)); + assertThat(registered.getAllValues()).containsOnly(directoryAtOpen); + } + finally + { + recursiveDelete(getFileForPath("PDBStorageTest-moved")); + } + } + + /** + * A storage closed while a move of its directory waits for the restart deregisters from the disk + * monitor the directory it ran on, the one it registered. + */ + @Test + public void aStorageClosedWithAMovePendingDeregistersTheDirectoryItRanOn() throws Exception + { + final DiskSpaceMonitor monitor = serverContext.getDiskSpaceMonitor(); + final File directoryAtOpen = storage.getDirectory(); + final PDBBackendCfg cfg = createBackendCfg(); + when(cfg.getDBDirectory()).thenReturn("PDBStorageTest-moved"); + try + { + assertThat(storage.applyConfigurationChange(cfg).getResultCode()).isEqualTo(ResultCode.SUCCESS); + + storage.close(); + + verify(monitor).deregisterMonitoredDirectory(directoryAtOpen, storage); + } + finally + { + recursiveDelete(getFileForPath("PDBStorageTest-moved")); + } + } + + /** + * A mode changed along with a move is written to the directory moved to alone, the one the + * database runs on keeps its own: a later change which moves back with the same mode still writes + * it to the running directory. + */ + @Test + public void aModeChangedAlongWithAMoveReachesTheRunningDirectoryWhenMovedBack() throws Exception + { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) + { + throw new SkipException("the directory mode is POSIX alone"); + } + final File directoryAtOpen = storage.getDirectory(); + final PDBBackendCfg movedOut = createBackendCfg(); + when(movedOut.getDBDirectory()).thenReturn("PDBStorageTest-moved"); + when(movedOut.getDBDirectoryPermissions()).thenReturn("700"); + final PDBBackendCfg movedBack = createBackendCfg(); + when(movedBack.getDBDirectoryPermissions()).thenReturn("700"); + try + { + assertThat(storage.applyConfigurationChange(movedOut).getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwxr-xr-x")); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(movedBack); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).isEmpty(); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwx------")); + } + finally + { + recursiveDelete(getFileForPath("PDBStorageTest-moved")); + } + } + + /** + * The open writes the configured mode to the directory it runs on, a mode which came with a move + * made while the storage was closed as well: a later change back to the former mode writes that + * one to the running directory again. + */ + @Test + public void aModeTheOpenWroteIsWhatALaterChangeIsHeldAgainst() throws Exception + { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) + { + throw new SkipException("the directory mode is POSIX alone"); + } + final File directoryAtOpen = storage.getDirectory(); + final PDBBackendCfg movedOut = createBackendCfg(); + when(movedOut.getDBDirectory()).thenReturn("PDBStorageTest-moved"); + when(movedOut.getDBDirectoryPermissions()).thenReturn("700"); + storage.close(); + try + { + assertThat(storage.applyConfigurationChange(movedOut).getResultCode()).isEqualTo(ResultCode.SUCCESS); + storage.open(AccessMode.READ_WRITE); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwx------")); + + final ConfigChangeResult ccr = storage.applyConfigurationChange(createBackendCfg()); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ccr.getMessages()).isEmpty(); + assertThat(Files.getPosixFilePermissions(directoryAtOpen.toPath())) + .isEqualTo(PosixFilePermissions.fromString("rwxr-xr-x")); } finally {