dschneider-pivotal commented on a change in pull request #7113:
URL: https://github.com/apache/geode/pull/7113#discussion_r750506696
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
##########
@@ -1155,6 +1155,7 @@ public void close() {
isClosed = true;
persistentMemberManager.removeRevocationListener(profileChangeListener);
cacheDistributionAdvisor.removeProfileChangeListener(profileChangeListener);
+ persistentStateListeners = Collections.emptySet();
Review comment:
add unit test coverage for this change to this class
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
##########
@@ -182,12 +183,23 @@ public void run2() {
if (!warningLogged) {
sleepMillis = SLEEP_PERIOD / 2;
}
+ if (regions.isEmpty()) {
Review comment:
I'm not sure about all these changes but this method needs unit test
coverage for them.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PersistentBucketRecoverer.java
##########
@@ -246,8 +264,12 @@ public RegionStatus(PartitionedRegion region) {
}
public void removeListeners() {
+ BucketPersistenceAdvisor tempBPA;
for (ProxyBucketRegion proxyBucket : bucketRegions) {
-
proxyBucket.getPersistenceAdvisor().removeListener(PersistentBucketRecoverer.this);
+ tempBPA = proxyBucket.getPersistenceAdvisor();
+ if (!tempBPA.isClosed()) {
+
proxyBucket.getPersistenceAdvisor().removeListener(PersistentBucketRecoverer.this);
Review comment:
why do you refetch "proxyBucket.getPersistenceAdvisor()" instead of
using tempBPA?
Also, would it be better to do this isClosed check in
BucketPersistenceAdvisor?
Also why did you make this particular change? You have changed close to set
the listeners to an empty collection. You could just have removeListener check
if the set isEmpty and return. It seems like this method would not need to
change at all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
