You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by sa...@apache.org on 2016/04/05 00:29:03 UTC
incubator-geode git commit: fixed review comments
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 <sb...@pivotal.io>
Authored: Mon Apr 4 15:25:48 2016 -0700
Committer: Sai Boorlagadda <sb...@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);
}
}