From 4c7b80df408088a959bca12fe168ef4ffc039cdb Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Wed, 01 May 2013 17:23:20 +0000
Subject: [PATCH] CR-1632 Fix OPENDJ-888 Maintaining ds-sync-hist for a large group is inefficient
---
opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java | 8 +-
opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java | 95 ++++++++++++++++---------------
opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java | 17 +++--
opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java | 5 +
opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java | 32 ++++++----
5 files changed, 86 insertions(+), 71 deletions(-)
diff --git a/opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java b/opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java
index 29cb91c..9a12261 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java
@@ -23,11 +23,12 @@
*
*
* Copyright 2006-2010 Sun Microsystems, Inc.
+ * Portions Copyright 2013 ForgeRock, AS.
*/
package org.opends.server.replication.plugin;
-import java.util.ArrayList;
import java.util.Iterator;
+import java.util.Map;
import org.opends.server.replication.common.ChangeNumber;
import org.opends.server.types.AttributeType;
@@ -46,7 +47,7 @@
* It should use whatever historical information is stored in this class
* to solve the conflict and modify the mod and the mods iterator accordingly
*
- * @param modsIterator The iterator on the mods from which the mod is\
+ * @param modsIterator The iterator on the mods from which the mod is
* extracted.
* @param changeNumber The changeNumber associated to the operation.
* @param modifiedEntry The entry modified by this operation.
@@ -93,7 +94,8 @@
*
* @return the List of ValueInfo
*/
- public abstract ArrayList<AttrValueHistorical> getValuesHistorical();
+ public abstract Map<AttrValueHistorical,AttrValueHistorical>
+ getValuesHistorical();
/**
diff --git a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
index 43e9e37..a48125b 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
@@ -27,8 +27,9 @@
*/
package org.opends.server.replication.plugin;
-import java.util.ArrayList;
import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
import org.opends.server.replication.common.ChangeNumber;
import org.opends.server.types.Attribute;
@@ -39,9 +40,8 @@
import org.opends.server.types.Modification;
import org.opends.server.types.ModificationType;
-
/**
- * This classes is used to store historical information for multiple valued
+ * This class is used to store historical information for multiple valued
* attributes.
* One object of this type is created for each attribute that was changed in
* the entry.
@@ -51,9 +51,11 @@
*/
public class AttrHistoricalMultiple extends AttrHistorical
{
- private ChangeNumber deleteTime, // last time when the attribute was deleted
- lastUpdateTime; // last time the attribute was modified
- private ArrayList<AttrValueHistorical> valuesHist;
+ /** Last time when the attribute was deleted. */
+ private ChangeNumber deleteTime;
+ /** Last time the attribute was modified. */
+ private ChangeNumber lastUpdateTime;
+ private final Map<AttrValueHistorical,AttrValueHistorical> valuesHist;
/**
* Create a new object from the provided informations.
@@ -63,12 +65,13 @@
*/
public AttrHistoricalMultiple(ChangeNumber deleteTime,
ChangeNumber updateTime,
- ArrayList<AttrValueHistorical> valuesHist)
+ Map<AttrValueHistorical,AttrValueHistorical> valuesHist)
{
this.deleteTime = deleteTime;
this.lastUpdateTime = updateTime;
if (valuesHist == null)
- this.valuesHist = new ArrayList<AttrValueHistorical>();
+ this.valuesHist =
+ new LinkedHashMap<AttrValueHistorical,AttrValueHistorical>();
else
this.valuesHist = valuesHist;
}
@@ -80,7 +83,8 @@
{
this.deleteTime = null;
this.lastUpdateTime = null;
- this.valuesHist = new ArrayList<AttrValueHistorical>();
+ this.valuesHist =
+ new LinkedHashMap<AttrValueHistorical,AttrValueHistorical>();
}
/**
@@ -96,6 +100,7 @@
* Returns the last time when the attribute was deleted.
* @return the last time when the attribute was deleted
*/
+ @Override
public ChangeNumber getDeleteTime()
{
return deleteTime;
@@ -125,7 +130,7 @@
// iterate through the values in the valuesInfo
// and suppress all the values that have not been added
// after the date of this delete.
- Iterator<AttrValueHistorical> it = this.valuesHist.iterator();
+ Iterator<AttrValueHistorical> it = valuesHist.keySet().iterator();
while (it.hasNext())
{
AttrValueHistorical info = it.next();
@@ -154,8 +159,8 @@
protected void delete(AttributeValue val, ChangeNumber CN)
{
AttrValueHistorical info = new AttrValueHistorical(val, null, CN);
- this.valuesHist.remove(info);
- this.valuesHist.add(info);
+ valuesHist.remove(info);
+ valuesHist.put(info, info);
if (CN.newer(lastUpdateTime))
{
lastUpdateTime = CN;
@@ -176,8 +181,8 @@
for (AttributeValue val : attr)
{
AttrValueHistorical info = new AttrValueHistorical(val, null, CN);
- this.valuesHist.remove(info);
- this.valuesHist.add(info);
+ valuesHist.remove(info);
+ valuesHist.put(info, info);
if (CN.newer(lastUpdateTime))
{
lastUpdateTime = CN;
@@ -196,8 +201,8 @@
protected void add(AttributeValue addedValue, ChangeNumber CN)
{
AttrValueHistorical info = new AttrValueHistorical(addedValue, CN, null);
- this.valuesHist.remove(info);
- valuesHist.add(info);
+ valuesHist.remove(info);
+ valuesHist.put(info, info);
if (CN.newer(lastUpdateTime))
{
lastUpdateTime = CN;
@@ -217,8 +222,8 @@
for (AttributeValue val : attr)
{
AttrValueHistorical info = new AttrValueHistorical(val, CN, null);
- this.valuesHist.remove(info);
- valuesHist.add(info);
+ valuesHist.remove(info);
+ valuesHist.put(info, info);
if (CN.newer(lastUpdateTime))
{
lastUpdateTime = CN;
@@ -231,7 +236,8 @@
*
* @return the list of historical informations for the values.
*/
- public ArrayList<AttrValueHistorical> getValuesHistorical()
+ @Override
+ public Map<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
{
return valuesHist;
}
@@ -239,6 +245,7 @@
/**
* {@inheritDoc}
*/
+ @Override
public boolean replayOperation(
Iterator<Modification> modsIterator, ChangeNumber changeNumber,
Entry modifiedEntry, Modification m)
@@ -331,6 +338,7 @@
* @param changeNumber The changeNumber of the operation to process
* @param mod The modify operation to process.
*/
+ @Override
public void processLocalOrNonConflictModification(ChangeNumber changeNumber,
Modification mod)
{
@@ -417,8 +425,8 @@
m.setModificationType(ModificationType.REPLACE);
AttributeBuilder builder = new AttributeBuilder(modAttr, true);
- for (Iterator<AttrValueHistorical> it = getValuesHistorical().iterator();
- it.hasNext();)
+ Iterator<AttrValueHistorical> it = valuesHist.keySet().iterator();
+ while (it.hasNext())
{
AttrValueHistorical valInfo; valInfo = it.next();
@@ -457,25 +465,21 @@
}
else
{
- /*
- * we are processing DELETE of some attribute values
- */
- ArrayList<AttrValueHistorical> valuesInfo = getValuesHistorical();
+ // we are processing DELETE of some attribute values
AttributeBuilder builder = new AttributeBuilder(modAttr);
for (AttributeValue val : modAttr)
{
- Boolean deleteIt = true; // true if the delete must be done
- Boolean addedInCurrentOp = false;
+ boolean deleteIt = true; // true if the delete must be done
+ boolean addedInCurrentOp = false;
/* update historical information */
AttrValueHistorical valInfo =
new AttrValueHistorical(val, null, changeNumber);
- int index = valuesInfo.indexOf(valInfo);
- if (index != -1)
+ AttrValueHistorical oldValInfo = valuesHist.get(valInfo);
+ if (oldValInfo != null)
{
/* this value already exist in the historical information */
- AttrValueHistorical oldValInfo = valuesInfo.get(index);
if (changeNumber.equals(oldValInfo.getValueUpdateTime()))
{
// This value was added earlier in the same operation
@@ -485,8 +489,8 @@
if (changeNumber.newerOrEquals(oldValInfo.getValueDeleteTime()) &&
changeNumber.newerOrEquals(oldValInfo.getValueUpdateTime()))
{
- valuesInfo.remove(index);
- valuesInfo.add(valInfo);
+ valuesHist.remove(oldValInfo);
+ valuesHist.put(valInfo, valInfo);
}
else if (oldValInfo.isUpdate())
{
@@ -495,7 +499,8 @@
}
else
{
- valuesInfo.add(valInfo);
+ valuesHist.remove(oldValInfo);
+ valuesHist.put(valInfo, valInfo);
}
/* if the attribute value is not to be deleted
@@ -563,32 +568,30 @@
AttributeBuilder builder = new AttributeBuilder(m.getAttribute());
for (AttributeValue addVal : m.getAttribute())
{
- ArrayList<AttrValueHistorical> valuesInfo = getValuesHistorical();
AttrValueHistorical valInfo =
new AttrValueHistorical(addVal, changeNumber, null);
- int index = valuesInfo.indexOf(valInfo);
- if (index == -1)
+ AttrValueHistorical oldValInfo = valuesHist.get(valInfo);
+ if (oldValInfo == null)
{
- /* this values does not exist yet
+ /* this value does not exist yet
* add it in the historical information
* let the operation process normally
*/
- valuesInfo.add(valInfo);
+ valuesHist.put(valInfo, valInfo);
}
else
{
- AttrValueHistorical oldValueInfo = valuesInfo.get(index);
- if (oldValueInfo.isUpdate())
+ if (oldValInfo.isUpdate())
{
/* if the value is already present
* check if the updateTime must be updated
* in all cases suppress this value from the value list
* as it is already present in the entry
*/
- if (changeNumber.newer(oldValueInfo.getValueUpdateTime()))
+ if (changeNumber.newer(oldValInfo.getValueUpdateTime()))
{
- valuesInfo.remove(index);
- valuesInfo.add(valInfo);
+ valuesHist.remove(oldValInfo);
+ valuesHist.put(valInfo, valInfo);
}
builder.remove(addVal);
}
@@ -597,15 +600,15 @@
/* this value is marked as a deleted value
* check if this mod is more recent the this delete
*/
- if (changeNumber.newerOrEquals(oldValueInfo.getValueDeleteTime()))
+ if (changeNumber.newerOrEquals(oldValInfo.getValueDeleteTime()))
{
/* this add is more recent,
* remove the old delete historical information
* and add our more recent one
* let the operation process
*/
- valuesInfo.remove(index);
- valuesInfo.add(valInfo);
+ valuesHist.remove(oldValInfo);
+ valuesHist.put(valInfo, valInfo);
}
else
{
diff --git a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
index 8cc59ea..99bbc23 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
@@ -27,8 +27,9 @@
*/
package org.opends.server.replication.plugin;
-import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
+import java.util.Map;
import org.opends.server.replication.common.ChangeNumber;
import org.opends.server.types.Attribute;
@@ -39,7 +40,7 @@
import org.opends.server.types.ModificationType;
/**
- * This classes is used to store historical information for single valued
+ * This class is used to store historical information for single valued
* attributes.
* One object of this type is created for each attribute that was changed in
* the entry.
@@ -48,13 +49,17 @@
*/
public class AttrHistoricalSingle extends AttrHistorical
{
- private ChangeNumber deleteTime = null; // last time when the attribute was
- // deleted
- private ChangeNumber addTime = null; // last time when a value was added
- private AttributeValue value = null; // last added value
+ /** Last time when the attribute was deleted. */
+ private ChangeNumber deleteTime = null;
+ /** Last time when a value was added. */
+ private ChangeNumber addTime = null;
+ /** Last added value. */
+ private AttributeValue value = null;
- // last operation applied. This is only used for multiple mods on the same
- // single valued attribute in the same modification.
+/**
+ * last operation applied. This is only used for multiple mods on the same
+ * single valued attribute in the same modification.
+ */
private HistAttrModificationKey lastMod = null;
/**
@@ -70,17 +75,18 @@
* {@inheritDoc}
*/
@Override
- public ArrayList<AttrValueHistorical> getValuesHistorical()
+ public Map<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
{
if (addTime == null)
{
- return new ArrayList<AttrValueHistorical>();
+ return Collections.<AttrValueHistorical,AttrValueHistorical>emptyMap();
}
else
{
- ArrayList<AttrValueHistorical> values =
- new ArrayList<AttrValueHistorical>();
- values.add(new AttrValueHistorical(value, addTime, null));
+ AttrValueHistorical val = new AttrValueHistorical(value, addTime, null);
+ Map<AttrValueHistorical,AttrValueHistorical> values =
+ Collections.<AttrValueHistorical,AttrValueHistorical>
+ singletonMap(val, val);
return values;
}
}
diff --git a/opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java b/opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java
index 5ff0c41..1561d53 100644
--- a/opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java
+++ b/opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2006-2010 Sun Microsystems, Inc.
- * Portions Copyright 2011-2012 ForgeRock AS
+ * Portions Copyright 2011-2013 ForgeRock AS
*/
package org.opends.server.replication.plugin;
@@ -493,7 +493,8 @@
delAttr = true;
}
- for (AttrValueHistorical attrValHist : attrHist.getValuesHistorical())
+ for (AttrValueHistorical attrValHist : attrHist.getValuesHistorical()
+ .keySet())
{
// Encode an attribute value
String strValue;
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java
index da52880..35cd909 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java
@@ -23,10 +23,13 @@
*
*
* Copyright 2006-2010 Sun Microsystems, Inc.
+ * Portions Copyright 2013 ForgeRock AS.
*/
package org.opends.server.replication.plugin;
import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Map;
import org.opends.server.core.DirectoryServer;
import org.opends.server.replication.ReplicationTestCase;
@@ -89,15 +92,15 @@
// Check add(AttributeValue val, ChangeNumber CN)
attrInfo1.add(att, updateTime);
- ArrayList<AttrValueHistorical> values1 = attrInfo1.getValuesHistorical();
+ Map<AttrValueHistorical,AttrValueHistorical> values1 = attrInfo1.getValuesHistorical();
assertTrue(values1.size() == 1);
AttrValueHistorical valueInfo1 = new AttrValueHistorical(att, updateTime, null);
- assertTrue(values1.get(0).equals(valueInfo1));
+ assertTrue(values1.containsKey(valueInfo1));
// Check constructor with parameter
AttrValueHistorical valueInfo2 = new AttrValueHistorical(att, updateTime, deleteTime);
- ArrayList<AttrValueHistorical> values = new ArrayList<AttrValueHistorical>();
- values.add(valueInfo2);
+ HashMap<AttrValueHistorical,AttrValueHistorical> values = new HashMap<AttrValueHistorical,AttrValueHistorical>();
+ values.put(valueInfo2,valueInfo2);
AttrHistoricalMultiple attrInfo2 = new AttrHistoricalMultiple(deleteTime, updateTime, values);
// Check equality
@@ -106,14 +109,14 @@
// Check constructor with time parameter and not Value
AttrHistoricalMultiple attrInfo3 = new AttrHistoricalMultiple(deleteTime, updateTime, null);
attrInfo3.add(att, updateTime);
- ArrayList<AttrValueHistorical> values3 = attrInfo3.getValuesHistorical();
+ Map<AttrValueHistorical,AttrValueHistorical> values3 = attrInfo3.getValuesHistorical();
assertTrue(values3.size() == 1);
valueInfo1 = new AttrValueHistorical(att, updateTime, null);
- assertTrue(values3.get(0).equals(valueInfo1));
+ assertTrue(values3.containsKey(valueInfo1));
// Check duplicate
AttrHistoricalMultiple attrInfo4 = attrInfo3.duplicate();
- ArrayList<AttrValueHistorical> values4 = attrInfo4.getValuesHistorical();
+ Map<AttrValueHistorical,AttrValueHistorical> values4 = attrInfo4.getValuesHistorical();
assertTrue(attrInfo4.getDeleteTime().compareTo(attrInfo3.getDeleteTime())==0);
assertEquals(values4.size(), values3.size());
--
Gitblit v1.10.0