diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java index 8781e700bf..a63f74a2cc 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java @@ -1442,8 +1442,6 @@ protected void initializeRemote(int serverToInitialize, int serverRunningTheTask, Task initTask, int initWindow) throws DirectoryException { - final ImportExportContext ieCtx = acquireIEContext(false); - /* We manage the list of servers to initialize in order : - to test at the end that all expected servers have reconnected @@ -1451,7 +1449,12 @@ protected void initializeRemote(int serverToInitialize, - to update the task with the server(s) where this test failed */ - Map replicaInfos = getReplicaInfos(); + // Validate the request before acquiring the import/export context: a + // validation failure must not leave the context acquired, otherwise + // ieRunning() would remain true forever and the domain would reject every + // subsequent total update as a simultaneous import/export. + final Map replicaInfos = getReplicaInfos(); + final DSInfo targetDsi; if (serverToInitialize == RoutableMsg.ALL_SERVERS) { if (replicaInfos.isEmpty()) @@ -1459,7 +1462,44 @@ protected void initializeRemote(int serverToInitialize, throw new DirectoryException(UNWILLING_TO_PERFORM, ERR_FULL_UPDATE_NO_REMOTES.get(getBaseDN(), getServerId())); } + targetDsi = null; + } + else + { + targetDsi = getDsInfoOrNull(replicaInfos.values(), serverToInitialize); + if (targetDsi == null) + { + throw new DirectoryException(UNWILLING_TO_PERFORM, + ERR_FULL_UPDATE_MISSING_REMOTE.get(getBaseDN(), getServerId(), serverToInitialize)); + } + } + final ImportExportContext ieCtx = acquireIEContext(false); + try + { + initializeRemote(ieCtx, replicaInfos, targetDsi, serverToInitialize, + serverRunningTheTask, initTask, initWindow); + } + finally + { + // Release the context whatever the outcome, otherwise ieRunning() would + // remain true forever (resolves the historical "FIXME should not this + // be in a finally?"). + releaseIEContext(); + } + } + + /** + * Performs the remote initialization with the import/export context already + * acquired - and released - by the caller. + */ + private void initializeRemote(ImportExportContext ieCtx, + Map replicaInfos, DSInfo targetDsi, + int serverToInitialize, int serverRunningTheTask, Task initTask, + int initWindow) throws DirectoryException + { + if (serverToInitialize == RoutableMsg.ALL_SERVERS) + { logger.info(NOTE_FULL_UPDATE_ENGAGED_FOR_REMOTE_START_ALL, countEntries(), getBaseDN(), getServerId()); @@ -1475,18 +1515,11 @@ protected void initializeRemote(int serverToInitialize, } else { - DSInfo dsi = getDsInfoOrNull(replicaInfos.values(), serverToInitialize); - if (dsi == null) - { - throw new DirectoryException(UNWILLING_TO_PERFORM, - ERR_FULL_UPDATE_MISSING_REMOTE.get(getBaseDN(), getServerId(), serverToInitialize)); - } - logger.info(NOTE_FULL_UPDATE_ENGAGED_FOR_REMOTE_START, countEntries(), getBaseDN(), getServerId(), serverToInitialize); ieCtx.startList.add(serverToInitialize); - ieCtx.setAckVal(dsi.getDsId(), 0); + ieCtx.setAckVal(targetDsi.getDsId(), 0); } DirectoryException exportRootException = null; @@ -1625,9 +1658,6 @@ protected void initializeRemote(int serverToInitialize, ERR_INIT_NO_SUCCESS_END_FROM_SERVERS.get(getGenerationID(), ieCtx.failureList)); } - // Don't forget to release IEcontext acquired at beginning. - releaseIEContext(); // FIXME should not this be in a finally? - final String cause = exportRootException == null ? "" : exportRootException.getLocalizedMessage(); if (serverToInitialize == RoutableMsg.ALL_SERVERS) diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java index e2d0836b85..64f16958f5 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java @@ -656,6 +656,7 @@ public void initializeTargetExport() throws Exception } // Launch in S1 the task that will initialize S2 + waitForRemoteReplicas(server2ID); addTask(taskInitTargetS2, ResultCode.SUCCESS, null); // Signal RS we just entered the full update status @@ -714,6 +715,7 @@ public void initializeTargetExportAll() throws Exception } // Launch in S1 the task that will initialize S2 + waitForRemoteReplicas(server2ID, server3ID); addTask(taskInitTargetAll, ResultCode.SUCCESS, null); // Tests that entries have been received by S2 @@ -1004,6 +1006,7 @@ public void initializeTargetExportMultiSS() throws Exception // Launch in S1 the task that will initialize S2 log(testCase + " add task " + Thread.currentThread()); + waitForRemoteReplicas(server2ID); addTask(taskInitTargetS2, ResultCode.SUCCESS, null); log(testCase + " " + server2.getServerId() + " wait target " + Thread.currentThread()); @@ -1027,6 +1030,47 @@ public void initializeTargetExportMultiSS() throws Exception } } + /** + * Tests that an InitializeTarget task failing its validation (the remote + * replica is unknown to the domain) does not leave the import/export + * context acquired (issue #730): the domain used to reject every subsequent + * total update as a simultaneous import/export. + */ + @Test(enabled=true) + public void initializeTargetUnknownRemote() throws Exception + { + String testCase = "initializeTargetUnknownRemote"; + log("Starting " + testCase); + try + { + replServer1 = createReplicationServer(replServer1ID, testCase); + connectServer1ToReplServer(replServer1ID); + addTestEntriesToDB(); + + // DS(42424) is unknown in the topology: the task must fail to start... + Entry taskInitTargetUnknown = TestCaseUtils.makeEntry( + "dn: ds-task-id=" + UUID.randomUUID() + ",cn=Scheduled Tasks,cn=Tasks", + "objectclass: top", + "objectclass: ds-task", + "objectclass: ds-task-initialize-remote-replica", + "ds-task-class-name: org.opends.server.tasks.InitializeTargetTask", + "ds-task-initialize-domain-dn: " + EXAMPLE_DN, + "ds-task-initialize-replica-server-id: " + 42424); + addTask(taskInitTargetUnknown, ResultCode.SUCCESS, null); + waitTaskState(taskInitTargetUnknown, STOPPED_BY_ERROR, 20000, null); + + // ...but must not leave the import/export context acquired + assertFalse(replDomain.ieRunning(), + "ReplicationDomain: Import/Export is not expected to be running after a failed InitializeTarget"); + + log("Successfully ending " + testCase); + } + finally + { + afterTest(testCase); + } + } + private void waitForInitializeTargetMsg(String testCase, ReplicationBroker server) throws Exception { @@ -1293,9 +1337,30 @@ public void initializeSimultaneous() throws Exception * Disconnect broker and remove entries from the local DB * @param testCase The name of the test case. */ + /** + * Waits until the local replication domain sees the provided replicas in + * its topology view, so that an InitializeTarget task does not race the + * topology propagation and fail to start with "the remote directory server + * DS(x) is unknown" - which, combined with the import/export context leak + * (issue #730), used to poison the domain for the rest of the test class. + */ + private void waitForRemoteReplicas(Integer... serverIds) throws Exception + { + for (int serverId : serverIds) + { + for (int i = 0; i < 100 && !replDomain.getReplicaInfos().containsKey(serverId); i++) + { + sleep(100); + } + assertTrue(replDomain.getReplicaInfos().containsKey(serverId), + "DS(" + serverId + ") is not known to the local replication domain"); + } + } + private void afterTest(String testCase) throws Exception { // Check that the domain has completed the import/export task. + boolean ieStillRunning = false; if (replDomain != null) { // race condition could cause the main thread to reach @@ -1309,7 +1374,10 @@ private void afterTest(String testCase) throws Exception } sleep(500); } - assertFalse(replDomain.ieRunning(), "ReplicationDomain: Import/Export is not expected to be running"); + // asserted only after the cleanup below: failing before it would leak + // the domain config, the brokers and the replication servers into the + // following tests and cascade the failure over the whole class + ieStillRunning = replDomain.ieRunning(); } // Remove domain config super.cleanConfigEntries(); @@ -1330,6 +1398,8 @@ private void afterTest(String testCase) throws Exception Arrays.fill(replServerPort, 0); log("Successfully cleaned " + testCase); + + assertFalse(ieStillRunning, "ReplicationDomain: Import/Export is not expected to be running"); } /** diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java index 3f302f834a..54d858cb82 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java @@ -13,6 +13,7 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC. */ package org.opends.server.replication; @@ -726,11 +727,13 @@ public Entry call() throws Exception } } - if (expectedTaskState == RUNNING && taskState == COMPLETED_SUCCESSFULLY) + if (expectedTaskState == RUNNING + && (taskState == COMPLETED_SUCCESSFULLY || taskState == STOPPED_BY_ERROR)) { // We usually wait the running state after adding the task - // and if the task is fast enough then it may be already done - // and we can go on. + // and if the task is fast enough then it may already be done + // (successfully or not) and we can go on: callers interested in the + // final state wait for it explicitly afterwards. } else {