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 <[email protected]> Authored: Mon Apr 4 15:25:48 2016 -0700 Committer: Sai Boorlagadda <[email protected]> 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); } }
