From 20d43c0d51839d0ef2e6963c8fcd1d2e4c62b234 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 27 Aug 2013 07:42:15 +0000
Subject: [PATCH] ServerState.java: Assigned serverIdToChangeNumber field in the declaration. In reload(), called update(). In cover(ServerState), cover(ChangeNumber). In duplicate(), called Map.putAll().
---
opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ExternalChangeLogTest.java | 80 ++++++++++----------------
opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java | 15 ----
opends/src/server/org/opends/server/replication/common/ServerState.java | 44 ++++----------
3 files changed, 47 insertions(+), 92 deletions(-)
diff --git a/opends/src/server/org/opends/server/replication/common/ServerState.java b/opends/src/server/org/opends/server/replication/common/ServerState.java
index 5970c1c..18f57b5 100644
--- a/opends/src/server/org/opends/server/replication/common/ServerState.java
+++ b/opends/src/server/org/opends/server/replication/common/ServerState.java
@@ -48,7 +48,8 @@
{
/** Associates a serverId with a ChangeNumber. */
- private final Map<Integer, ChangeNumber> serverIdToChangeNumber;
+ private final Map<Integer, ChangeNumber> serverIdToChangeNumber =
+ new HashMap<Integer, ChangeNumber>();
/**
* Whether the state has been saved to persistent storage. It starts at true,
* and moves to false when an update is made to the current object.
@@ -60,7 +61,7 @@
*/
public ServerState()
{
- serverIdToChangeNumber = new HashMap<Integer, ChangeNumber>();
+ super();
}
/**
@@ -92,10 +93,11 @@
{
try
{
- serverIdToChangeNumber = new HashMap<Integer, ChangeNumber>();
-
while (endpos > pos)
{
+ // FIXME JNR: why store the serverId separately from the changeNumber
+ // since the changeNumber already contains the serverId?
+
// read the ServerId
int length = getNextLength(in, pos);
String serverIdString = new String(in, pos, length, "UTF-8");
@@ -108,10 +110,9 @@
ChangeNumber cn = new ChangeNumber(cnString);
pos += length +1;
- // Add the serverid
+ // Add the serverId
serverIdToChangeNumber.put(serverId, cn);
}
-
} catch (UnsupportedEncodingException e)
{
throw new DataFormatException("UTF-8 is not supported by this jvm.");
@@ -183,7 +184,6 @@
return false;
boolean updated = false;
-
for (ChangeNumber cn : serverState.serverIdToChangeNumber.values())
{
if (update(cn))
@@ -191,7 +191,6 @@
updated = true;
}
}
-
return updated;
}
@@ -207,22 +206,11 @@
return false;
}
- boolean result = false;
-
synchronized (serverIdToChangeNumber)
{
clear();
- for (Integer serverId : serverState)
- {
- ChangeNumber maxChangeNumber = serverState.getChangeNumber(serverId);
- if (update(maxChangeNumber))
- {
- result = true;
- }
- }
+ return update(serverState);
}
-
- return result;
}
/**
@@ -357,7 +345,7 @@
{
for (ChangeNumber tmpMax : serverIdToChangeNumber.values())
{
- if ((maxCN==null) || (tmpMax.newer(maxCN)))
+ if (maxCN == null || tmpMax.newer(maxCN))
maxCN = tmpMax;
}
}
@@ -395,6 +383,7 @@
for (Entry<Integer, ChangeNumber> entry : serverIdToChangeNumber
.entrySet())
{
+ // serverId is useless, see comment in ServerState ctor
String serverIdStr = String.valueOf(entry.getKey());
idList.add(serverIdStr);
length += serverIdStr.length() + 1;
@@ -439,9 +428,7 @@
{
for (ChangeNumber coveredChange : covered.serverIdToChangeNumber.values())
{
- ChangeNumber change =
- this.serverIdToChangeNumber.get(coveredChange.getServerId());
- if ((change == null) || (change.older(coveredChange)))
+ if (!cover(coveredChange))
{
return false;
}
@@ -460,7 +447,7 @@
{
ChangeNumber change =
this.serverIdToChangeNumber.get(covered.getServerId());
- return !((change == null) || (change.older(covered)));
+ return change != null && !change.older(covered);
}
/**
@@ -482,10 +469,7 @@
ServerState newState = new ServerState();
synchronized (serverIdToChangeNumber)
{
- for (ChangeNumber change : serverIdToChangeNumber.values())
- {
- newState.serverIdToChangeNumber.put(change.getServerId(), change);
- }
+ newState.serverIdToChangeNumber.putAll(serverIdToChangeNumber);
}
return newState;
}
@@ -502,7 +486,7 @@
public static int diffChanges(ServerState ss1, ServerState ss2)
throws IllegalArgumentException
{
- if ((ss1 == null) || (ss2 == null))
+ if (ss1 == null || ss2 == null)
{
throw new IllegalArgumentException("Null server state(s)");
}
diff --git a/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java b/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
index 10e0b71..7a86d28 100644
--- a/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
+++ b/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
@@ -44,9 +44,9 @@
import org.opends.server.replication.common.ServerState;
import org.opends.server.replication.server.ReplicationServer;
import org.opends.server.replication.server.ReplicationServerDomain;
-import org.opends.server.replication.server.changelog.api.ChangelogException;
import org.opends.server.replication.server.changelog.api.ChangelogDB;
import org.opends.server.replication.server.changelog.api.ChangelogDBIterator;
+import org.opends.server.replication.server.changelog.api.ChangelogException;
import org.opends.server.replication.server.changelog.je.DraftCNDB.*;
import org.opends.server.types.Attribute;
import org.opends.server.types.Attributes;
@@ -317,15 +317,6 @@
return;
}
- // FIXME is this correct?
- // This code is not setting the excludedBaseDNs of the RS which means it
- // could take any value set by one of the other methods!
- // In addition, this code is not thread safe, but I suspect it is used in a
- // multi-threaded way.
- // The call to RS.getEligibleCN() is not reliable in any way and could
- // return very different values even if the DB content did not change!!
- ChangeNumber crossDomainEligibleCN = replicationServer.getEligibleCN();
-
for (int i = 0; i < 100; i++)
{
final DraftCNDBCursor cursor = db.openDeleteCursor();
@@ -361,10 +352,6 @@
}
final ServerState startState = domain.getStartState();
-
- // We don't use the returned endState but it's updating CN as reading
- domain.getEligibleState(crossDomainEligibleCN);
-
final ChangeNumber fcn = startState.getChangeNumber(cn.getServerId());
final int currentDraftCN = cursor.currentKey();
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ExternalChangeLogTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ExternalChangeLogTest.java
index 2f9eb2d..87841bb 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ExternalChangeLogTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ExternalChangeLogTest.java
@@ -101,11 +101,11 @@
/** The port of the replicationServer. */
private int replicationServerPort;
- private static final String TEST_ROOT_DN_STRING2 = "o=test2";
private static final String TEST_BACKEND_ID2 = "test2";
+ private static final String TEST_ROOT_DN_STRING2 = "o=" + TEST_BACKEND_ID2;
- private static final String TEST_ROOT_DN_STRING3 = "o=test3";
private static final String TEST_BACKEND_ID3 = "test3";
+ private static final String TEST_ROOT_DN_STRING3 = "o=" + TEST_BACKEND_ID3;
/** The LDAPStatistics object associated with the LDAP connection handler. */
private LDAPStatistics ldapStatistics;
@@ -648,18 +648,15 @@
server01.publish(delMsg1);
debugInfo(tn, "publishes:" + delMsg1);
- // Initialize a second test backend o=test2, in addition to o=test
// Configure replication on this backend
// Add the root entry in the backend
- backend2 = initializeTestBackend(false, TEST_ROOT_DN_STRING2,
- TEST_BACKEND_ID2);
+ backend2 = initializeTestBackend(false, TEST_BACKEND_ID2);
backend2.setPrivateBackend(true);
SortedSet<String> replServers = newSet("localhost:" + replicationServerPort);
DomainFakeCfg domainConf =
new DomainFakeCfg(baseDn2, 1602, replServers);
- domain2 = createDomain(domainConf, null,null);
- domain2.start();
+ domain2 = startNewDomain(domainConf, null,null);
sleep(1000);
addEntry(createEntry(baseDn2));
@@ -706,11 +703,7 @@
{
domain2.shutdown();
}
- if (backend2 != null)
- {
- removeTestBackend2(backend2);
- }
-
+ removeTestBackend(backend2);
stop(server01);
replicationServer.clearDb();
}
@@ -730,10 +723,10 @@
ReplicationBroker s2test = null;
ReplicationBroker s2test2 = null;
+ Backend backend2 = null;
try
{
- // Initialize a second test backend
- initializeTestBackend(true, TEST_ROOT_DN_STRING2, TEST_BACKEND_ID2);
+ backend2 = initializeTestBackend(true, TEST_BACKEND_ID2);
LDIFWriter ldifWriter = getLDIFWriter();
@@ -893,6 +886,7 @@
}
finally
{
+ removeTestBackend(backend2);
stop(s1test2, s2test, s1test, s2test2);
replicationServer.clearDb();
}
@@ -2229,14 +2223,10 @@
/**
* Utility - create a second backend in order to test ECL with 2 suffixes.
*/
- private static Backend initializeTestBackend(
- boolean createBaseEntry,
- String rootDN,
- String backendId)
- throws Exception
+ private static Backend initializeTestBackend(boolean createBaseEntry,
+ String backendId) throws Exception
{
-
- DN baseDN = DN.decode(rootDN);
+ DN baseDN = DN.decode("o=" + backendId);
// Retrieve backend. Warning: it is important to perform this each time,
// because a test may have disabled then enabled the backend (i.e a test
@@ -2265,12 +2255,15 @@
return memoryBackend;
}
- private static void removeTestBackend2(Backend backend)
+ private static void removeTestBackend(Backend backend)
{
- MemoryBackend memoryBackend = (MemoryBackend)backend;
- memoryBackend.clearMemoryBackend();
- memoryBackend.finalizeBackend();
- DirectoryServer.deregisterBackend(memoryBackend);
+ if (backend != null)
+ {
+ MemoryBackend memoryBackend = (MemoryBackend) backend;
+ memoryBackend.clearMemoryBackend();
+ memoryBackend.finalizeBackend();
+ DirectoryServer.deregisterBackend(memoryBackend);
+ }
}
private void ChangeTimeHeartbeatTest() throws Exception
@@ -2281,14 +2274,11 @@
ReplicationBroker s2test = null;
ReplicationBroker s1test2 = null;
ReplicationBroker s2test2 = null;
-
- // Initialize a second test backend
Backend backend2 = null;
try
{
- backend2 = initializeTestBackend(true, TEST_ROOT_DN_STRING2,
- TEST_BACKEND_ID2);
+ backend2 = initializeTestBackend(true, TEST_BACKEND_ID2);
// --
s1test = openReplicationSession(
@@ -2366,12 +2356,8 @@
finally
{
stop(s1test2, s2test2);
- if (backend2 != null)
- {
- removeTestBackend2(backend2);
- }
+ removeTestBackend(backend2);
stop(s1test, s2test);
-
replicationServer.clearDb();
}
debugInfo(tn, "Ending test successfully");
@@ -3052,10 +3038,9 @@
LDAPReplicationDomain domain21 = null;
try
{
- // Initialize a second test backend o=test2, in addtion to o=test
// Configure replication on this backend
// Add the root entry in the backend
- backend2 = initializeTestBackend(false, TEST_ROOT_DN_STRING2, TEST_BACKEND_ID2);
+ backend2 = initializeTestBackend(false, TEST_BACKEND_ID2);
DN baseDn2 = DN.decode(TEST_ROOT_DN_STRING2);
SortedSet<String> replServers = newSet("localhost:" + replicationServerPort);
@@ -3064,10 +3049,9 @@
SortedSet<String> eclInclude = newSet("sn", "roomnumber");
DomainFakeCfg domainConf = new DomainFakeCfg(baseDn2, 1702, replServers);
- domain2 = createDomain(domainConf, eclInclude, eclInclude);
- domain2.start();
+ domain2 = startNewDomain(domainConf, eclInclude, eclInclude);
- backend3 = initializeTestBackend(false, TEST_ROOT_DN_STRING3, TEST_BACKEND_ID3);
+ backend3 = initializeTestBackend(false, TEST_BACKEND_ID3);
DN baseDn3 = DN.decode(TEST_ROOT_DN_STRING3);
// on o=test3,sid=1703 include attrs set to : 'objectclass'
@@ -3076,15 +3060,13 @@
SortedSet<String> eclIncludeForDeletes = newSet("*");
domainConf = new DomainFakeCfg(baseDn3, 1703, replServers);
- domain3 = createDomain(domainConf, eclInclude, eclIncludeForDeletes);
- domain3.start();
+ domain3 = startNewDomain(domainConf, eclInclude, eclIncludeForDeletes);
// on o=test2,sid=1704 include attrs set to : 'cn'
eclInclude = newSet("cn");
domainConf = new DomainFakeCfg(baseDn2, 1704, replServers);
- domain21 = createDomain(domainConf, eclInclude, eclInclude);
- domain21.start();
+ domain21 = startNewDomain(domainConf, eclInclude, eclInclude);
sleep(1000);
@@ -3208,13 +3190,13 @@
{
domain2.shutdown();
}
- removeTestBackend2(backend2);
+ removeTestBackend(backend2);
if (domain3 != null)
{
domain3.shutdown();
}
- removeTestBackend2(backend3);
+ removeTestBackend(backend3);
}
finally
{
@@ -3229,7 +3211,7 @@
return new TreeSet<String>(Arrays.asList(values));
}
- private LDAPReplicationDomain createDomain(DomainFakeCfg domainConf,
+ private LDAPReplicationDomain startNewDomain(DomainFakeCfg domainConf,
SortedSet<String> eclInclude, SortedSet<String> eclIncludeForDeletes)
throws Exception
{
@@ -3238,7 +3220,9 @@
// Set a Changetime heartbeat interval low enough (less than default value
// that is 1000 ms) for the test to be sure to consider all changes as eligible.
domainConf.setChangetimeHeartbeatInterval(10);
- return MultimasterReplication.createNewDomain(domainConf);
+ LDAPReplicationDomain newDomain = MultimasterReplication.createNewDomain(domainConf);
+ newDomain.start();
+ return newDomain;
}
private void runModifyOperation(Entry entry, List<Modification> mods)
--
Gitblit v1.10.0