[#907] Change the base DNs of a pluggable backend outside the write the storage replays

[maximthomas]

Fixes #907.

The defect

Storage.write states its contract plainly:

In case of a write operation rollback, implementations must ensure the write operation is retried until it succeeds.

So a WriteOperation must be idempotent. BackendImpl.applyConfigurationChange was not: it did the
registry work — which no rollback reaches — inside the operation, ahead of the only part a rollback
undoes.

for (DN baseDN : cfg.getBaseDN())          // cfg is only replaced at the END of run()
{
  if (!newBaseDNs.contains(baseDN))
  {
    serverContext.getBackendConfigManager().deregisterBaseDN(baseDN);   // global, not transactional
    EntryContainer ec = rootContainer.unregisterEntryContainer(baseDN); // global, not transactional
    ec.close();                                                        // global, not transactional
    ec.delete(txn);                                                    // the only rollback-able part
  }
}

A replay therefore half-applies the change, in two different ways:

Removal. The replay re-reads the old cfg.getBaseDN() and calls deregisterBaseDN on a DN the
first attempt already removed. Registry.deregisterBaseDN throws
UNWILLING_TO_PERFORM / ERR_DEREGISTER_BASEDN_NOT_REGISTERED, which is not a conflict, so the retry
loop rethrows it. The operator sees "unwilling to perform" against the DN they just deleted instead of
the conflict that actually happened, and the backend is left half-deregistered while the rollback has
restored the trees.

Creation — worse, because it is silent. createNewBaseDNs guarded on
rootContainer.getBaseDNs().contains(baseDN). Because registerEntryContainer had already run, the
replay skipped the DN, so the trees the rollback discarded were never recreated. The result is a
registered base DN backed by nothing, reported as success. Each discarded attempt also leaked the five
configuration listeners the EntryContainer constructor registers, which only close() takes back.

Exposure per engine, read from the code — narrower than the issue text suggests:

engine	replays write()?	reachable?
PDB	yes — PDBStorage.java:648 retries on any RollbackException, commit()'s included	yes, on any conflict
JDBC	yes, unless partlyCommitted	only for a conflict on the first drop table, and only on postgresql/sql server
JE	no — JEStorage.java:885 has no retry loop	not by replay; still exposed to the half-applied state on any failure

The fix

The operation now performs only work a rollback undoes — ec.delete(txn) and openEntryContainer —
and the base DNs to remove and to add are worked out once, ahead of the write, so no attempt can see
different work to do than the attempt it replaces. Everything global happens after the commit:
deregisterBaseDN / unregisterEntryContainer / close, then registerEntryContainer /
registerBaseDN, then baseDNs and cfg.

Three consequences worth calling out:

• The operation starts by closeSilently-ing whatever a previous attempt opened, so the replay gives
  up its trees and its five listeners before opening new ones.
• The trees of a removed base DN are now deleted while it is still registered, so its entry containers
  are held exclusively across the write, as RootContainer.close() already does. This does not close
  the pre-existing race for a thread that took the container before the lock; that is unchanged.
• On failure the message names the base DNs the change was about — the bare stack trace never did.
  On PDB, JE and JDBC/postgresql+sqlserver nothing at all has been applied; where the DDL commits on
  its own (mysql, oracle) or there is no transaction (cassandra), a removed base DN's trees may be gone
  already and only a restart puts that right. The comment at the catch says so.

Tests

ReplayedConfigChangeTest (new, 4 cases) injects a Storage decorator under TracedStorage. The
conflict is raised as the RollbackException PersistIt itself raises, from inside a single
delegate.write(...), so the replay is driven by PDBStorage's own retry loop — which keeps one
storage implementation, and with it its cache of PersistIt exchanges, across every attempt. A decorator
that simply called write() twice would hand the replay a fresh cache and hide that reuse.

case	conflict	pins
removalIsReplayableWhenTheTransactionConflictsBeforeAnyStorageAccess	before any storage access	the reported UNWILLING_TO_PERFORM
additionIsReplayableWhenTheTransactionConflictsAtCommitTime	at commit	the created trees, and the persisted TRUSTED flag, survive
aRemovalAndAnAdditionInOneChangeSurviveRepeatedReplay	at commit, twice	ec.delete(txn) replayed against real storage; the contract is "until it succeeds", not "survives one replay"
aFailureWhichIsNotReplayedAppliesNothingAndNamesTheBaseDNs	unreplayable failure	nothing applied, and the message names the DNs

Each was confirmed to fail on unfixed code, for its own symptom: the UNWILLING_TO_PERFORM, the
missing dc=b907c index tree, dc=b907b's six trees still present after a "successful" removal, and
the unnamed DNs.

The addition case also asserts
verify(cfg, times(1)).removePluggableChangeListener(any()), which is what pins the listener-leak
guard.

Verification

mvn -P precommit -pl opendj-server-legacy verify \
  -Dit.test='PDBTestCase,JETestCase,BackendConfigManagerTestCase,ReplayedConfigChangeTest'

PDBTestCase 35, JETestCase 35, BackendConfigManagerTestCase 11, ReplayedConfigChangeTest 4
Tests run: 85, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS

No SPI, RootContainer, EntryContainer or storage-engine changes; no new message keys, so no churn
across the eight locale .properties files.

Out of scope

RootContainer.open() has the identical defect one level up — openAndRegisterEntryContainers
registers inside write(), and JDBCStorage.replayReason's javadoc already cites it by name. Left for
a separate change; with this one landed it is the last instance.

[vharseko]

Thanks for taking this on — confining the write to work a rollback undoes is the right shape for #907, and the javadoc explaining why the registries are updated afterwards is welcome.

A few things in the new ordering look like they need another pass. The first three I'd call blocking:

1. registerNewBaseDNs leaks the entry container it just opened when registration fails — an open container plus the configuration listeners its constructor registered, which nothing can reclaim afterwards.
2. baseDNs/cfg are now assigned unconditionally, so a failed registration is still recorded as applied and getBaseDNs() advertises a DN the server will never route here.
3. A removed base DN stays registered and reachable after its trees are gone on engines where deleteTree is not rolled back (cassandra; DDL auto-commit on mysql/oracle) — the old ordering turned that into a clean "no such entry".

Also worth addressing: ec.delete(txn) now runs against its own documented precondition; deregisterDeletedBaseDNs closes the container even when deregistration failed; the exclusive ec.lock() is held across PDB's unbounded retry loop; the new operator-facing message is hardcoded English and the failure path never sets setAdminActionRequired.

Details inline, smaller notes marked nit.

[vharseko]

If registerEntryContainer throws — ERR_ENTRY_CONTAINER_ALREADY_REGISTERED when the DN is already mapped, for instance — the container that the write opened is neither registered nor closed here. It stays alive holding the configuration listeners its constructor registered (addPluggableChangeListener, the backend index and VLV index add/delete listeners) for the life of the server, and neither closeBackend() nor RootContainer.close() can reclaim it since both iterate the registered containers. That is the same leak on the replay path this PR sets out to fix.

closeSilently(entry.getValue()) in this catch would cover it — but only when the registration did not take: if registerEntryContainer succeeded and only registerBaseDN failed, the container is registered and must not be closed here.

[maximthomas]

Fixed. registerNewBaseDNs now records whether registerEntryContainer took, and closes the container only when it did not — one the root container is holding is left alone, so closeBackend() can still reclaim it, exactly as you split it. The failure also sets setAdminActionRequired(true) now.

[vharseko]

These two assignments now run even when deregisterDeletedBaseDNs/registerNewBaseDNs have recorded a failure into ccr. Before this change if (!createNewBaseDNs(...)) return; skipped them.

So after a registerBaseDN that failed because another backend owns the DN, getBaseDNs() advertises a base DN that BackendConfigManager will never dispatch to this backend, and isIndexed(), the monitors and everything else iterating getBaseDNs() see it. Worth keeping baseDNs derived from what actually registered — e.g. rebuild it from rootContainer.getBaseDNs() after the two loops.

[maximthomas]

Fixed, with your suggestion: baseDNs is rebuilt from rootContainer.getBaseDNs() rather than from newCfg.

I moved that rebuild into the finally rather than leaving it on the success path, so that the storage-failure path — which returns early, and which may now deregister a base DN whose trees the engine did not roll back — leaves the field agreeing with the registry too. cfg = newCfg stays on the paths where the write committed.

[vharseko]

Two things here.

EntryContainer.delete documents "The entry container should be closed before calling this method", and the close() that used to precede it has moved past the commit into deregisterDeletedBaseDNs. It works today only because close() leaves attrIndexMap/vlvIndexMap populated, so listTrees() still enumerates everything — an undocumented coincidence. Any later change to close() that clears those maps turns this into a silent partial tree deletion. Either update that javadoc to state the new contract, or keep the close before the delete.

The bigger one: the trees are dropped here while the base DN is still registered in both BackendConfigManager and rootContainer. On the engines your comment below names — cassandra, and mysql/oracle where the DDL auto-commits — a later failure in this same run() (say openEntryContainer for an added DN) leaves that DN registered, still routed here, with its trees already gone: every search/add/modify against it then fails with StorageRuntimeException indefinitely, where the old deregister → unregister → close → delete order gave a clean "no such entry".

[maximthomas]

Two answers, since they are two different things.

The javadoc. Updated it rather than moving the close back, because the wording was already not what the code does: OnDiskMergeImporter.beforePhaseOne calls entryContainer.delete(...) on an open container and then goes on using it. It now says the container may be open or closed, why the tree set is the same either way, and what a close() which cleared attrIndexMap/vlvIndexMap would break — so the coupling is written down where someone changing close() will read it.

One correction on the mechanism, though: the hazard you describe runs the other way round. The new order is delete then close, so a close() which cleared those maps would not affect this call at all — it is the old order (close, then delete) that depended on the maps surviving close(). The javadoc now warns about a call made after close(), which is the one that would silently delete part of the container.

The bigger one. Agreed, and fixed in two places.

• Inside the write, the added base DNs are now opened before the removed ones are deleted. The failure you name — openEntryContainer for an added DN — is therefore reached while everything is still there to roll back to, on every engine.
• On the failure path, deregisterBaseDNsWhoseTreesAreGone asks Storage.listTrees() which of the removed containers actually lost their trees, and gives up exactly those: deregisterBaseDN + unregisterEntryContainer + close. So on persistit, je and jdbc/postgresql/sqlserver the rollback put the trees back, nothing is given up, and the backend is left as it was — which is what aFailureWhichIsNotReplayedAppliesNothingAndNamesTheBaseDNs already pins. On cassandra and jdbc/mysql/oracle the trees are gone, the base DN stops being routed here, and the operator gets the plain "no such entry" instead of an endless StorageRuntimeException. setAdminActionRequired(true) is set in exactly that case, and if listTrees() itself cannot be run, nothing is given up on the strength of a probe that failed — the flag alone is raised.

New test: aFailureWhichIsNotRolledBackGivesUpTheBaseDNsWhoseTreesAreGone, which commits the operation and then reports the failure, which is what a non-transactional deletion leaves behind.

[vharseko]

BackendConfigManager.deregisterBaseDN builds a copy of the registry and only assigns registry = newRegister once the deregistration succeeded. If it throws for any reason other than "not registered" — a RuntimeException out of removeNamingContext/switchNamingContextIsSubSuffix/retrieveParentSuffix — the live registry still maps the DN to this backend, yet this finally removes and closes the container anyway.

Requests for that DN then get ERR_BACKEND_ENTRY_DOESNT_EXIST from a backend that owns nothing; closeBackend() cannot release it either, since it deregisters only the DNs in rootContainer.getBaseDNs() — from which it has just been removed. The DN stays claimed, and no other backend can register it, until a restart.

[maximthomas]

Fixed. The finally is gone. unregisterEntryContainer + close now run only when deregisterBaseDN returned, or when the registry shows the base DN is not mapped here anyway — getLocalBackendWithBaseDN(baseDN) != this, which is what an earlier change whose registerBaseDN failed leaves behind, and where holding on to the container would strand it for good.

A real failure keeps the container registered so closeBackend() can still reclaim the DN through rootContainer.getBaseDNs(), reports ERR_BACKEND_CANNOT_DEREGISTER_BASEDN naming the DN, and sets setAdminActionRequired(true).

[vharseko]

lock() sets exclusiveAccessPending, which parks every subsequent beginSharedAccess() caller on sharedAccessMonitor.wait() with no timeout, and it is now held for the whole of Storage.write — retries included, as the comment says. PDBStorage.WriteableStorageImpl.write is for (;;) { txn.begin(); ... catch (RollbackException e) { Thread.sleep(random) } } with no attempt cap.

Under sustained write contention on the volume that means the admin thread spins in the retry loop while every worker thread touching the removed base DN parks indefinitely; before this change no entry-container lock was taken at all. lock() also sleep-polls the drain uninterruptibly, so a long-running search on that suffix stalls the config-change thread first. Bounding the retries (or the wait) would keep a configuration change from becoming an indefinite stall.

[maximthomas]

The facts are right, but I do not think this PR is where the bound belongs, and I would rather not add one here.

Holding entryContainer.lock() across a storage.write(...) — the same unbounded for (;;) { txn.begin(); ... catch (RollbackException) { sleep } } — is the existing convention in this package, not something this ordering introduces:

• EntryContainer.AttributeJEIndexCfgManager.applyConfigurationDelete (EntryContainer.java:248) takes EntryContainer.this.lock() and holds it across storage.write(index.closeAndDelete(txn)).
• AttributeIndex.applyConfigurationChange (AttributeIndex.java:945) does the same across the write that deletes the removed indexes, with the comment "We get exclusive lock to ensure that no query is actually using the indexes that will be deleted" — which is exactly the reason here.
• RootContainer.close() (RootContainer.java:330) holds it across ec.close().

Bounding it means bounding Storage.write's retry contract, which is storage-wide, reaches those three call sites as well as this one, and would change what the replay tests in this PR pin (aRemovalAndAnAdditionInOneChangeSurviveRepeatedReplay relies on "replayed until it succeeds"). Happy to open a follow-up issue for a bounded write or a lock(timeout), but it seems wrong to land it inside a fix for #907.

What did change here: the whole prologue — locks included — is skipped when no base DN moves, so the common configuration change never reaches lock() at all; and the comment now names the two existing call sites rather than only RootContainer.close().

[maximthomas]

Opened #921 for this.

It covers the bound itself and the two things that have to be settled with it: sizing the window against the conflicts that actually drive the loop (page-level conflicts from other base DNs in the same backend, since the containers under the exclusive lock are the ones no user write can reach), and the fact that aRemovalAndAnAdditionInOneChangeSurviveRepeatedReplay currently pins "replayed until it succeeds" and would need its comment rewritten alongside a cap.

Worth noting while looking at it: PDB is the only engine here with no bound at all. JDBCStorage.write is bounded twice, by MAX_RETRIES = 10 and by a 10 s MAX_RETRY_WINDOW_NANOS, with exponential backoff; JEStorage.write does not replay; cassandra has no transaction. So there is a working model in-tree to copy rather than a design to invent.

[vharseko]

nit: this message does not name the base DN it failed on, while the sibling registerNewBaseDNs uses ERR_BACKEND_CANNOT_REGISTER_BASEDN.get(baseDN, e) and the write's catch deliberately names both lists. Removing three base DNs and having the second one fail leaves the operator a bare stack trace with no way to tell which DN is now inconsistent.

[maximthomas]

Fixed — ERR_BACKEND_CANNOT_DEREGISTER_BASEDN_620 in backend.properties, naming the base DN alongside the stack trace, so a three-DN removal whose second DN fails says which one.

[vharseko]

nit: when deleted and added are both empty — every change to index-entry-limit, db-cache-percent, preload-time-limit and so on, i.e. the common case — this still opens a write transaction and commits an empty one. For PDB that is newStorageImpl(), txn.begin(), an empty body, txn.commit(commitPolicy) and txn.end(): a durable commit for a no-op. An early if (deleted.isEmpty() && added.isEmpty()) that goes straight to cfg = newCfg would skip it.

[maximthomas]

Fixed — if (deleted.isEmpty() && added.isEmpty()) sets baseDNs/cfg and returns before the locks and before the write, so index-entry-limit and friends no longer open a transaction. New test aChangeWhichLeavesTheBaseDNsAloneOpensNoTransaction pins that the storage is not asked for a write at all.

[vharseko]

nit: the map keys each container by a DN the value already carries — EntryContainer.getBaseDN(), which is exactly what baseDNsOf below uses. A List<EntryContainer> would let registerNewBaseDNs iterate containers directly, drop the java.util.Map/java.util.LinkedHashMap imports and the Map.Entry loop, and leave deleted, created and locked three lists of the same shape.

[maximthomas]

Fixed — created is a List<EntryContainer>, registerNewBaseDNs takes the DN from ec.getBaseDN(), and the java.util.Map/java.util.LinkedHashMap imports are gone. deleted, created and locked are three lists of the same shape now.

[vharseko]

nit: rootContainer is null-checked once here and then re-read from the mutable field at each use, and that window now stretches past the commit into the registration loops. importLDIF, rebuildBackend, exportLDIF and verifyBackend all assign the field and null it out again in their finally blocks, so a concurrent one can make this NPE after the trees are already committed — which the pre-PR version could not do. final RootContainer rc = rootContainer; once, used throughout, closes that.

[maximthomas]

Fixed — final RootContainer rc = rootContainer; read once at the top, used throughout and passed to the helpers, so nothing re-reads the field after the commit.

[vharseko]

nit: new files in this repo carry a standalone Copyright 2026 3A Systems, LLC. line rather than a Portions Copyright one — cf. PersistentCompressedSchemaTest.java in this same package (655132f). Portions is for existing upstream files being modified.

[maximthomas]

Fixed — standalone Copyright 2026 3A Systems, LLC., with the bracketed line back to Portions copyright, matching PersistentCompressedSchemaTest.java.

[vharseko]

Thanks — the previous round is properly answered. Every point from the last review is either fixed or answered with a reason I accept, and the two I got wrong you corrected correctly: the delete-then-close order does invert the attrIndexMap hazard I described, and the unbounded retry is the package's existing convention (EntryContainer.java:248, AttributeIndex.java:945, RootContainer.java:330), not something this ordering introduces — bounding it belongs in #921, not here, and the early return for the no-base-DN-change case keeps the common change away from lock() entirely. Consider that thread closed on my side.

The core of this remains right, and it is the right diagnosis of #907: the registry work has no business inside a WriteOperation the storage replays, the add/remove sets have to be worked out once ahead of the write, and the replay has to give back what the previous attempt opened (closeSilently(created) + re-open) so the five configuration listeners each EntryContainer registers do not pile up. Working from RootContainer.getBaseDNs() rather than from cfg is the right source of truth. The two new message keys are unique with the right arity, and the six-case test covers the replay properly.

Four things on the new shape.

Blocking

1. Lock-order inversion: EntryContainer.exclusiveLock is now held across BackendConfigManager.writeLock

This is separate from the unbounded-retry thread, and it is new here rather than a package convention: BackendImpl.java:920-924 holds the deleted containers' exclusive lock through deregisterDeletedBaseDNs() (line 970) and registerNewBaseDNs() (line 971), both of which take BackendConfigManager's single global writeLock (BackendConfigManager.java:603, 673).

The opposite direction already exists in the server, and not only on the config path:

• BackendConfigManager.shutdownLocalBackends() (:1181) holds writeLock and calls backend.finalizeBackend() -> closeBackend() -> rootContainer.close() -> EntryContainer.lock() (RootContainer.java:330).
• applyConfigurationChange(BackendCfg) (:809, ds-cfg-enabled: false) and applyConfigurationDelete (:1112) do the same at lines 829 and 1134.

So a base-DN change in flight — holding ec.exclusiveLock, waiting on writeLock inside deregisterBaseDN — against a shutdown holding writeLock and waiting on the very same container's lock is a deadlock with no timeout on either side, and it hangs shutdown. The three call sites you cite for the convention hold the container lock across storage.write only; none of them calls into BackendConfigManager underneath it.

The registry half does not need the container lock: it runs after the commit, and the container it deregisters is closed immediately afterwards. Releasing the locks once write() returns, before the deregister/register loops, keeps the new direction out without touching anything in #921.

2. The catch-block comment's premise does not hold for the JDBC backend

Lines 959-962 say that on "persistit, je, and the jdbc backend on postgresql and sql server ... nothing at all has been applied", and your reply on the failure-path thread says the same ("on persistit, je and jdbc/postgresql/sqlserver the rollback put the trees back, nothing is given up"). That holds for PDB and JE but not for JDBC on any engine.

JDBCStorage.WriteableTransactionTransactionImpl.commitStatement() (JDBCStorage.java:2931-2942) issues the statement and then calls con.commit() unconditionally; deleteTree drops the table through it (:3323), as openTree creates it (:2986). commitsBeforeDdl() only decides which side of the statement partlyCommitted goes up on — mysql/oracle before, everything else after — not whether the DDL commits. Your own table in the description reads the same way: JDBC is replayable "only for a conflict on the first drop table" precisely because that drop has committed.

So on postgresql and sql server the dropped tables, the catalog unenrolment and the created tables are all durable when write() fails. deregisterBaseDNsWhoseTreesAreGone() does run and does save the behaviour, so this is a comment-and-description fix rather than a code one — but as written it tells the next reader that path is a mysql/oracle/cassandra concern only, which is the opposite of what the JDBC backend does.

3. Trees created for the added base DNs are never dropped after a failed write

The catch (lines 953-967) closes the created EntryContainers and inspects deleted only. Opening the added DNs first does reach the likely failure while the removals can still be rolled back — but on JDBC (any engine) and on mysql/oracle/cassandra, rc.openEntryContainer() has itself already committed create table opendj_<...>_id2entry and the rest, plus the catalog rows. The added base DN then appears in neither rootContainer.getBaseDNs() nor cfg, so no later configuration change, no closeBackend() and no restart will ever name those tables again — they leak permanently, and ERR_BACKEND_CANNOT_CHANGE_BASEDNS speaks only of base DNs "whose trees are gone", so the operator is never told they exist.

deregisterBaseDNsWhoseTreesAreGone needs its counterpart: the created containers whose trees survived the failure have to be deleted, or at the very least named in the message.

4. applyConfigurationChange can now throw instead of returning a ConfigChangeResult

The prologue, lines 877-924, sits outside any try. Before this change the entire body was in try { ... } catch (Exception e) { ccr.setResultCode(serverError); ... } (master, BackendImpl.java:852-882), so the admin framework was guaranteed a ccr whatever happened.

rc.getEntryContainer(baseDN) at line 888 returns null for a container unregistered between the getBaseDNs() copy at line 882 and that call — importLDIF/rebuildBackend reach rootContainer.close(), which unregisters every container (RootContainer.java:327-339). The ancestor walk cannot substitute a sibling, since Registry.registerBaseDN rejects hierarchically related base DNs within one backend (BackendConfigManager.java:1330). deleted then holds a null and ec.lock() at line 922 throws NPE out of the listener dispatch. Wrapping the method again, or null-checking the lookup and failing the change cleanly, restores the old guarantee.

Worth addressing

• allTreesStored treats a partially deleted container as fully gone (line 1019). On mysql/oracle/JDBC a write that drops three of six trees and then fails leads to the base DN being deregistered and unregistered, so the three surviving tables are named by nothing: the stored configuration no longer lists the DN, RootContainer.open() will not open it, and no clear/removeStorageFiles path reaches it. ERR_BACKEND_CANNOT_CHANGE_BASEDNS nevertheless tells the operator a restart will make what the backend holds match the configuration.

• listTrees() failing sets adminActionRequired with no message of its own (lines 1010-1015). Not giving up a base DN on the strength of a probe that failed is right; the operator just ends up with the write failure plus a bare "admin action required" in exactly the case where they most need to be told the backend could not work out what survived.

• Diagnostics differ between the two halves of the same failure. Line 1095 formats with ERR_BACKEND_CANNOT_REGISTER_BASEDN.get(baseDN, e) — e.toString(), no cause chain — while line 1067 uses stackTraceToSingleLineString(e). The latter on both.

• ReplayedConfigChangeTest: the backend is created and opened outside the tests' try/finally. If openBackend() throws after newRootContainer() succeeded, finalizeBackend() never runs — the PersistIt volume stays open and rootContainerMonitor stays registered, so every following test fails in openBackend() and masks the first failure. @AfterMethod only deregisters base DNs.

• Minor simplifications: locked always mirrors deleted (or a prefix of it), so it is a second list that has to be kept in step for the unlock loop to stay correct; allTreesStored(ec, storedTrees) is storedTrees.containsAll(namesOf(ec.listTrees())); deregisterDeletedBaseDNs is a one-line loop with a single caller.

Not for this PR, noted only

Keeping the container registered when deregisterBaseDN fails is what I asked for last round and is the right call — but it does leave the committed path deliberately holding a container whose trees are gone, which is the state deregisterBaseDNsWhoseTreesAreGone's javadoc (lines 991-996) argues against. Both choices are defensible; the two rationales sitting three methods apart are not. A sentence in one of the two javadocs saying why the answer differs when the registry is still routing to the DN would settle it for the next reader.

A failed registerBaseDN also leaves the base DN in rootContainer.getBaseDNs() but unrouted, so re-submitting the identical configuration takes the deleted.isEmpty() && added.isEmpty() early return and reports SUCCESS with no messages. That is not a regression — master's createNewBaseDNs guards on rootContainer.getBaseDNs().contains(baseDN) after the same register order and behaves identically — so it is out of scope here, but it is the one case where the new finally comment ("a base DN whose registration failed is not one this backend serves") does not describe what line 982 actually stores. Worth an issue of its own.