Repository: reef
Updated Branches:
  refs/heads/master 0480d0ebd -> 314928725


[REEF-1322] Allow Communication Group to be removed from IGroupCommDriver

Adding remove API in GroupCommunicationDriver
Add test cases

JIRA: [REEF-1322](https://issues.apache.org/jira/browse/REEF-1322)
This closes #949


Project: http://git-wip-us.apache.org/repos/asf/reef/repo
Commit: http://git-wip-us.apache.org/repos/asf/reef/commit/31492872
Tree: http://git-wip-us.apache.org/repos/asf/reef/tree/31492872
Diff: http://git-wip-us.apache.org/repos/asf/reef/diff/31492872

Branch: refs/heads/master
Commit: 31492872577c7342f12ad421b5430daeede47302
Parents: 0480d0e
Author: Julia Wang <[email protected]>
Authored: Tue Apr 12 14:58:07 2016 -0700
Committer: Andrew Chung <[email protected]>
Committed: Wed Apr 13 15:57:23 2016 -0700

----------------------------------------------------------------------
 .../GroupCommunicationTests.cs                  | 57 ++++++++++++++++++++
 .../Group/Driver/IGroupCommDriver.cs            |  7 +++
 .../Group/Driver/Impl/GroupCommDriver.cs        | 44 +++++++++++----
 3 files changed, 97 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/reef/blob/31492872/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/GroupCommunicationTests.cs
----------------------------------------------------------------------
diff --git 
a/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/GroupCommunicationTests.cs
 
b/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/GroupCommunicationTests.cs
index d242531..175ce05 100644
--- 
a/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/GroupCommunicationTests.cs
+++ 
b/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/GroupCommunicationTests.cs
@@ -160,6 +160,63 @@ namespace Org.Apache.REEF.Network.Tests.GroupCommunication
         }
 
         /// <summary>
+        /// Test create a new group then remove it from the GroupDriver
+        /// </summary>
+        [Fact]
+        public void TestRemoveCommunicationGroup()
+        {
+            const string groupName = "group1";
+            const string groupName2 = "group2";
+            const string masterTaskId = "task0";
+            const string driverId = "Driver Id";
+            const int numTasks = 3;
+            const int fanOut = 2;
+
+            var groupCommunicationDriver = 
GetInstanceOfGroupCommDriver(driverId, masterTaskId, groupName, fanOut, 
numTasks);
+            var group = 
groupCommunicationDriver.NewCommunicationGroup(groupName2, 5);
+            Assert.NotNull(group);
+            groupCommunicationDriver.RemoveCommunicationGroup(groupName2);
+
+            Action remove = () => 
groupCommunicationDriver.RemoveCommunicationGroup(groupName2);
+            Assert.Throws<ArgumentException>(remove);
+        }
+
+        /// <summary>
+        /// Test remove default group
+        /// </summary>
+        [Fact]
+        public void TestRemoveDefaultGroup()
+        {
+            const string groupName = "group1";
+            const string masterTaskId = "task0";
+            const string driverId = "Driver Id";
+            const int numTasks = 3;
+            const int fanOut = 2;
+
+            var groupCommunicationDriver = 
GetInstanceOfGroupCommDriver(driverId, masterTaskId, groupName, fanOut, 
numTasks);
+            var group = groupCommunicationDriver.DefaultGroup;
+            Assert.NotNull(group);
+            groupCommunicationDriver.RemoveCommunicationGroup(groupName);
+
+            Action remove = () => 
groupCommunicationDriver.RemoveCommunicationGroup(groupName);
+            Assert.Throws<ArgumentException>(remove);
+        }
+
+        [Fact]
+        public void TestRemoveNoExistGroup()
+        {
+            const string groupName = "group1";
+            const string masterTaskId = "task0";
+            const string driverId = "Driver Id";
+            const int numTasks = 3;
+            const int fanOut = 2;
+
+            var groupCommunicationDriver = 
GetInstanceOfGroupCommDriver(driverId, masterTaskId, groupName, fanOut, 
numTasks);
+            Action remove = () => 
groupCommunicationDriver.RemoveCommunicationGroup(groupName);
+            Assert.Throws<ArgumentException>(remove);
+        }
+
+        /// <summary>
         /// This is to test operator injection in CommunicationGroupClient 
with int[] as message type
         /// </summary>
         [Fact]

http://git-wip-us.apache.org/repos/asf/reef/blob/31492872/lang/cs/Org.Apache.REEF.Network/Group/Driver/IGroupCommDriver.cs
----------------------------------------------------------------------
diff --git a/lang/cs/Org.Apache.REEF.Network/Group/Driver/IGroupCommDriver.cs 
b/lang/cs/Org.Apache.REEF.Network/Group/Driver/IGroupCommDriver.cs
index a8f29dc..acf640b 100644
--- a/lang/cs/Org.Apache.REEF.Network/Group/Driver/IGroupCommDriver.cs
+++ b/lang/cs/Org.Apache.REEF.Network/Group/Driver/IGroupCommDriver.cs
@@ -42,6 +42,13 @@ namespace Org.Apache.REEF.Network.Group.Driver
         ICommunicationGroupDriver NewCommunicationGroup(string groupName, int 
numTasks);
 
         /// <summary>
+        /// remove a communication group
+        /// Throw ArgumentException if the group does not exist
+        /// </summary>
+        /// <param name="groupName"></param>
+        void RemoveCommunicationGroup(string groupName);
+
+        /// <summary>
         /// Generates context configuration with a unique identifier.
         /// </summary>
         /// <returns>The configured context configuration</returns>

http://git-wip-us.apache.org/repos/asf/reef/blob/31492872/lang/cs/Org.Apache.REEF.Network/Group/Driver/Impl/GroupCommDriver.cs
----------------------------------------------------------------------
diff --git 
a/lang/cs/Org.Apache.REEF.Network/Group/Driver/Impl/GroupCommDriver.cs 
b/lang/cs/Org.Apache.REEF.Network/Group/Driver/Impl/GroupCommDriver.cs
index 1f0752d..8426451 100644
--- a/lang/cs/Org.Apache.REEF.Network/Group/Driver/Impl/GroupCommDriver.cs
+++ b/lang/cs/Org.Apache.REEF.Network/Group/Driver/Impl/GroupCommDriver.cs
@@ -33,6 +33,7 @@ using Org.Apache.REEF.Tang.Implementations.Tang;
 using Org.Apache.REEF.Tang.Interface;
 using Org.Apache.REEF.Tang.Util;
 using Org.Apache.REEF.Utilities.Logging;
+using Org.Apache.REEF.Utilities.Diagnostics;
 using ContextConfiguration = 
Org.Apache.REEF.Common.Context.ContextConfiguration;
 
 namespace Org.Apache.REEF.Network.Group.Driver.Impl
@@ -46,7 +47,7 @@ namespace Org.Apache.REEF.Network.Group.Driver.Impl
         private const string MasterTaskContextName = "MasterTaskContext";
         private const string SlaveTaskContextName = "SlaveTaskContext";
 
-        private static Logger LOGGER = 
Logger.GetLogger(typeof(GroupCommDriver));
+        private static Logger Logger = 
Logger.GetLogger(typeof(GroupCommDriver));
 
         private readonly string _driverId;
         private readonly string _nameServerAddr;
@@ -113,8 +114,8 @@ namespace Org.Apache.REEF.Network.Group.Driver.Impl
                     {
                         NewCommunicationGroup(_defaultGroupName, 
_defaultGroupNumberOfTasks);
                     }
+                    return _commGroups[_defaultGroupName];
                 }
-                return _commGroups[_defaultGroupName];
             }
         }
 
@@ -128,22 +129,43 @@ namespace Org.Apache.REEF.Network.Group.Driver.Impl
         {
             if (string.IsNullOrEmpty(groupName))
             {
-                throw new ArgumentNullException("groupName");
+                Exceptions.Throw(new ArgumentNullException("groupName"), 
Logger);
             }
 
             if (numTasks < 1)
             {
-                throw new ArgumentException("NumTasks must be greater than 0");
+               Exceptions.Throw(new ArgumentException("NumTasks must be 
greater than 0"), Logger);
             }
-            
-            if (_commGroups.ContainsKey(groupName))
+
+            lock (_groupsLock)
             {
-                throw new ArgumentException("Group Name already registered 
with GroupCommunicationDriver");
+                if (_commGroups.ContainsKey(groupName))
+                {
+                    Exceptions.Throw(new ArgumentException("Group Name already 
registered with GroupCommunicationDriver"), Logger);
+                }
+
+                var commGroup = new CommunicationGroupDriver(groupName, 
_driverId, numTasks, _fanOut, _configSerializer);
+                _commGroups[groupName] = commGroup;
+                return commGroup;
             }
+        }
 
-            var commGroup = new CommunicationGroupDriver(groupName, _driverId, 
numTasks, _fanOut, _configSerializer);
-            _commGroups[groupName] = commGroup;
-            return commGroup;
+        /// <summary>
+        /// Remove a group from the GroupCommDriver
+        /// Throw ArgumentException if the group does not exist
+        /// </summary>
+        /// <param name="groupName"></param>
+        public void RemoveCommunicationGroup(string groupName)
+        {
+            lock (_groupsLock)
+            {
+                if (!_commGroups.ContainsKey(groupName))
+                {
+                    Exceptions.Throw(new ArgumentException("Group Name is not 
registered with GroupCommunicationDriver"), Logger);
+                }
+
+                _commGroups.Remove(groupName);
+            }
         }
 
         /// <summary>
@@ -252,7 +274,7 @@ namespace Org.Apache.REEF.Network.Group.Driver.Impl
             string[] parts = activeContext.Id.Split('-');
             if (parts.Length != 2)
             {
-                throw new ArgumentException("Invalid id in active context");
+                Exceptions.Throw(new ArgumentException("Invalid id in active 
context"), Logger);
             }
 
             return int.Parse(parts[1], CultureInfo.InvariantCulture);

Reply via email to