geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sai_boorlaga...@apache.org
Subject incubator-geode git commit: fixed review comments
Date Mon, 04 Apr 2016 22:29:03 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-1153 c964c27ac -> 8c6adb71f


fixed review comments


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/8c6adb71
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/8c6adb71
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/8c6adb71

Branch: refs/heads/feature/GEODE-1153
Commit: 8c6adb71faa3540549ee520af215496469df4f9d
Parents: c964c27
Author: Sai Boorlagadda <sboorlagadda@pivotal.io>
Authored: Mon Apr 4 15:25:48 2016 -0700
Committer: Sai Boorlagadda <sboorlagadda@pivotal.io>
Committed: Mon Apr 4 15:25:48 2016 -0700

----------------------------------------------------------------------
 .../PartitionedRegionRebalanceOp.java           | 60 +++++++----------
 .../rebalance/BucketOperatorImpl.java           | 25 +++----
 .../rebalance/BucketOperatorImplTest.java       | 71 +++++++-------------
 3 files changed, 58 insertions(+), 98 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8c6adb71/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionedRegionRebalanceOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionedRegionRebalanceOp.java
b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionedRegionRebalanceOp.java
index f36c74f..641d43d 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionedRegionRebalanceOp.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionedRegionRebalanceOp.java
@@ -415,7 +415,7 @@ public class PartitionedRegionRebalanceOp {
     }
     BucketOperator operator = simulate ? 
         new SimulatedBucketOperator() 
-        : new BucketOperatorImpl(leaderRegion, isRebalance, replaceOfflineData);
+        : new BucketOperatorImpl(this);
     BucketOperatorWrapper wrapper = new BucketOperatorWrapper(
         operator, rebalanceDetails, stats, leaderRegion);
     return wrapper;
@@ -511,17 +511,12 @@ public class PartitionedRegionRebalanceOp {
    *          the member on which to create the redundant bucket
    * @param bucketId
    *          the identifier of the bucket
-   * @param pr
-   *          the partitioned region which contains the bucket
-   * @param forRebalance
-   *          true if part of a rebalance operation
    * @return true if the redundant bucket was created
    */
-  public static boolean createRedundantBucketForRegion(
-      InternalDistributedMember target, int bucketId, PartitionedRegion pr,
-      boolean forRebalance, boolean replaceOfflineData) {
-    return pr.getRedundancyProvider().createBackupBucketOnMember(bucketId,
-        target, forRebalance, replaceOfflineData,null, true);
+  public boolean createRedundantBucketForRegion(
+      InternalDistributedMember target, int bucketId) {
+    return getLeaderRegion().getRedundancyProvider().createBackupBucketOnMember(bucketId,
+        target, isRebalance, replaceOfflineData,null, true);
   }
   
   /**
@@ -531,20 +526,18 @@ public class PartitionedRegionRebalanceOp {
    *          the member on which to create the redundant bucket
    * @param bucketId
    *          the identifier of the bucket
-   * @param pr
-   *          the partitioned region which contains the bucket
    * @return true if the redundant bucket was removed
    */
-  public static boolean removeRedundantBucketForRegion(
-      InternalDistributedMember target, int bucketId, PartitionedRegion pr) {
+  public boolean removeRedundantBucketForRegion(
+      InternalDistributedMember target, int bucketId) {
     boolean removed = false;
-    if (pr.getDistributionManager().getId().equals(target)) {
+    if (getLeaderRegion().getDistributionManager().getId().equals(target)) {
       // invoke directly on local member...
-      removed = pr.getDataStore().removeBucket(bucketId, false);
+      removed = getLeaderRegion().getDataStore().removeBucket(bucketId, false);
     }
     else {
       // send message to remote member...
-      RemoveBucketResponse response = RemoveBucketMessage.send(target, pr,
+      RemoveBucketResponse response = RemoveBucketMessage.send(target, getLeaderRegion(),
           bucketId, false);
       if (response != null) {
         removed = response.waitForResponse();
@@ -560,28 +553,23 @@ public class PartitionedRegionRebalanceOp {
    *          the member which should be primary
    * @param bucketId
    *          the identifier of the bucket
-   * @param pr
-   *          the partitioned region which contains the bucket
-   * @param forRebalance
-   *          true if part of a rebalance operation
    * @return true if the move was successful
    */
-  public static boolean movePrimaryBucketForRegion(
-      InternalDistributedMember target, int bucketId, PartitionedRegion pr,
-      boolean forRebalance) {
+  public boolean movePrimaryBucketForRegion(
+      InternalDistributedMember target, int bucketId) {
     boolean movedPrimary = false;
-    if (pr.getDistributionManager().getId().equals(target)) {
+    if (getLeaderRegion().getDistributionManager().getId().equals(target)) {
       // invoke directly on local member...
-      BucketAdvisor bucketAdvisor = pr.getRegionAdvisor().getBucketAdvisor(
+      BucketAdvisor bucketAdvisor = getLeaderRegion().getRegionAdvisor().getBucketAdvisor(
           bucketId);
       if (bucketAdvisor.isHosting()) {
-        movedPrimary = bucketAdvisor.becomePrimary(forRebalance);
+        movedPrimary = bucketAdvisor.becomePrimary(isRebalance);
       }
     }
     else {
       // send message to remote member...
       BecomePrimaryBucketResponse response = BecomePrimaryBucketMessage.send(
-          target, pr, bucketId, forRebalance);
+          target, getLeaderRegion(), bucketId, isRebalance);
       if (response != null) {
         movedPrimary = response.waitForResponse();
       }
@@ -598,20 +586,18 @@ public class PartitionedRegionRebalanceOp {
    *          member which should receive the bucket
    * @param bucketId
    *          the identifier of the bucket
-   * @param pr
-   *          the partitioned region which contains the bucket
    * @return true if the bucket was moved
    */
-  public static boolean moveBucketForRegion(InternalDistributedMember source,
-      InternalDistributedMember target, int bucketId, PartitionedRegion pr) {
+  public boolean moveBucketForRegion(InternalDistributedMember source,
+      InternalDistributedMember target, int bucketId) {
     boolean movedBucket = false;
-    if (pr.getDistributionManager().getId().equals(target)) {
+    if (getLeaderRegion().getDistributionManager().getId().equals(target)) {
       // invoke directly on local member...
-      movedBucket = pr.getDataStore().moveBucket(bucketId, source, false);
+      movedBucket = getLeaderRegion().getDataStore().moveBucket(bucketId, source, false);
     }
     else {
       // send message to remote member...
-      MoveBucketResponse response = MoveBucketMessage.send(target, pr,
+      MoveBucketResponse response = MoveBucketMessage.send(target, getLeaderRegion(),
           bucketId, source);
       if (response != null) {
         movedBucket = response.waitForResponse();
@@ -628,6 +614,10 @@ public class PartitionedRegionRebalanceOp {
            leaderRegion.getDataPolicy().withPersistence());
   }
   
+  public PartitionedRegion getLeaderRegion() {
+    return leaderRegion;
+  }
+  
   private class MembershipChangeListener implements MembershipListener {
 
     public void memberDeparted(InternalDistributedMember id, boolean crashed) {

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8c6adb71/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImpl.java
b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImpl.java
index b7c172b..62ef630 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImpl.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImpl.java
@@ -3,20 +3,15 @@ package com.gemstone.gemfire.internal.cache.partitioned.rebalance;
 import java.util.Map;
 
 import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember;
-import com.gemstone.gemfire.internal.cache.PartitionedRegion;
 import com.gemstone.gemfire.internal.cache.control.InternalResourceManager;
 import com.gemstone.gemfire.internal.cache.partitioned.PartitionedRegionRebalanceOp;
 
 public class BucketOperatorImpl implements BucketOperator {
   
-  private PartitionedRegion leaderRegion;
-  private boolean isRebalance;
-  private boolean replaceOfflineData;
+  private PartitionedRegionRebalanceOp rebalanceOp;
 
-  public BucketOperatorImpl(PartitionedRegion leaderRegion, boolean isRebalance, boolean
replaceOfflineData) {
-    this.leaderRegion = leaderRegion;
-    this.isRebalance = isRebalance;
-    this.replaceOfflineData = replaceOfflineData;
+  public BucketOperatorImpl(PartitionedRegionRebalanceOp rebalanceOp) {
+    this.rebalanceOp = rebalanceOp;
   }
 
   @Override
@@ -25,8 +20,8 @@ public class BucketOperatorImpl implements BucketOperator {
       Map<String, Long> colocatedRegionBytes) {
 
     InternalResourceManager.getResourceObserver().movingBucket(
-        leaderRegion, bucketId, source, target);
-    return PartitionedRegionRebalanceOp.moveBucketForRegion(source, target, bucketId, leaderRegion);
+        rebalanceOp.getLeaderRegion(), bucketId, source, target);
+    return rebalanceOp.moveBucketForRegion(source, target, bucketId);
   }
 
   @Override
@@ -34,8 +29,8 @@ public class BucketOperatorImpl implements BucketOperator {
       InternalDistributedMember target, int bucketId) {
 
     InternalResourceManager.getResourceObserver().movingPrimary(
-        leaderRegion, bucketId, source, target);
-    return PartitionedRegionRebalanceOp.movePrimaryBucketForRegion(target, bucketId, leaderRegion,
isRebalance); 
+        rebalanceOp.getLeaderRegion(), bucketId, source, target);
+    return rebalanceOp.movePrimaryBucketForRegion(target, bucketId); 
   }
 
   @Override
@@ -44,8 +39,7 @@ public class BucketOperatorImpl implements BucketOperator {
       Map<String, Long> colocatedRegionBytes, Completion completion) {
     boolean result = false;
     try {
-      result = PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember,
bucketId,
-        leaderRegion, isRebalance, replaceOfflineData);
+      result = rebalanceOp.createRedundantBucketForRegion(targetMember, bucketId);
     } finally {
       if(result) {
         completion.onSuccess();
@@ -63,7 +57,6 @@ public class BucketOperatorImpl implements BucketOperator {
   @Override
   public boolean removeBucket(InternalDistributedMember targetMember, int bucketId,
       Map<String, Long> colocatedRegionBytes) {
-    return PartitionedRegionRebalanceOp.removeRedundantBucketForRegion(targetMember, bucketId,
-        leaderRegion);
+    return rebalanceOp.removeRedundantBucketForRegion(targetMember, bucketId);
   }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/8c6adb71/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImplTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImplTest.java
b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImplTest.java
index d3643da..5d79d0e 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImplTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/rebalance/BucketOperatorImplTest.java
@@ -1,10 +1,11 @@
 package com.gemstone.gemfire.internal.cache.partitioned.rebalance;
 
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
@@ -15,12 +16,6 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.powermock.api.mockito.PowerMockito;
-import org.powermock.core.classloader.annotations.PowerMockIgnore;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember;
 import com.gemstone.gemfire.internal.cache.PartitionedRegion;
@@ -30,34 +25,32 @@ import com.gemstone.gemfire.internal.cache.partitioned.rebalance.BucketOperator.
 import com.gemstone.gemfire.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
-@RunWith(PowerMockRunner.class)
-@PowerMockIgnore("*.UnitTest")
-@PrepareForTest({ InternalResourceManager.class, PartitionedRegionRebalanceOp.class })
 public class BucketOperatorImplTest {
 
   private InternalResourceManager.ResourceObserver resourceObserver;
 
   private BucketOperatorImpl operator;
 
-  @Mock
   private PartitionedRegion region;
-  private boolean isRebalance = true;
-  private boolean replaceOfflineData = true;
+  private PartitionedRegionRebalanceOp rebalanceOp;
+  private Completion completion;
 
   private Map<String, Long> colocatedRegionBytes = new HashMap<String, Long>();
   private int bucketId = 1;
   private InternalDistributedMember sourceMember, targetMember;
-  @Mock
-  Completion completion;
 
   @Before
   public void setup() throws UnknownHostException {
+    region = mock(PartitionedRegion.class);
+    rebalanceOp = mock(PartitionedRegionRebalanceOp.class);
+    completion = mock(Completion.class);
+    
     resourceObserver = spy(new InternalResourceManager.ResourceObserverAdapter());
+    InternalResourceManager.setResourceObserver(resourceObserver);
+    
+    doReturn(region).when(rebalanceOp).getLeaderRegion();
 
-    PowerMockito.mockStatic(InternalResourceManager.class);
-    when(InternalResourceManager.getResourceObserver()).thenReturn(resourceObserver);
-
-    operator = new BucketOperatorImpl(region, isRebalance, replaceOfflineData);
+    operator = new BucketOperatorImpl(rebalanceOp);
 
     sourceMember = new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 1);
     targetMember = new InternalDistributedMember(InetAddress.getByName("127.0.0.2"), 1);
@@ -70,76 +63,60 @@ public class BucketOperatorImplTest {
 
   @Test
   public void moveBucketShouldDelegateToParRegRebalanceOpMoveBucketForRegion() throws UnknownHostException
{
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.moveBucketForRegion(sourceMember, targetMember, bucketId,
region)).thenReturn(true);
+    doReturn(true).when(rebalanceOp).moveBucketForRegion(sourceMember, targetMember, bucketId);
 
     operator.moveBucket(sourceMember, targetMember, bucketId, colocatedRegionBytes);
 
     verify(resourceObserver, times(1)).movingBucket(region, bucketId, sourceMember, targetMember);
-
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.moveBucketForRegion(sourceMember, targetMember, bucketId,
region);
+    verify(rebalanceOp, times(1)).moveBucketForRegion(sourceMember, targetMember, bucketId);
   }
 
   @Test
   public void movePrimaryShouldDelegateToParRegRebalanceOpMovePrimaryBucketForRegion() throws
UnknownHostException {
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.movePrimaryBucketForRegion(targetMember, bucketId,
region, isRebalance)).thenReturn(true);
+    doReturn(true).when(rebalanceOp).movePrimaryBucketForRegion(targetMember, bucketId);
 
     operator.movePrimary(sourceMember, targetMember, bucketId);
 
     verify(resourceObserver, times(1)).movingPrimary(region, bucketId, sourceMember, targetMember);
-
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.movePrimaryBucketForRegion(targetMember, bucketId, region,
isRebalance);
+    verify(rebalanceOp, times(1)).movePrimaryBucketForRegion(targetMember, bucketId);
   }
 
   @Test
   public void createBucketShouldDelegateToParRegRebalanceOpCreateRedundantBucketForRegion()
throws UnknownHostException {
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId,
region, isRebalance, replaceOfflineData)).thenReturn(true);
+    doReturn(true).when(rebalanceOp).createRedundantBucketForRegion(targetMember, bucketId);
 
     operator.createRedundantBucket(targetMember, bucketId, colocatedRegionBytes, completion);
 
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId, region,
isRebalance, replaceOfflineData);
+    verify(rebalanceOp, times(1)).createRedundantBucketForRegion(targetMember, bucketId);
   }
 
   @Test
   public void createBucketShouldInvokeOnSuccessIfCreateBucketSucceeds() {
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId,
region, isRebalance, replaceOfflineData)).thenReturn(true);
+    doReturn(true).when(rebalanceOp).createRedundantBucketForRegion(targetMember, bucketId);
 
     operator.createRedundantBucket(targetMember, bucketId, colocatedRegionBytes, completion);
 
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId, region,
isRebalance, replaceOfflineData);
-
+    verify(rebalanceOp, times(1)).createRedundantBucketForRegion(targetMember, bucketId);
     verify(completion, times(1)).onSuccess();
   }
 
   @Test
   public void createBucketShouldInvokeOnFailureIfCreateBucketFails() {
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId,
region, isRebalance, replaceOfflineData)).thenReturn(false); //return false for create fail
+    doReturn(false).when(rebalanceOp).createRedundantBucketForRegion(targetMember, bucketId);
//return false for create fail
 
     operator.createRedundantBucket(targetMember, bucketId, colocatedRegionBytes, completion);
 
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.createRedundantBucketForRegion(targetMember, bucketId, region,
isRebalance, replaceOfflineData);
-
+    verify(rebalanceOp, times(1)).createRedundantBucketForRegion(targetMember, bucketId);
     verify(completion, times(1)).onFailure();
   }
 
   @Test
   public void removeBucketShouldDelegateToParRegRebalanceOpRemoveRedundantBucketForRegion()
{
-    PowerMockito.mockStatic(PartitionedRegionRebalanceOp.class);
-    when(PartitionedRegionRebalanceOp.removeRedundantBucketForRegion(targetMember, bucketId,
region)).thenReturn(true);
+    doReturn(true).when(rebalanceOp).removeRedundantBucketForRegion(targetMember, bucketId);
 
     operator.removeBucket(targetMember, bucketId, colocatedRegionBytes);
 
-    PowerMockito.verifyStatic(times(1));
-    PartitionedRegionRebalanceOp.removeRedundantBucketForRegion(targetMember, bucketId, region);
+    verify(rebalanceOp, times(1)).removeRedundantBucketForRegion(targetMember, bucketId);
   }
 
 }


Mime
View raw message