[
https://issues.apache.org/jira/browse/GEODE-3599?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246220#comment-16246220
]
ASF GitHub Bot commented on GEODE-3599:
---------------------------------------
dschneider-pivotal closed pull request #1037: GEODE-3599: remove getInstance
calls
URL: https://github.com/apache/geode/pull/1037
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DM.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DM.java
index 1ce742188b..43de293c18 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DM.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DM.java
@@ -24,6 +24,8 @@
import java.util.concurrent.ExecutorService;
import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheClosedException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.Role;
import org.apache.geode.distributed.internal.locks.ElderState;
@@ -464,7 +466,22 @@ public void addHostedLocators(InternalDistributedMember
member, Collection<Strin
int getDMType();
+ /**
+ * The returned cache will be null if the cache does not yet exist. Note
that the returned cache
+ * may be one that is already closed. Callers of
GemFireCacheImpl.getInstance() should try to use
+ * this method.
+ */
InternalCache getCache();
+ /**
+ * Returns an existing non-closed cache associated with this DM. Callers of
+ * CacheFactory.getAnyInstance(),
CacheFactory.getInstance(DistributedSystem) or
+ * GemFireCacheImpl.getExisting() should try to use this method.
+ *
+ * @throws CacheClosedException if a cache has not yet been associated with
this DM or it has been
+ * {@link Cache#isClosed closed}.
+ */
+ InternalCache getExistingCache();
+
void setCache(InternalCache instance);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
index 339d673605..6130e12f71 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
@@ -53,6 +53,7 @@
import org.apache.geode.SystemFailure;
import org.apache.geode.ToDataException;
import org.apache.geode.admin.GemFireHealthConfig;
+import org.apache.geode.cache.CacheClosedException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DistributedSystemDisconnectedException;
import org.apache.geode.distributed.Locator;
@@ -3939,7 +3940,7 @@ public String getDistributionConfigDescription() {
/* -----------------------------Health Monitor------------------------------
*/
private final ConcurrentMap hmMap = new ConcurrentHashMap();
- private InternalCache cache;
+ private volatile InternalCache cache;
/**
* Returns the health monitor for this distribution manager and owner.
@@ -4820,4 +4821,19 @@ public void setCache(InternalCache instance) {
public InternalCache getCache() {
return this.cache;
}
+
+ @Override
+ public InternalCache getExistingCache() {
+ InternalCache result = this.cache;
+ if (result == null) {
+ throw new CacheClosedException(
+
LocalizedStrings.CacheFactory_A_CACHE_HAS_NOT_YET_BEEN_CREATED.toLocalizedString());
+ }
+ result.getCancelCriterion().checkCancelInProgress(null);
+ if (result.isClosed()) {
+ throw result.getCacheClosedException(
+
LocalizedStrings.CacheFactory_THE_CACHE_HAS_BEEN_CLOSED.toLocalizedString(),
null);
+ }
+ return result;
+ }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java
index 8bbe019889..ffe4054d16 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java
@@ -22,6 +22,7 @@
import org.apache.geode.CancelCriterion;
import org.apache.geode.InternalGemFireError;
+import org.apache.geode.cache.CacheClosedException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DurableClientAttributes;
import org.apache.geode.distributed.Role;
@@ -1249,7 +1250,7 @@ public RuntimeException
generateCancelledException(Throwable e) {
}
private final Stopper stopper = new Stopper();
- private InternalCache cache;
+ private volatile InternalCache cache;
public CancelCriterion getCancelCriterion() {
return stopper;
@@ -1379,12 +1380,27 @@ public boolean
isSharedConfigurationServiceEnabledForDS() {
}
@Override
+ public void setCache(InternalCache instance) {
+ this.cache = instance;
+ }
+
+ @Override
public InternalCache getCache() {
return this.cache;
}
@Override
- public void setCache(InternalCache instance) {
- this.cache = instance;
+ public InternalCache getExistingCache() {
+ InternalCache result = this.cache;
+ if (result == null) {
+ throw new CacheClosedException(
+
LocalizedStrings.CacheFactory_A_CACHE_HAS_NOT_YET_BEEN_CREATED.toLocalizedString());
+ }
+ result.getCancelCriterion().checkCancelInProgress(null);
+ if (result.isClosed()) {
+ throw result.getCacheClosedException(
+
LocalizedStrings.CacheFactory_THE_CACHE_HAS_BEEN_CLOSED.toLocalizedString(),
null);
+ }
+ return result;
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java
index 5680472857..94abc68e6d 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java
@@ -344,7 +344,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- InternalCache cache = (InternalCache)
CacheFactory.getInstance(dm.getSystem());
+ InternalCache cache = dm.getExistingCache();
// Fix for bug 42051 - Discover any regions that are in the process
// of being destroyed
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index e8aac768ac..36c07d1d09 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -676,7 +676,12 @@ public void write(int i) {
/** Map of Futures used to track Regions that are being reinitialized */
private final ConcurrentMap reinitializingRegions = new ConcurrentHashMap();
- /** Returns the last created instance of GemFireCache */
+ /**
+ * Returns the last created instance of GemFireCache
+ *
+ * @deprecated: use DM.getCache instead
+ */
+ @Deprecated
public static GemFireCacheImpl getInstance() {
return instance;
}
@@ -695,7 +700,9 @@ public static GemFireCacheImpl
setInstanceForTests(GemFireCacheImpl cache) {
*
* @return the existing cache
* @throws CacheClosedException if an existing cache can not be found.
+ * @deprecated use DM.getExistingCache instead.
*/
+ @Deprecated
public static GemFireCacheImpl getExisting() {
final GemFireCacheImpl result = instance;
if (result != null && !result.isClosing) {
@@ -715,7 +722,9 @@ public static GemFireCacheImpl getExisting() {
* @param reason the reason an existing cache is being requested.
* @return the existing cache
* @throws CacheClosedException if an existing cache can not be found.
+ * @deprecated use DM.getExistingCache instead.
*/
+ @Deprecated
public static GemFireCacheImpl getExisting(String reason) {
GemFireCacheImpl result = getInstance();
if (result == null) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipFlushRequest.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipFlushRequest.java
index 2a308568ae..25980d9b9e 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipFlushRequest.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipFlushRequest.java
@@ -73,7 +73,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- Cache cache = CacheFactory.getInstance(dm.getSystem());
+ Cache cache = dm.getExistingCache();
PartitionedRegion region = (PartitionedRegion)
cache.getRegion(this.regionPath);
if (region != null && region.getRegionAdvisor().isInitialized()) {
ProxyBucketRegion[] proxyBuckets =
region.getRegionAdvisor().getProxyBucketArray();
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipViewRequest.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipViewRequest.java
index 420d2567f3..82dc8150a8 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipViewRequest.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/MembershipViewRequest.java
@@ -108,7 +108,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- Cache cache = CacheFactory.getInstance(dm.getSystem());
+ Cache cache = dm.getExistingCache();
Region region = cache.getRegion(this.regionPath);
PersistenceAdvisor persistenceAdvisor = null;
if (region instanceof DistributedRegion) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentStateQueryMessage.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentStateQueryMessage.java
index 018325d38c..e87fd465f8 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentStateQueryMessage.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentStateQueryMessage.java
@@ -101,7 +101,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- Cache cache = CacheFactory.getInstance(dm.getSystem());
+ Cache cache = dm.getExistingCache();
Region region = cache.getRegion(this.regionPath);
PersistenceAdvisor persistenceAdvisor = null;
if (region instanceof DistributedRegion) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PrepareNewPersistentMemberMessage.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PrepareNewPersistentMemberMessage.java
index 5029bb0e13..f0385d96b4 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PrepareNewPersistentMemberMessage.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PrepareNewPersistentMemberMessage.java
@@ -91,7 +91,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- Cache cache = CacheFactory.getInstance(dm.getSystem());
+ Cache cache = dm.getExistingCache();
Region region = cache.getRegion(this.regionPath);
PersistenceAdvisor persistenceAdvisor = null;
if (region instanceof DistributedRegion) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/RemovePersistentMemberMessage.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/RemovePersistentMemberMessage.java
index a035c74350..0d67ca070e 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/RemovePersistentMemberMessage.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/RemovePersistentMemberMessage.java
@@ -95,7 +95,7 @@ protected void process(DistributionManager dm) {
// get the region from the path, but do NOT wait on initialization,
// otherwise we could have a distributed deadlock
- Cache cache = CacheFactory.getInstance(dm.getSystem());
+ Cache cache = dm.getExistingCache();
Region region = cache.getRegion(this.regionPath);
PersistenceAdvisor persistenceAdvisor = null;
if (region instanceof DistributedRegion) {
diff --git a/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
b/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
index cd75a04ddb..5148f5fe11 100644
--- a/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
+++ b/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
@@ -98,6 +98,7 @@ public static GemFireCacheImpl cache() {
when(distributionManager.getSystem()).thenReturn(system);
when(distributionManager.getCancelCriterion()).thenReturn(systemCancelCriterion);
when(distributionManager.getCache()).thenReturn(cache);
+ when(distributionManager.getExistingCache()).thenReturn(cache);
return cache;
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> refactor Message classes process methods to not call CacheFactory.getInstance
> -----------------------------------------------------------------------------
>
> Key: GEODE-3599
> URL: https://issues.apache.org/jira/browse/GEODE-3599
> Project: Geode
> Issue Type: Sub-task
> Components: regions
> Reporter: Darrel Schneider
> Assignee: Darrel Schneider
>
> The following message classes all call CacheFactory.getInstance from their
> process methods but should instead call "dm.getCache":
> org.apache.geode.internal.cache.CreateRegionProcessor.CreateRegionMessage.process(DistributionManager)
> org.apache.geode.internal.cache.persistence.MembershipFlushRequest.process(DistributionManager)
> org.apache.geode.internal.cache.persistence.MembershipViewRequest.process(DistributionManager)
> org.apache.geode.internal.cache.persistence.PersistentStateQueryMessage.process(DistributionManager)
> org.apache.geode.internal.cache.persistence.PrepareNewPersistentMemberMessage.process(DistributionManager)
> org.apache.geode.internal.cache.persistence.RemovePersistentMemberMessage.process(DistributionManager)
> org.apache.geode.internal.cache.SearchLoadAndWriteProcessor.NetWriteRequestMessage.process(DistributionManager)
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
