This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new 8a66bc3 GEODE-8467: server fails to notify of a ForcedDisconnect and
fails to tear down the cache (#5490)
8a66bc3 is described below
commit 8a66bc3b376f391b7f387c74b9b5246d72882178
Author: Bruce Schuchardt <bschucha...@pivotal.io>
AuthorDate: Tue Sep 1 07:32:47 2020 -0700
GEODE-8467: server fails to notify of a ForcedDisconnect and fails to tear
down the cache (#5490)
Catch exceptions that occur during XML generation and disable auto
reconnect.
Ensure that the DisconnectThread is launched by placing it in a
"finally" block.
(cherry picked from commit e402ed35102a4a885ad24a1052216b0542672bc7)
---
.../geode/internal/cache/GemFireCacheImpl.java | 70 ++++++++++++++--------
.../geode/internal/cache/GemFireCacheImplTest.java | 12 ++++
.../internal/membership/gms/GMSMembership.java | 15 ++---
3 files changed, 65 insertions(+), 32 deletions(-)
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 c4c8db5..2f6b0b4 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
@@ -59,6 +59,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
import java.io.Reader;
import java.io.StringBufferInputStream;
import java.io.StringWriter;
@@ -1126,34 +1127,53 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
@Override
public void saveCacheXmlForReconnect() {
- // there are two versions of this method so it can be unit-tested
- boolean sharedConfigEnabled =
getDistributionManager().getConfig().getUseSharedConfiguration();
+ prepareForReconnect((pw) -> CacheXmlGenerator.generate((Cache) this, pw,
false));
+ }
- if (!Boolean.getBoolean(GEMFIRE_PREFIX + "autoReconnect-useCacheXMLFile")
- && !sharedConfigEnabled) {
- try {
- logger.info("generating XML to rebuild the cache after reconnect
completes");
- StringPrintWriter pw = new StringPrintWriter();
- CacheXmlGenerator.generate((Cache) this, pw, false);
- String cacheXML = pw.toString();
- getCacheConfig().setCacheXMLDescription(cacheXML);
- logger.info("XML generation completed: {}", cacheXML);
- } catch (CancelException e) {
- logger.info("Unable to generate XML description for reconnect of cache
due to exception",
- e);
- }
- } else if (sharedConfigEnabled && !getCacheServers().isEmpty()) {
- // we need to retain a cache-server description if this JVM was started
by gfsh
- List<CacheServerCreation> list = new
ArrayList<>(getCacheServers().size());
- for (Object o : getCacheServers()) {
- CacheServerImpl cs = (CacheServerImpl) o;
- if (cs.isDefaultServer()) {
- CacheServerCreation bsc = new CacheServerCreation(this, cs);
- list.add(bsc);
+
+ /**
+ * Testable version of saveCacheXmlForReconnect() that allows us to inject
an XML generator
+ *
+ * @param xmlGenerator a consumer of a PrintWriter that generates a
description of the Cache
+ */
+ protected void prepareForReconnect(Consumer<PrintWriter> xmlGenerator) {
+ boolean sharedConfigEnabled =
+ getInternalDistributedSystem().getConfig().getUseSharedConfiguration();
+
+ try {
+ if (!Boolean.getBoolean(GEMFIRE_PREFIX + "autoReconnect-useCacheXMLFile")
+ && !sharedConfigEnabled) {
+ try {
+ logger.info("generating XML to rebuild the cache after reconnect
completes");
+ StringPrintWriter pw = new StringPrintWriter();
+ xmlGenerator.accept(pw);
+ String cacheXML = pw.toString();
+ getCacheConfig().setCacheXMLDescription(cacheXML);
+ logger.info("XML generation completed: {}", cacheXML);
+ } catch (CancelException e) {
+ logger.info("Unable to generate XML description for reconnect of
cache due to exception",
+ e);
}
+ } else if (sharedConfigEnabled && !getCacheServers().isEmpty()) {
+ // we need to retain a cache-server description if this JVM was
started by gfsh
+ logger.info("saving cache server configuration for use with the
cluster-configuration "
+ + "service on reconnect");
+ List<CacheServerCreation> list = new
ArrayList<>(getCacheServers().size());
+ for (Object o : getCacheServers()) {
+ CacheServerImpl cs = (CacheServerImpl) o;
+ if (cs.isDefaultServer()) {
+ CacheServerCreation bsc = new CacheServerCreation(this, cs);
+ list.add(bsc);
+ }
+ }
+ getCacheConfig().setCacheServerCreation(list);
+ logger.info("cache server configuration saved");
}
- getCacheConfig().setCacheServerCreation(list);
- logger.info("CacheServer configuration saved");
+ } catch (Throwable throwable) {
+ logger.info("Saving of cache configuration for auto-reconnect has
failed. "
+ + "Auto-reconnect will be disabled since the cache cannot be
rebuilt.", throwable);
+ getInternalDistributedSystem().getConfig().setDisableAutoReconnect(true);
+
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
index a0e7f28..cb36ebd 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
@@ -19,7 +19,9 @@ import static
org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -84,6 +86,7 @@ public class GemFireCacheImplTest {
typeRegistry = mock(TypeRegistry.class);
DistributionConfig distributionConfig = mock(DistributionConfig.class);
+ when(distributionConfig.getUseSharedConfiguration()).thenReturn(false);
DistributionManager distributionManager = mock(DistributionManager.class);
ReplyProcessor21 replyProcessor21 = mock(ReplyProcessor21.class);
@@ -620,6 +623,15 @@ public class GemFireCacheImplTest {
.isSameAs(gemFireCacheImpl.getCacheServers());
}
+ @Test
+ public void cacheXmlGenerationErrorDisablesAutoReconnect() {
+ gemFireCacheImpl.prepareForReconnect((printWriter) -> {
+ throw new RuntimeException("error generating cache XML");
+ });
+
verify(internalDistributedSystem.getConfig()).setDisableAutoReconnect(Boolean.TRUE);
+ verify(cacheConfig, never()).setCacheXMLDescription(isA(String.class));
+ }
+
@SuppressWarnings({"LambdaParameterHidesMemberVariable",
"OverlyCoupledMethod", "unchecked"})
private GemFireCacheImpl gemFireCacheImpl(boolean useAsyncEventListeners) {
return new GemFireCacheImpl(
diff --git
a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
index db2ecae..1986931 100644
---
a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
+++
b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
@@ -2028,13 +2028,14 @@ public class GMSMembership<ID extends MemberIdentifier>
implements Membership<ID
return;
}
- listener.saveConfig();
-
- Thread reconnectThread = new LoggingThread("DisconnectThread", false, ()
-> {
- lifecycleListener.forcedDisconnect();
- uncleanShutdown(reason, shutdownCause);
- });
- reconnectThread.start();
+ try {
+ listener.saveConfig();
+ } finally {
+ new LoggingThread("DisconnectThread", false, () -> {
+ lifecycleListener.forcedDisconnect();
+ uncleanShutdown(reason, shutdownCause);
+ }).start();
+ }
}
