[#1068] Apply what a running backend takes of a configuration change, and ask for a restart for what it does not - #1069
Open
vharseko wants to merge 5 commits into
Conversation
…n listeners only once it has opened EntryContainer registered itself and its two configuration managers from its constructor, and only close() takes them off again. open() caught StorageRuntimeException alone, so a ConfigException - a VLV filter or sort order which does not parse, an index type the attribute has no matching rule for - left a container nothing holds a reference to, registered on the configuration of a backend which did not start. Register the five at the end of a successful open() instead, and catch every failure there rather than the storage ones alone. Hold each index in its map before opening it: an attribute index registers its listener at the end of open() and a VLV index from its constructor, so the one being opened was not yet one close() could find - the hole the existing catch already had. RootContainer gives back what a failed open took: the entry containers it registered, its own listener and the storage, the last only when this call is what opened it. openAndRegisterEntryContainers runs inside a write the storage may replay, so it now gives up what a rolled back attempt registered before opening again; without it a write-write conflict failed the backend with ERR_ENTRY_CONTAINER_ALREADY_REGISTERED. Fixes OpenIdentityPlatform#993
…ok, and pin the give-back of the root container Review round 2 of OpenIdentityPlatform#999. PDBStorage.open() and startImport() give back what the attempt took before it failed - the cache size buildConfiguration drew from the memory quota, the listener the constructor registered on the backend configuration, and the database when the open got that far - as JDBCStorage.open already does. Every failed enable of a PDB backend drained one cache size for the life of the JVM and left a storage answering the configuration changes of a backend which is not running. The guard against a double open runs before anything is taken, and close() releases the quota once and tolerates a database the failed open registered no monitor for. RootContainer registers an entry container as soon as it has opened, before its highest entry ID is read: a container which opened has registered every listener it ever will, and only what the registry holds is given back. The rationale of the storageOpened arm is the true one: the storage's own open() threw, and what that open took is the storage's own to give back - no root container is ever opened over a storage another one holds. FailedBackendOpenTest pins the give-back loop with a second base DN, the registration of an opened container, the root container's own listener when the storage did not open, and the positive twin - one registration of each listener once a container has opened, counted per occurrence. PDBStorageTest pins the give-back, a close() which follows it, and the refusal of a double open.
…k, and pin the give-back past the database open JEStorage had the shape PDBStorage had before the previous round: the quota taken in buildConfiguration ahead of open0, the double-open guard behind it, a failed `new Environment` leaving the quota and the constructor's listener behind with nobody to close the storage, and a close() which released the quota on every call - three times on the import road, for two acquisitions. It gets the same moves: rejectIfOpen() ahead of buildConfiguration in open() and startImport(), openOrGiveBack() around open0(), the quota released once, an environment without a monitor tolerated. Both storages now give back the quota, the listener and the monitored directory ahead of the database, so that a database whose own close throws keeps nothing else. JEStorageTest, new: the three cases of PDBStorageTest over a directory the server cannot use - a locked directory is not a JE road inside a JVM, DbEnvPool shares the environment. PDBStorageTest gains the case past the database open, with the disk monitor refusing the directory: the volume, the monitor and the quota are given back. FailedBackendOpenTest: a case whose expected failure does not come closes the backend it opened after all, so the case which follows fails on its own assertion rather than on the base DN the previous one left registered.
…han what the configuration says by then, and ask for a restart when the cache size changes PDBStorage and JEStorage reserved their cache size from the memory quota by reading config in buildConfiguration and released it by reading config again in close(). applyConfigurationChange swapped config in between without touching the quota or the cache, and neither db-cache-size nor db-cache-percent was marked as needing a restart, so a cache grown from 64 MB to 128 MB while the backend ran released 128 against 64 taken at the next disable - the one an online import makes included - and the quota believed 64 MB free that the server did not have, for the life of the JVM; a shrink left the difference reserved by nobody. The running cache was the old size throughout. Both storages now keep two numbers of their own: the cache size of the configuration they opened with, and of it what the quota granted - a tryAcquire it refused, which an open at startup is not checked against, reserved nothing and used to be released all the same. close() gives back the granted size. isConfigurationChangeAcceptable admits the difference to what is held rather than to config, which a change admitted but not yet applied has already moved to the new size. applyConfigurationChange on an open storage whose cache size the change moves sets adminActionRequired and says so (NOTE_CONFIG_DB_CACHE_REQUIRES_RESTART): PersistIt cannot resize a buffer pool once the database is open, and JEStorage has never resized its environment. The two properties are marked component-restart in both configuration XMLs, as db-directory is. PDBStorageTest and JEStorageTest, six cases each: the grow and the shrink give back what was taken, the change asks for a restart and names both sizes, a change which leaves the cache alone asks for nothing, admission is against what is held, and a reservation the quota refused is not given back.
…onfiguration 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 OpenIdentityPlatform#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.
vharseko
added
bug
tests
This was referenced Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1068. Stacked on #1066 (
5d5b3e1f09, itself on #999): the same lines of bothapplyConfigurationChange, both XMLs and the ordinal after its 630;JEStorageTestexists only from #999 on.What was wrong
Nine properties of the JE and PDB backends were neither applied to a running backend nor marked as requiring a restart, so
dsconfigreported a change of them applied and the backend ran on unchanged until it was next opened.JEStorage.applyConfigurationChangehandleddb-directory, its permissions and the disk thresholds and left the environment - configured once, at the open - as it was; nothing in the server calledEnvironment.setMutableConfig. The same went for a native property changed throughje-property, mutable or not (not in the issue's table, same cause).Two things the issue had wrong, found on the way:
requires-admin-actionin the XML reaches the reference documentation and property help alone -dsconfigprints nothing about it at set time, and a change result withoutadminActionRequiredcarries nothing to the error log either. So "asdb-log-file-maxalready does" was not true of the change result: the marking is all those properties had. The oldRootContainerreported every changed immutable parameter in the change result, which the server logs as a warning (WARN 647); that shape is restored for all of them.db-txn-write-no-syncis on by default, sodsconfig set-backend-prop --set db-txn-no-sync:trueon a JE backend yields a durability which sets both flags. Nothing checked that at change time: the change was admitted and written, and the backend failed to open onERR_CONFIG_JEB_DURABILITY_CONFLICTat its next restart. (aChangeWhichLeavesTheCacheSizeAloneAsksForNothingof [#1063] Give back what the open reserved rather than what the configuration says by then, and ask for a restart when the cache size changes #1066 used exactly that as its "unrelated change"; it now sets the write flag off as well.)What this does
JE -
applyToEnvironmentbuilds the environment configuration the changed configuration describes (ConfigurableEnvironment.toEnvironmentConfig) and hands it toEnvironment.setMutableConfig, which takes of it what JE accepts while it runs:db-cleaner-min-utilization,db-run-cleaner,db-evictor-core-threads,db-evictor-max-threads,db-evictor-keep-alive,db-num-cleaner-threads, the durability (db-txn-no-sync/db-txn-write-no-sync, every way - a configuration which sets neither now setsCOMMIT_SYNCexplicitly, since JE leaves the durability an environment has in place when handed none) and a mutableje-property. All but the cache:je.maxMemory/je.maxMemoryPercentare mutable too, but the cache stays with the memory reserved for it until the restart #1063 asks for, so the change hands the environment its current values back. Every immutable JE parameter whose value differs from the running environment's is reported with the newNOTE_CONFIG_DB_PROPERTY_REQUIRES_RESTART(631), naming the property asdsconfigknows it (or the JE property name forje-property), the value the environment runs with and the one configured. 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; held to the import's configuration, every property the import sets differently would ask for a restart.toEnvironmentConfigis the build alone - no cache size probe against the memory quota (#1067), no level set on the JE loggers (that moves toparseConfigEntry, the open's road) - so that a change can be checked against it:isConfigurationChangeAcceptableandisConfigurationAcceptablenow refuse a conflicting durability and a native property JE does not know before the change is written.PDB -
db-checkpointer-wakeup-intervalis set on the PersistIt configuration at the open alone andPersistit.setConfigurationrefuses once one is set, so a change of it reports 631 against the interval the database opened with (db.getConfiguration().getCheckpointInterval(), no new field), and the property is markedcomponent-restartinPDBBackendConfiguration.xml.je-property's definition says which of its changes wait for a restart.Left as they are:
db-logging-levelanddb-logging-file-handler-onkeep theircomponent-restartmarking. The JUL level is set by the open alone, as before; the file handler's level (je.env.fileLoggingLevel) is mutable in JE and so follows a change from now on - the marking is conservative about it, not wrong.Tests
JEStorageTest: the six mapped properties reachenv.getMutableConfig(); the durability follows the change every way; a mutableje-propertyis applied and an immutable one asks for a restart with 631;db-log-file-maxasks for a restart with 631; a change while open leaves the cache where the open reserved it (the #1063 interplay); a change during an import leaves the import's environment alone; a change while closed touches nothing; a durability which sets both flags and an unknown native property are refused by both acceptability checks.PDBStorageTest: a changed interval asks for a restart with 631 and the database keeps its own; a change while closed asks for nothing.Verified locally: the issue reproduced on master, on the #999 head and on the #1066 head with a direct TestNG repro of every row (JE 7/7 red, PDB 1/1 red,
db-txn-no-syncon PDB green as a control); the new tests are red without the fix (5 JE + 1 PDB) and green with it; mutants (nosetMutableConfig, cache not pinned, immutables unreported, import's environment held to the configuration, PDB silent) each red on their own case; regression set of 18 classes / 240 tests (FailedBackendOpenTest, PDB/JE TestCase + Encrypted, ReplayedConfigChangeTest, OnDiskMergeImporterTest, the pluggable tree tests, ImportLDIF/RebuildIndex/VerifyIndex, BackendConfigManagerTestCase) green.