From da7eec061560d498abfb2eb261f59ca41e84a4bd Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Mon, 29 Apr 2013 17:07:14 +0000
Subject: [PATCH] CR-1620 Partial fix OPENDJ-888 Maintaining ds-sync-hist for a large group is inefficient

---
 opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java |    9 ++--
 opends/src/server/org/opends/server/replication/plugin/AttrHistorical.java                             |    7 ++-
 opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java                     |   61 +++++++++++++-----------------
 opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/AttrInfoTest.java       |   18 +++++----
 opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java                            |    5 +-
 opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java                       |   13 +++---
 6 files changed, 55 insertions(+), 58 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..af74a85 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;
@@ -93,8 +94,8 @@
    *
    * @return the List of ValueInfo
    */
-  public abstract ArrayList<AttrValueHistorical> getValuesHistorical();
-
+  public abstract Map<AttrValueHistorical,AttrValueHistorical>
+      getValuesHistorical();
 
   /**
    * Returns the last time when this attribute was deleted.
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..302977b 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java
@@ -27,7 +27,7 @@
  */
 package org.opends.server.replication.plugin;
 
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
 
 import org.opends.server.replication.common.ChangeNumber;
@@ -53,7 +53,7 @@
 {
    private ChangeNumber deleteTime, // last time when the attribute was deleted
                         lastUpdateTime; // last time the attribute was modified
-   private ArrayList<AttrValueHistorical> valuesHist;
+   private HashMap<AttrValueHistorical,AttrValueHistorical> valuesHist;
 
    /**
     * Create a new object from the provided informations.
@@ -63,12 +63,12 @@
     */
    public AttrHistoricalMultiple(ChangeNumber deleteTime,
        ChangeNumber updateTime,
-       ArrayList<AttrValueHistorical> valuesHist)
+       HashMap<AttrValueHistorical,AttrValueHistorical> valuesHist)
    {
      this.deleteTime = deleteTime;
      this.lastUpdateTime = updateTime;
      if (valuesHist == null)
-       this.valuesHist = new ArrayList<AttrValueHistorical>();
+       this.valuesHist = new HashMap<AttrValueHistorical,AttrValueHistorical>();
      else
        this.valuesHist = valuesHist;
    }
@@ -80,7 +80,7 @@
    {
      this.deleteTime = null;
      this.lastUpdateTime = null;
-     this.valuesHist = new ArrayList<AttrValueHistorical>();
+     this.valuesHist = new HashMap<AttrValueHistorical,AttrValueHistorical>();
    }
 
    /**
@@ -125,7 +125,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 = this.valuesHist.keySet().iterator();
      while (it.hasNext())
      {
        AttrValueHistorical info = it.next();
@@ -154,8 +154,7 @@
    protected void delete(AttributeValue val, ChangeNumber CN)
    {
      AttrValueHistorical info = new AttrValueHistorical(val, null, CN);
-     this.valuesHist.remove(info);
-     this.valuesHist.add(info);
+     this.valuesHist.put(info, info);
      if (CN.newer(lastUpdateTime))
      {
        lastUpdateTime = CN;
@@ -176,8 +175,7 @@
     for (AttributeValue val : attr)
     {
       AttrValueHistorical info = new AttrValueHistorical(val, null, CN);
-      this.valuesHist.remove(info);
-      this.valuesHist.add(info);
+      this.valuesHist.put(info, info);
       if (CN.newer(lastUpdateTime))
       {
         lastUpdateTime = CN;
@@ -196,8 +194,7 @@
    protected void add(AttributeValue addedValue, ChangeNumber CN)
    {
      AttrValueHistorical info = new AttrValueHistorical(addedValue, CN, null);
-     this.valuesHist.remove(info);
-     valuesHist.add(info);
+     this.valuesHist.put(info, info);
      if (CN.newer(lastUpdateTime))
      {
        lastUpdateTime = CN;
@@ -217,8 +214,7 @@
     for (AttributeValue val : attr)
     {
       AttrValueHistorical info = new AttrValueHistorical(val, CN, null);
-      this.valuesHist.remove(info);
-      valuesHist.add(info);
+      this.valuesHist.put(info, info);
       if (CN.newer(lastUpdateTime))
       {
         lastUpdateTime = CN;
@@ -231,7 +227,7 @@
    *
    * @return the list of historical informations for the values.
    */
-  public ArrayList<AttrValueHistorical> getValuesHistorical()
+  public HashMap<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
   {
     return valuesHist;
   }
@@ -417,10 +413,10 @@
       m.setModificationType(ModificationType.REPLACE);
       AttributeBuilder builder = new AttributeBuilder(modAttr, true);
 
-      for (Iterator<AttrValueHistorical> it = getValuesHistorical().iterator();
-        it.hasNext();)
+      Iterator<AttrValueHistorical> it = this.valuesHist.keySet().iterator();
+      while (it.hasNext())
       {
-        AttrValueHistorical valInfo; valInfo = it.next();
+        AttrValueHistorical valInfo = it.next();
 
         if (changeNumber.older(valInfo.getValueUpdateTime()))
         {
@@ -460,7 +456,8 @@
       /*
        * we are processing DELETE of some attribute values
        */
-      ArrayList<AttrValueHistorical> valuesInfo = getValuesHistorical();
+      HashMap<AttrValueHistorical,AttrValueHistorical> valuesInfo =
+          getValuesHistorical();
       AttributeBuilder builder = new AttributeBuilder(modAttr);
 
       for (AttributeValue val : modAttr)
@@ -471,11 +468,10 @@
         /* update historical information */
         AttrValueHistorical valInfo =
           new AttrValueHistorical(val, null, changeNumber);
-        int index = valuesInfo.indexOf(valInfo);
-        if (index != -1)
+        AttrValueHistorical oldValInfo = valuesInfo.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 +481,7 @@
           if (changeNumber.newerOrEquals(oldValInfo.getValueDeleteTime()) &&
               changeNumber.newerOrEquals(oldValInfo.getValueUpdateTime()))
           {
-            valuesInfo.remove(index);
-            valuesInfo.add(valInfo);
+            valuesInfo.put(valInfo, valInfo);
           }
           else if (oldValInfo.isUpdate())
           {
@@ -495,7 +490,7 @@
         }
         else
         {
-          valuesInfo.add(valInfo);
+          valuesInfo.put(valInfo, valInfo);
         }
 
         /* if the attribute value is not to be deleted
@@ -563,21 +558,21 @@
     AttributeBuilder builder = new AttributeBuilder(m.getAttribute());
     for (AttributeValue addVal : m.getAttribute())
     {
-      ArrayList<AttrValueHistorical> valuesInfo = getValuesHistorical();
+      HashMap<AttrValueHistorical,AttrValueHistorical> valuesInfo =
+          getValuesHistorical();
       AttrValueHistorical valInfo =
         new AttrValueHistorical(addVal, changeNumber, null);
-      int index = valuesInfo.indexOf(valInfo);
-      if (index == -1)
+      if (valuesInfo.containsKey(valInfo) == false)
       {
         /* this values does not exist yet
          * add it in the historical information
          * let the operation process normally
          */
-        valuesInfo.add(valInfo);
+        valuesInfo.put(valInfo, valInfo);
       }
       else
       {
-        AttrValueHistorical oldValueInfo = valuesInfo.get(index);
+        AttrValueHistorical oldValueInfo = valuesInfo.get(valInfo);
         if  (oldValueInfo.isUpdate())
         {
           /* if the value is already present
@@ -587,8 +582,7 @@
            */
           if (changeNumber.newer(oldValueInfo.getValueUpdateTime()))
           {
-            valuesInfo.remove(index);
-            valuesInfo.add(valInfo);
+            valuesInfo.put(valInfo, valInfo);
           }
           builder.remove(addVal);
         }
@@ -604,8 +598,7 @@
              * and add our more recent one
              * let the operation process
              */
-            valuesInfo.remove(index);
-            valuesInfo.add(valInfo);
+            valuesInfo.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..debaef9 100644
--- a/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
+++ b/opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java
@@ -27,7 +27,7 @@
  */
 package org.opends.server.replication.plugin;
 
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
 
 import org.opends.server.replication.common.ChangeNumber;
@@ -70,17 +70,18 @@
    * {@inheritDoc}
    */
   @Override
-  public ArrayList<AttrValueHistorical> getValuesHistorical()
+  public HashMap<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
   {
     if (addTime == null)
     {
-      return new ArrayList<AttrValueHistorical>();
+      return new HashMap<AttrValueHistorical,AttrValueHistorical>(0);
     }
     else
     {
-      ArrayList<AttrValueHistorical> values =
-        new ArrayList<AttrValueHistorical>();
-      values.add(new AttrValueHistorical(value, addTime, null));
+      HashMap<AttrValueHistorical,AttrValueHistorical> values =
+        new HashMap<AttrValueHistorical,AttrValueHistorical>(1);
+      AttrValueHistorical val = new AttrValueHistorical(value, addTime, null);
+      values.put(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..9a79ee5 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,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.HashMap;
+import java.util.Map;
 
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.replication.ReplicationTestCase;
@@ -89,15 +91,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 +108,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());
 
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
index 2c5e1bd..0640508 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/ModifyConflictTest.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;
 
@@ -1395,14 +1395,13 @@
     }
 
     /*
-     * Check that the encoding decoding of historical information
-     * works  by encoding decoding and checking that the result is the same
-     * as the initial value.
+     * Check that the historical information attributes produced are
+     * equivalent. (The attribute values may be in a different order.)
      */
     entry.removeAttribute(historicalAttrType);
     entry.addAttribute(hist.encodeAndPurge(), null);
     EntryHistorical hist2 = EntryHistorical.newInstanceFromEntry(entry);
-    assertEquals(hist2.encodeAndPurge().toString(), hist.encodeAndPurge().toString());
+    assertEquals(hist2.encodeAndPurge(), hist.encodeAndPurge());
 
     return mods;
   }

--
Gitblit v1.10.0