Describe the bug
Storage.write() documents its WriteOperation as idempotent, because every pluggable storage engine may
replay it after a transaction conflict — JDBCStorage.write() and PDBStorage.write() both do. The
WriteOperation in PluggableBackendImpl.applyConfigurationChange is not idempotent: it performs global,
non-transactional side effects, and it performs them before the transactional work a rollback would undo.
// opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendImpl.java:886
private void removeDeletedBaseDNs(SortedSet<DN> newBaseDNs, WriteableTransaction txn) throws DirectoryException
{
for (DN baseDN : cfg.getBaseDN())
{
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 part a rollback undoes
}
}
}
cfg is reassigned to newCfg at the very end of run(), so a replay re-reads the old base DNs and repeats
the loop over a DN that has already been deregistered.
What the replay does
deregisterBaseDN throws before anything else can:
// BackendConfigManager.java, Registry.deregisterBaseDN
Backend<?> backend = backendsByName.get(baseDN);
if (backend == null)
{
throw new DirectoryException(UNWILLING_TO_PERFORM, ERR_DEREGISTER_BASEDN_NOT_REGISTERED.get(baseDN));
}
That DirectoryException is not a transaction conflict, so the retry loop rethrows it. The operator sees
ERR_DEREGISTER_BASEDN_NOT_REGISTERED — "unwilling to perform" against a DN they just removed — instead of the
deadlock that actually happened, and the backend is left half-deregistered: the DN is out of the registry and its
EntryContainer is closed and unregistered, but its trees are still there, restored by the rollback.
Scope
Not JDBC-specific. BackendImpl is the base of every pluggable backend, and PDBStorage.write() replays on its
own conflict exception, so JE/PDB reach it the same way. The trigger is narrow — a base DN removed from a
backend's configuration while a transaction conflict fires on the delete or its commit — but the outcome is a
half-applied configuration change reported under the wrong error.
Suggested fix
Move the non-transactional work out of write(): keep only ec.delete(txn) inside, and run deregisterBaseDN,
unregisterEntryContainer and ec.close() after the write has committed. Deriving the set to remove from
rootContainer rather than from the stale cfg would make the loop replay-safe on its own, but it would still
leave the deregistration and the close unrecoverable if a later part of the change fails, so the reordering is
the durable fix.
A null guard on unregisterEntryContainer's result is not sufficient — it is unreachable, since
deregisterBaseDN throws one line earlier.
Related
Found while reviewing #904 (#904 (review)).
That PR increases how often a JDBC conflict is replayed, which raises the odds of hitting this, but the defect is
present on master independently of it.
Describe the bug
Storage.write()documents itsWriteOperationas idempotent, because every pluggable storage engine mayreplay it after a transaction conflict —
JDBCStorage.write()andPDBStorage.write()both do. TheWriteOperationinPluggableBackendImpl.applyConfigurationChangeis not idempotent: it performs global,non-transactional side effects, and it performs them before the transactional work a rollback would undo.
cfgis reassigned tonewCfgat the very end ofrun(), so a replay re-reads the old base DNs and repeatsthe loop over a DN that has already been deregistered.
What the replay does
deregisterBaseDNthrows before anything else can:That
DirectoryExceptionis not a transaction conflict, so the retry loop rethrows it. The operator seesERR_DEREGISTER_BASEDN_NOT_REGISTERED— "unwilling to perform" against a DN they just removed — instead of thedeadlock that actually happened, and the backend is left half-deregistered: the DN is out of the registry and its
EntryContaineris closed and unregistered, but its trees are still there, restored by the rollback.Scope
Not JDBC-specific.
BackendImplis the base of every pluggable backend, andPDBStorage.write()replays on itsown conflict exception, so JE/PDB reach it the same way. The trigger is narrow — a base DN removed from a
backend's configuration while a transaction conflict fires on the delete or its commit — but the outcome is a
half-applied configuration change reported under the wrong error.
Suggested fix
Move the non-transactional work out of
write(): keep onlyec.delete(txn)inside, and runderegisterBaseDN,unregisterEntryContainerandec.close()after the write has committed. Deriving the set to remove fromrootContainerrather than from the stalecfgwould make the loop replay-safe on its own, but it would stillleave the deregistration and the close unrecoverable if a later part of the change fails, so the reordering is
the durable fix.
A null guard on
unregisterEntryContainer's result is not sufficient — it is unreachable, sincederegisterBaseDNthrows one line earlier.Related
Found while reviewing #904 (#904 (review)).
That PR increases how often a JDBC conflict is replayed, which raises the odds of hitting this, but the defect is
present on master independently of it.