[geode] branch develop updated: GEODE-10410: Fix bucket lost during rebalance (#7857)
This is an automated email from the ASF dual-hosted git repository.
alberto pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 67ebd727be GEODE-10410: Fix bucket lost during rebalance (#7857)
67ebd727be is described below
commit 67ebd727bef5c613bfe2aaf4258a5472ac433978
Author: WeijieEST <108109958+weijie...@users.noreply.github.com>
AuthorDate: Wed Sep 21 01:37:57 2022 +0800
GEODE-10410: Fix bucket lost during rebalance (#7857)
* GEODE-10410: Fix bucket lost during rebalance
* improve test case name
* improve test case comments and test case names
---
.../partitioned/rebalance/model/MemberRollup.java | 31
.../PartitionedRegionLoadModelJUnitTest.java | 82 +-
2 files changed, 79 insertions(+), 34 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/MemberRollup.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/MemberRollup.java
index be9c4df2ed..6078cf028d 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/MemberRollup.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/MemberRollup.java
@@ -131,26 +131,23 @@ class MemberRollup extends Member {
@Override
public RefusalReason willAcceptBucket(Bucket bucket, Member source, boolean
checkIPAddress) {
-RefusalReason reason = super.willAcceptBucket(bucket, source,
checkIPAddress);
-if (reason.willAccept()) {
- BucketRollup bucketRollup = (BucketRollup) bucket;
- MemberRollup sourceRollup = (MemberRollup) source;
- for (Map.Entry entry : getColocatedMembers().entrySet())
{
-String region = entry.getKey();
-Member member = entry.getValue();
-Bucket colocatedBucket =
bucketRollup.getColocatedBuckets().get(region);
-Member colocatedSource =
-sourceRollup == null ? null :
sourceRollup.getColocatedMembers().get(region);
-if (colocatedBucket != null) {
- reason = member.willAcceptBucket(colocatedBucket, colocatedSource,
checkIPAddress);
- if (!reason.willAccept()) {
-return reason;
- }
+RefusalReason reason;
+BucketRollup bucketRollup = (BucketRollup) bucket;
+MemberRollup sourceRollup = (MemberRollup) source;
+for (Map.Entry entry : getColocatedMembers().entrySet()) {
+ String region = entry.getKey();
+ Member member = entry.getValue();
+ Bucket colocatedBucket = bucketRollup.getColocatedBuckets().get(region);
+ Member colocatedSource =
+ sourceRollup == null ? null :
sourceRollup.getColocatedMembers().get(region);
+ if (colocatedBucket != null) {
+reason = member.willAcceptBucket(colocatedBucket, colocatedSource,
checkIPAddress);
+if (!reason.willAccept()) {
+ return reason;
}
}
- return RefusalReason.NONE;
}
-return reason;
+return RefusalReason.NONE;
}
Map getColocatedMembers() {
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java
index 26b2b98b8e..5423b08a28 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java
@@ -66,7 +66,7 @@ public class PartitionedRegionLoadModelJUnitTest {
private static final int MAX_MOVES = 5000;
private static final boolean DEBUG = true;
-
+ private static final long MB = 1024 * 1024;
private MyBucketOperator bucketOperator;
private final PartitionedRegion partitionedRegion =
mock(PartitionedRegion.class);
final ClusterDistributionManager clusterDistributionManager =
@@ -443,7 +443,8 @@ public class PartitionedRegionLoadModelJUnitTest {
* lmm, it will prevent a bucket move
*/
@Test
- public void testColocationEnforceLocalMaxMemory() throws
UnknownHostException {
+ public void testColocationTwoNonEvictionRegionsEnforceLocalMaxMemory()
+ throws UnknownHostException {
PartitionedRegionLoadModel model = new
PartitionedRegionLoadModel(bucketOperator, 1, 4,
getAddressComparor(false), Collections.emptySet(), partitionedRegion);
@@ -452,25 +453,27 @@ public class PartitionedRegionLoadModelJUnitTest {
InternalDistributedMember member2 =
new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 2);
-// Create some buckets with low redundancy on member 1
+// Create some buckets with low redundancy on member 1 and enough lmm for
re
[geode] branch develop updated: GEODE-10395 remove locks from List if dlock.acquireTryLocks return false (#7846)
This is an automated email from the ASF dual-hosted git repository.
mkevo pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 4cb75ae484 GEODE-10395 remove locks from List if dlock.acquireTryLocks
return false (#7846)
4cb75ae484 is described below
commit 4cb75ae4848250606db2f4b14300601755586192
Author: Mario Kevo <48509719+mk...@users.noreply.github.com>
AuthorDate: Tue Sep 20 19:04:08 2022 +0200
GEODE-10395 remove locks from List if dlock.acquireTryLocks return false
(#7846)
---
.../internal/cache/locks/TXLockServiceImpl.java| 26 ++--
.../internal/StartupMessageJUnitTest.java | 4 +-
.../cache/locks/TXLockServiceImplTest.java | 71 ++
3 files changed, 95 insertions(+), 6 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLockServiceImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLockServiceImpl.java
index f7e7aebe0f..44b9c9f440 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLockServiceImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLockServiceImpl.java
@@ -22,6 +22,7 @@ import java.util.Set;
import org.apache.logging.log4j.Logger;
+import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.CommitConflictException;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
@@ -53,7 +54,7 @@ public class TXLockServiceImpl extends TXLockService {
/**
* List of active txLockIds
*/
- protected List txLockIdList = new ArrayList();
+ protected final List txLockIdList = new ArrayList<>();
/**
* True if grantor recovery is in progress; used to keep
release from waiting for
@@ -70,6 +71,14 @@ public class TXLockServiceImpl extends TXLockService {
/** The distributed system for cancellation checks. */
private final InternalDistributedSystem system;
+ @VisibleForTesting
+ TXLockServiceImpl(InternalDistributedSystem sys,
StoppableReentrantReadWriteLock recoveryLock,
+ DLockService dlock) {
+system = sys;
+this.recoveryLock = recoveryLock;
+this.dlock = dlock;
+ }
+
TXLockServiceImpl(String name, InternalDistributedSystem sys) {
if (sys == null) {
throw new IllegalStateException(
@@ -129,10 +138,16 @@ public class TXLockServiceImpl extends TXLockService {
if (gotLocks) { // ...otherwise race can occur between tryLocks and
readLock
acquireRecoveryReadLock();
} else if (keyIfFail[0] != null) {
+synchronized (txLockIdList) {
+ txLockIdList.remove(txLockId);
+}
throw new CommitConflictException(
String.format("Concurrent transaction commit detected %s",
keyIfFail[0]));
} else {
+synchronized (txLockIdList) {
+ txLockIdList.remove(txLockId);
+}
throw new CommitConflictException(
String.format("Failed to request try locks from grantor: %s",
dlock.getLockGrantorId()));
@@ -225,9 +240,7 @@ public class TXLockServiceImpl extends TXLockService {
txLockId));
}
- dlock.releaseTryLocks(txLockId, () -> {
-return recovering;
- });
+ dlock.releaseTryLocks(txLockId, () -> recovering);
txLockIdList.remove(txLockId);
releaseRecoveryReadLock();
@@ -277,4 +290,9 @@ public class TXLockServiceImpl extends TXLockService {
dlock.destroyAndRemove();
}
+ @VisibleForTesting
+ public int getTxLockIdList() {
+return this.txLockIdList.size();
+ }
+
}
diff --git
a/geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageJUnitTest.java
index d017b96ad9..b51453a3a4 100644
---
a/geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageJUnitTest.java
@@ -95,7 +95,7 @@ public class StartupMessageJUnitTest {
startupMessage.process(distributionManager);
assertThat(
-startupMessage.getProcessorType() ==
OperationExecutors.WAITING_POOL_EXECUTOR);
+
startupMessage.getProcessorType()).isEqualTo(OperationExecutors.WAITING_POOL_EXECUTOR);
}
@Test
@@ -111,6 +111,6 @@ public class StartupMessageJUnitTest {
assertThat(
startupResponseMessage
-.getProcessorType() == OperationExecutors.WAITING_POOL_EXECUTOR);
+
.getProcessorType()).isEqualTo(OperationExecutors.WAITING_POOL_EXECUTOR);
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/locks/TXLockServiceImplTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/locks/T
