Distributed group store using eventual consistent map abstraction

Change-Id: I618a0f6fa80e0e25285d7a2026032f09ba90aa70
diff --git a/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
index 2d8f81c..6601cbf 100644
--- a/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
+++ b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
@@ -15,7 +15,13 @@
  */
 package org.onosproject.net.group.impl;
 
-import com.google.common.collect.Sets;
+import static org.slf4j.LoggerFactory.getLogger;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -48,12 +54,7 @@
 import org.onosproject.net.provider.AbstractProviderService;
 import org.slf4j.Logger;
 
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.Set;
-
-import static org.slf4j.LoggerFactory.getLogger;
+import com.google.common.collect.Sets;
 
 /**
  * Provides implementation of the group service APIs.
@@ -103,6 +104,7 @@
      */
     @Override
     public void addGroup(GroupDescription groupDesc) {
+        log.trace("In addGroup API");
         store.storeGroupDescription(groupDesc);
     }
 
@@ -121,6 +123,7 @@
      */
     @Override
     public Group getGroup(DeviceId deviceId, GroupKey appCookie) {
+        log.trace("In getGroup API");
         return store.getGroup(deviceId, appCookie);
     }
 
@@ -142,6 +145,7 @@
                            GroupBuckets buckets,
                            GroupKey newCookie,
                            ApplicationId appId) {
+        log.trace("In addBucketsToGroup API");
         store.updateGroupDescription(deviceId,
                                      oldCookie,
                                      UpdateType.ADD,
@@ -167,6 +171,7 @@
                                 GroupBuckets buckets,
                                 GroupKey newCookie,
                                 ApplicationId appId) {
+        log.trace("In removeBucketsFromGroup API");
         store.updateGroupDescription(deviceId,
                                      oldCookie,
                                      UpdateType.REMOVE,
@@ -188,6 +193,7 @@
     public void removeGroup(DeviceId deviceId,
                             GroupKey appCookie,
                             ApplicationId appId) {
+        log.trace("In removeGroup API");
         store.deleteGroupDescription(deviceId, appCookie);
     }
 
@@ -202,11 +208,13 @@
     @Override
     public Iterable<Group> getGroups(DeviceId deviceId,
                                      ApplicationId appId) {
+        log.trace("In getGroups API");
         return store.getGroups(deviceId);
     }
 
     @Override
     public Iterable<Group> getGroups(DeviceId deviceId) {
+        log.trace("In getGroups API");
         return store.getGroups(deviceId);
     }
 
@@ -217,6 +225,7 @@
      */
     @Override
     public void addListener(GroupListener listener) {
+        log.trace("In addListener API");
         listenerRegistry.addListener(listener);
     }
 
@@ -227,6 +236,7 @@
      */
     @Override
     public void removeListener(GroupListener listener) {
+        log.trace("In removeListener API");
         listenerRegistry.removeListener(listener);
     }
 
@@ -364,36 +374,52 @@
             Set<Group> extraneousStoredEntries =
                     Sets.newHashSet(store.getExtraneousGroups(deviceId));
 
-            log.trace("Displaying all southboundGroupEntries for device {}", deviceId);
+            log.trace("Displaying all ({}) southboundGroupEntries for device {}",
+                      southboundGroupEntries.size(),
+                      deviceId);
             for (Iterator<Group> it = southboundGroupEntries.iterator(); it.hasNext();) {
                 Group group = it.next();
                 log.trace("Group {} in device {}", group, deviceId);
             }
 
-            log.trace("Displaying all stored group entries for device {}", deviceId);
-            for (Iterator<Group> it = storedGroupEntries.iterator(); it.hasNext();) {
-                Group group = it.next();
+            log.trace("Displaying all ({}) stored group entries for device {}",
+                      storedGroupEntries.size(),
+                      deviceId);
+            for (Iterator<Group> it1 = storedGroupEntries.iterator(); it1.hasNext();) {
+                Group group = it1.next();
                 log.trace("Stored Group {} for device {}", group, deviceId);
             }
 
-            for (Iterator<Group> it = southboundGroupEntries.iterator(); it.hasNext();) {
-                Group group = it.next();
+            for (Iterator<Group> it2 = southboundGroupEntries.iterator(); it2.hasNext();) {
+                Group group = it2.next();
                 if (storedGroupEntries.remove(group)) {
                     // we both have the group, let's update some info then.
                     log.trace("Group AUDIT: group {} exists "
                             + "in both planes for device {}",
                             group.id(), deviceId);
                     groupAdded(group);
-                    it.remove();
+                    it2.remove();
                 }
             }
             for (Group group : southboundGroupEntries) {
-                // there are groups in the switch that aren't in the store
-                log.trace("Group AUDIT: extraneous group {} exists "
-                        + "in data plane for device {}",
-                        group.id(), deviceId);
-                extraneousStoredEntries.remove(group);
-                extraneousGroup(group);
+                if (store.getGroup(group.deviceId(), group.id()) != null) {
+                    // There is a group existing with the same id
+                    // It is possible that group update is
+                    // in progress while we got a stale info from switch
+                    if (!storedGroupEntries.remove(store.getGroup(
+                                 group.deviceId(), group.id()))) {
+                        log.warn("Group AUDIT: Inconsistent state:"
+                                + "Group exists in ID based table while "
+                                + "not present in key based table");
+                    }
+                } else {
+                    // there are groups in the switch that aren't in the store
+                    log.trace("Group AUDIT: extraneous group {} exists "
+                            + "in data plane for device {}",
+                            group.id(), deviceId);
+                    extraneousStoredEntries.remove(group);
+                    extraneousGroup(group);
+                }
             }
             for (Group group : storedGroupEntries) {
                 // there are groups in the store that aren't in the switch
diff --git a/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java b/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
index 2fe5e4b..fa3e987 100644
--- a/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
@@ -15,6 +15,12 @@
  */
 package org.onosproject.net.group.impl;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -38,6 +44,7 @@
 import org.onosproject.net.group.DefaultGroup;
 import org.onosproject.net.group.DefaultGroupBucket;
 import org.onosproject.net.group.DefaultGroupDescription;
+import org.onosproject.net.group.DefaultGroupKey;
 import org.onosproject.net.group.Group;
 import org.onosproject.net.group.GroupBucket;
 import org.onosproject.net.group.GroupBuckets;
@@ -58,8 +65,6 @@
 
 import com.google.common.collect.Iterables;
 
-import static org.junit.Assert.*;
-
 /**
  * Test codifying the group service & group provider service contracts.
  */
@@ -108,31 +113,6 @@
         mgr.eventDispatcher = null;
     }
 
-    private class TestGroupKey implements GroupKey {
-        private String groupId;
-
-        public TestGroupKey(String id) {
-            this.groupId = id;
-        }
-
-        public String id() {
-            return this.groupId;
-        }
-
-        @Override
-        public int hashCode() {
-            return groupId.hashCode();
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (obj instanceof TestGroupKey) {
-                return this.groupId.equals(((TestGroupKey) obj).id());
-            }
-            return false;
-        }
-    }
-
     /**
      * Tests group service north bound and south bound interfaces.
      * The following operations are tested:
@@ -177,7 +157,7 @@
                                PortNumber.portNumber(32)};
         PortNumber[] ports2 = {PortNumber.portNumber(41),
                                PortNumber.portNumber(42)};
-        TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+        GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes());
         List<GroupBucket> buckets = new ArrayList<GroupBucket>();
         List<PortNumber> outPorts = new ArrayList<PortNumber>();
         outPorts.addAll(Arrays.asList(ports1));
@@ -224,7 +204,7 @@
         providerService.pushGroupMetrics(DID, groupEntries);
         // First group metrics would trigger the device audit completion
         // post which all pending group requests are also executed.
-        TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+        GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes());
         Group createdGroup = groupService.getGroup(DID, key);
         int createdGroupId = createdGroup.id().id();
         assertNotEquals(gId1.id(), createdGroupId);
@@ -256,7 +236,7 @@
                                             0);
         List<Group> groupEntries = Arrays.asList(group1, group2);
         providerService.pushGroupMetrics(DID, groupEntries);
-        TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+        GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes());
         Group createdGroup = groupService.getGroup(DID, key);
         List<GroupOperation> expectedGroupOps = Arrays.asList(
                 GroupOperation.createDeleteGroupOperation(gId1,
@@ -271,7 +251,7 @@
 
     // Test AUDIT with confirmed groups
     private void testAuditWithConfirmedGroups() {
-        TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+        GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes());
         Group createdGroup = groupService.getGroup(DID, key);
         createdGroup = new DefaultGroup(createdGroup.id(),
                                         DID,
@@ -284,9 +264,9 @@
 
     // Test group add bucket operations
     private void testAddBuckets() {
-        TestGroupKey addKey = new TestGroupKey("group1AddBuckets");
+        GroupKey addKey = new DefaultGroupKey("group1AddBuckets".getBytes());
 
-        TestGroupKey prevKey = new TestGroupKey("group1BeforeAudit");
+        GroupKey prevKey = new DefaultGroupKey("group1BeforeAudit".getBytes());
         Group createdGroup = groupService.getGroup(DID, prevKey);
         List<GroupBucket> buckets = new ArrayList<GroupBucket>();
         buckets.addAll(createdGroup.buckets().buckets());
@@ -328,9 +308,9 @@
 
     // Test group remove bucket operations
     private void testRemoveBuckets() {
-        TestGroupKey removeKey = new TestGroupKey("group1RemoveBuckets");
+        GroupKey removeKey = new DefaultGroupKey("group1RemoveBuckets".getBytes());
 
-        TestGroupKey prevKey = new TestGroupKey("group1AddBuckets");
+        GroupKey prevKey = new DefaultGroupKey("group1AddBuckets".getBytes());
         Group createdGroup = groupService.getGroup(DID, prevKey);
         List<GroupBucket> buckets = new ArrayList<GroupBucket>();
         buckets.addAll(createdGroup.buckets().buckets());
@@ -372,7 +352,7 @@
 
     // Test group remove operations
     private void testRemoveGroup() {
-        TestGroupKey currKey = new TestGroupKey("group1RemoveBuckets");
+        GroupKey currKey = new DefaultGroupKey("group1RemoveBuckets".getBytes());
         Group existingGroup = groupService.getGroup(DID, currKey);
         groupService.removeGroup(DID, currKey, appId);
         List<GroupOperation> expectedGroupOps = Arrays.asList(
@@ -397,7 +377,7 @@
         PortNumber[] ports2 = {PortNumber.portNumber(41),
                 PortNumber.portNumber(42)};
         // Test Group creation before AUDIT process
-        TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+        GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes());
         List<GroupBucket> buckets = new ArrayList<GroupBucket>();
         List<PortNumber> outPorts = new ArrayList<PortNumber>();
         outPorts.addAll(Arrays.asList(ports1));