From c59e1a1d20a92a318e22141a5d9cfded75f61f89 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Fri, 11 Sep 2026 13:07:08 +0000
Subject: [PATCH] [#951] Keep the persisted position when a domain drops its in-memory ServerState (#970)
---
opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java | 17 +++-
opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/PersistentServerStateTest.java | 38 +++++++++
opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/DisabledDomainServerStateTest.java | 184 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 234 insertions(+), 5 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java
index 4f2ab78..6b5bba9 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java
@@ -370,16 +370,20 @@
}
/**
- * Empty the ServerState in memory.
+ * Drop the in-memory copy of the ServerState, leaving persistent storage
+ * holding whatever it holds.
* <p>
- * The emptied state is marked as not saved, so the next save writes the empty
- * state out - which is what {@link #clear()} is after. A caller that only
- * means to drop the in-memory copy, and expects the backend to keep what it
- * holds, has to keep saves away until it has loaded the state back.
+ * The emptied state is marked as saved, because nothing about it is waiting
+ * to be written: the callers - a domain being disabled, and a domain about to
+ * load its state back - drop the copy in memory without meaning the base
+ * entry to lose its position. Marking it as not saved would have the next
+ * checkpoint, or the last save the state checkpointer runs on its way out,
+ * replace the CSNs on the base entry with nothing.
*/
public void clearInMemory()
{
state.clear();
+ state.setSaved(true);
}
/**
@@ -388,6 +392,9 @@
void clear()
{
clearInMemory();
+ // Emptying persistent storage too is the point of this method, so the
+ // emptied state does have to be written out.
+ state.setSaved(false);
save();
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/DisabledDomainServerStateTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/DisabledDomainServerStateTest.java
new file mode 100644
index 0000000..3da1204
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/DisabledDomainServerStateTest.java
@@ -0,0 +1,184 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package org.opends.server.replication.plugin;
+
+import static org.forgerock.opendj.ldap.ModificationType.*;
+import static org.opends.server.TestCaseUtils.*;
+import static org.opends.server.protocols.internal.InternalClientConnection.*;
+import static org.opends.server.protocols.internal.Requests.*;
+import static org.testng.Assert.*;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.DN;
+import org.forgerock.opendj.ldap.ResultCode;
+import org.forgerock.opendj.ldap.SearchScope;
+import org.forgerock.opendj.server.config.meta.ReplicationDomainCfgDefn.IsolationPolicy;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.core.ModifyOperation;
+import org.opends.server.protocols.internal.InternalSearchOperation;
+import org.opends.server.protocols.internal.SearchRequest;
+import org.opends.server.replication.ReplicationTestCase;
+import org.opends.server.types.Attribute;
+import org.opends.server.types.SearchResultEntry;
+import org.testng.annotations.Test;
+
+/**
+ * Test that disabling a replication domain leaves the position it persisted in
+ * the base entry alone.
+ */
+@SuppressWarnings("javadoc")
+public class DisabledDomainServerStateTest extends ReplicationTestCase
+{
+ /** The attribute the domain checkpoints its ServerState to. */
+ private static final String REPLICATION_STATE = "ds-sync-state";
+
+ /**
+ * An import or a restore task disables the domain, and the server can be shut
+ * down before the task enables it back. The last save the state checkpointer
+ * runs on its way out must not write the emptied state over the CSNs the base
+ * entry carries: the domain would come back up with no position at all, and
+ * would have everything the replication server still holds replayed to it.
+ */
+ @Test
+ public void disabledDomainKeepsItsPersistedStateOnShutdown() throws Exception
+ {
+ final DN baseDN = DN.valueOf(TEST_ROOT_DN_STRING);
+ LDAPReplicationDomain domain = null;
+ try
+ {
+ domain = startDisconnectedDomain(baseDN);
+
+ // A change of our own gives the domain a position to lose.
+ final ModifyOperation op = getRootConnection().processModify(
+ modifyRequest(baseDN, REPLACE, "description", "test"));
+ assertEquals(op.getResultCode(), ResultCode.SUCCESS, op.getAdditionalLogItems().toString());
+
+ // Checkpoint it, the way the state checkpointer does every second.
+ domain.backupStart();
+ final List<String> checkpointed = persistedState(baseDN);
+ assertFalse(checkpointed.isEmpty(), "the change was not checkpointed to " + REPLICATION_STATE);
+
+ domain.disable();
+
+ // Shutting the domain down runs the last save of the checkpointer.
+ MultimasterReplication.deleteDomain(baseDN);
+ domain = null;
+
+ assertEquals(persistedState(baseDN), checkpointed,
+ "disabling the domain dropped the position persisted in " + REPLICATION_STATE);
+ }
+ finally
+ {
+ if (domain != null)
+ {
+ MultimasterReplication.deleteDomain(baseDN);
+ }
+ }
+ }
+
+ /**
+ * An online {@code import-ldif} on a replicated backend reaches
+ * {@code disable()} twice, and unlike the interleaving above this is not a
+ * race: it happens every time. The task disables the domain from
+ * {@code notifyImportBeginning} (MultimasterReplication:671), and the backend
+ * it disables next has the domain disabled again through the backend
+ * initialization listener it registers (LDAPReplicationDomain:279-287) -
+ * nothing keeps that second call out, since the flag which would,
+ * {@code ignoreBackendInitializationEvent}, belongs to the total update road.
+ * A restore of a replicated backend takes the same pair.
+ * <p>
+ * The first call saves the position and drops the copy in memory; the second
+ * one must not write that dropped copy over what the first one saved. The road
+ * where it costs the most is the one on which the import then leaves the data
+ * alone - an exclusive lock it could not take, for instance: the domain is
+ * enabled back with no position over data which still holds every change it
+ * had replayed.
+ */
+ @Test
+ public void aDomainDisabledTwiceKeepsItsPersistedState() throws Exception
+ {
+ final DN baseDN = DN.valueOf(TEST_ROOT_DN_STRING);
+ LDAPReplicationDomain domain = null;
+ try
+ {
+ domain = startDisconnectedDomain(baseDN);
+
+ // A change of our own gives the domain a position to lose.
+ final ModifyOperation op = getRootConnection().processModify(
+ modifyRequest(baseDN, REPLACE, "description", "test"));
+ assertEquals(op.getResultCode(), ResultCode.SUCCESS, op.getAdditionalLogItems().toString());
+
+ // The task disabling the domain, which is what saves the position.
+ domain.disable();
+ final List<String> saved = persistedState(baseDN);
+ assertFalse(saved.isEmpty(), "the position was not saved to " + REPLICATION_STATE + " by disable()");
+
+ // The backend the task disables next, disabling the domain a second time.
+ domain.disable();
+
+ assertEquals(persistedState(baseDN), saved,
+ "the second disable() dropped the position saved in " + REPLICATION_STATE);
+ }
+ finally
+ {
+ if (domain != null)
+ {
+ MultimasterReplication.deleteDomain(baseDN);
+ }
+ }
+ }
+
+ /**
+ * Starts a domain of the given base DN with no replication server to connect
+ * to: it accepts the changes anyway, which is all these tests need it to do.
+ */
+ private LDAPReplicationDomain startDisconnectedDomain(DN baseDN) throws Exception
+ {
+ final SortedSet<String> replServers = new TreeSet<>();
+ replServers.add("localhost:" + TestCaseUtils.findFreePort());
+ final DomainFakeCfg domainConf = new DomainFakeCfg(baseDN, 1, replServers);
+ domainConf.setHeartbeatInterval(100000);
+ domainConf.setIsolationPolicy(IsolationPolicy.ACCEPT_ALL_UPDATES);
+ final LDAPReplicationDomain domain = MultimasterReplication.createNewDomain(domainConf);
+ domain.start();
+ return domain;
+ }
+
+ /** Returns the values {@code ds-sync-state} carries on the base entry. */
+ private List<String> persistedState(DN baseDN)
+ {
+ final SearchRequest request =
+ newSearchRequest(baseDN, SearchScope.BASE_OBJECT).addAttribute(REPLICATION_STATE);
+ final InternalSearchOperation search = getRootConnection().processSearch(request);
+ assertEquals(search.getResultCode(), ResultCode.SUCCESS, search.getErrorMessage().toString());
+
+ final SearchResultEntry entry = search.getSearchEntries().getFirst();
+ final List<String> values = new ArrayList<>();
+ for (Attribute attr : entry.getAllAttributes(REPLICATION_STATE))
+ {
+ for (ByteString value : attr)
+ {
+ values.add(value.toString());
+ }
+ }
+ return values;
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/PersistentServerStateTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/PersistentServerStateTest.java
index 44ff6a2..8ea150e 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/PersistentServerStateTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/PersistentServerStateTest.java
@@ -103,6 +103,44 @@
}
/**
+ * Dropping the in-memory copy of the state does not ask for the emptied state
+ * to be written: the backend keeps the CSNs it holds until something loads
+ * them back or writes newer ones.
+ */
+ @Test(dataProvider = "suffix")
+ public void clearInMemoryLeavesTheBackendStateAlone(String dn) throws Exception
+ {
+ DN baseDn = DN.valueOf(dn);
+ ServerState origState = new ServerState();
+ PersistentServerState state = new PersistentServerState(baseDn, 1, origState);
+ CSNGenerator gen1 = new CSNGenerator(1, origState);
+ CSNGenerator gen2 = new CSNGenerator(2, origState);
+
+ CSN csn1 = gen1.newCSN();
+ CSN csn2 = gen2.newCSN();
+
+ state.update(csn1);
+ state.update(csn2);
+ state.save();
+
+ // What a domain being disabled does: the copy in memory goes, the backend
+ // is left holding the position.
+ state.clearInMemory();
+
+ // The next checkpoint, or the last one the flush thread runs on its way
+ // out, must not write the emptied state over it.
+ state.save();
+
+ PersistentServerState stateSaved =
+ new PersistentServerState(baseDn, 1, new ServerState());
+
+ assertEquals(stateSaved.getMaxCSN(1), csn1,
+ "csn1 was dropped from persistent storage by clearInMemory() for " + dn);
+ assertEquals(stateSaved.getMaxCSN(2), csn2,
+ "csn2 was dropped from persistent storage by clearInMemory() for " + dn);
+ }
+
+ /**
* An update landing while the state is being written cannot be part of that
* write, so it must leave the state unsaved and be written by the next save.
* Marking the state as saved on behalf of a write that does not carry the
--
Gitblit v1.10.0