This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-6630
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-6630 by this
push:
new 5db56c8 fix review comments.
5db56c8 is described below
commit 5db56c80918c6a8132a28c8efa8af1b35a025632
Author: eshu <[email protected]>
AuthorDate: Thu Apr 18 09:18:57 2019 -0700
fix review comments.
---
.../internal/cache/PRHARedundancyProvider.java | 40 ++-------------
.../partitioned/PersistentBucketRecoverer.java | 58 ++++++++++++++++++----
.../partitioned/PersistentBucketRecovererTest.java | 3 +-
3 files changed, 53 insertions(+), 48 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
index add48cf..7ec3523 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
@@ -1708,15 +1708,11 @@ public class PRHARedundancyProvider {
ArrayList<ProxyBucketRegion> bucketsHostedLocally =
new ArrayList<ProxyBucketRegion>(proxyBucketArray.length);
-
+ persistentBucketRecoverer =
createPersistentBucketRecoverer(proxyBucketArray.length);
/*
* Start the redundancy logger before recovering any proxy buckets.
*/
- persistentBucketRecoverer =
createPersistentBucketRecoverer(proxyBucketArray.length);
- Thread loggingThread = new LoggingThread(
- "PersistentBucketRecoverer for region " + prRegion.getName(), false,
- persistentBucketRecoverer);
- loggingThread.start();
+ persistentBucketRecoverer.startLoggingThread();
/*
* Spawn a separate thread for bucket that we previously hosted to recover
that bucket.
@@ -1738,7 +1734,6 @@ public class PRHARedundancyProvider {
if (proxyBucket.getPersistenceAdvisor().wasHosting()) {
final RecoveryRunnable recoveryRunnable = new RecoveryRunnable(this) {
-
@Override
public void run() {
// Fix for 44551 - make sure that we always count down
@@ -1989,22 +1984,8 @@ public class PRHARedundancyProvider {
*/
protected void waitForPersistentBucketRecoveryOrClose() {
if (getPersistentBucketRecoverer() != null) {
- boolean interrupted = false;
- while (true) {
- try {
- this.prRegion.getCancelCriterion().checkCancelInProgress(null);
- boolean done = getPersistentBucketRecoverer().await(
- PartitionedRegionHelper.DEFAULT_WAIT_PER_RETRY_ITERATION,
TimeUnit.MILLISECONDS);
- if (done) {
- break;
- }
- } catch (InterruptedException e) {
- interrupted = true;
- }
- }
- if (interrupted) {
- Thread.currentThread().interrupt();
- }
+ getPersistentBucketRecoverer().await(
+ PartitionedRegionHelper.DEFAULT_WAIT_PER_RETRY_ITERATION,
TimeUnit.MILLISECONDS);
}
List<PartitionedRegion> colocatedRegions =
@@ -2020,18 +2001,7 @@ public class PRHARedundancyProvider {
*/
protected void waitForPersistentBucketRecovery() {
if (getPersistentBucketRecoverer() != null) {
- boolean interrupted = false;
- while (true) {
- try {
- getPersistentBucketRecoverer().await();
- break;
- } catch (InterruptedException e) {
- interrupted = true;
- }
- }
- if (interrupted) {
- Thread.currentThread().interrupt();
- }
+ getPersistentBucketRecoverer().await();
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
index 813517d..021c03a 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
@@ -39,15 +39,15 @@ import org.apache.geode.internal.cache.ProxyBucketRegion;
import org.apache.geode.internal.cache.persistence.PersistentMemberID;
import org.apache.geode.internal.cache.persistence.PersistentStateListener;
import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.logging.LoggingThread;
import org.apache.geode.internal.net.SocketCreator;
import org.apache.geode.internal.process.StartupStatus;
import org.apache.geode.internal.util.TransformUtils;
/**
* Consolidates logging during the recovery of ProxyRegionBuckets that are not
hosted by this
- * member. This logger is meant to run in its own thread and utilizes the
PRHARedundancyProvider's
- * count down latch in order to determine when it is finished.
- *
+ * member. The logger is meant to run in its own thread.
+ * It uses a count down latch to determine whether the recovery is finished.
*/
public class PersistentBucketRecoverer extends RecoveryRunnable implements
PersistentStateListener {
@@ -95,12 +95,20 @@ public class PersistentBucketRecoverer extends
RecoveryRunnable implements Persi
allBucketsRecoveredFromDisk = new CountDownLatch(proxyBuckets);
membershipChanged = true;
addListeners();
+
}
List<PartitionedRegion> getColocatedChildRegions(PartitionedRegion
baseRegion) {
return ColocationHelper.getColocatedChildRegions(baseRegion);
}
+ public void startLoggingThread() {
+ Thread loggingThread = new LoggingThread(
+ "PersistentBucketRecoverer for region " +
redundancyProvider.prRegion.getName(), false,
+ this);
+ loggingThread.start();
+ }
+
/**
* Called when a member comes online for a bucket.
*/
@@ -157,7 +165,7 @@ public class PersistentBucketRecoverer extends
RecoveryRunnable implements Persi
public void run2() {
try {
boolean warningLogged = false;
- while (this.allBucketsRecoveredFromDisk.getCount() > 0) {
+ while (getLatchCount() > 0) {
int sleepMillis = SLEEP_PERIOD;
// reduce the first log time from 15secs so that higher layers can
// report sooner to user
@@ -342,8 +350,7 @@ public class PersistentBucketRecoverer extends
RecoveryRunnable implements Persi
Map<PersistentMemberID, Set<Integer>> offlineMembers =
getMembersToWaitFor(true);
Map<PersistentMemberID, Set<Integer>> allMembersToWaitFor =
getMembersToWaitFor(false);
- boolean thereAreBucketsToBeRecovered =
-
(PersistentBucketRecoverer.this.allBucketsRecoveredFromDisk.getCount() > 0);
+ boolean thereAreBucketsToBeRecovered = (getLatchCount() > 0);
/*
* Log any offline members the region is waiting for.
@@ -408,12 +415,37 @@ public class PersistentBucketRecoverer extends
RecoveryRunnable implements Persi
}
}
- public boolean await(long timeout, TimeUnit unit) throws
InterruptedException {
- return allBucketsRecoveredFromDisk.await(timeout, unit);
+ public void await(long timeout, TimeUnit unit) {
+ boolean interrupted = false;
+ while (true) {
+ try {
+
redundancyProvider.prRegion.getCancelCriterion().checkCancelInProgress(null);
+ boolean done = allBucketsRecoveredFromDisk.await(timeout, unit);
+ if (done) {
+ break;
+ }
+ } catch (InterruptedException e) {
+ interrupted = true;
+ }
+ }
+ if (interrupted) {
+ Thread.currentThread().interrupt();
+ }
}
- public void await() throws InterruptedException {
- allBucketsRecoveredFromDisk.await();
+ public void await() {
+ boolean interrupted = false;
+ while (true) {
+ try {
+ getAllBucketsRecoveredFromDiskLatch().await();
+ break;
+ } catch (InterruptedException e) {
+ interrupted = true;
+ }
+ }
+ if (interrupted) {
+ Thread.currentThread().interrupt();
+ }
}
public void countDown() {
@@ -421,12 +453,16 @@ public class PersistentBucketRecoverer extends
RecoveryRunnable implements Persi
}
public boolean hasRecoveryCompleted() {
- if (allBucketsRecoveredFromDisk.getCount() > 0) {
+ if (getLatchCount() > 0) {
return false;
}
return true;
}
+ long getLatchCount() {
+ return allBucketsRecoveredFromDisk.getCount();
+ }
+
CountDownLatch getAllBucketsRecoveredFromDiskLatch() {
return allBucketsRecoveredFromDisk;
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecovererTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecovererTest.java
index c2afc3a..0500d47 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecovererTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecovererTest.java
@@ -42,7 +42,6 @@ public class PersistentBucketRecovererTest {
new PersistentBucketRecoverer(provider, numberOfProxyBuckets);
assertThat(recoverer.getAllBucketsRecoveredFromDiskLatch()).isNotNull();
- assertThat(recoverer.getAllBucketsRecoveredFromDiskLatch().getCount())
- .isEqualTo(numberOfProxyBuckets);
+ assertThat(recoverer.getLatchCount()).isEqualTo(numberOfProxyBuckets);
}
}