gesterzhou commented on a change in pull request #6827:
URL: https://github.com/apache/geode/pull/6827#discussion_r717055360
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
##########
@@ -1797,19 +1815,9 @@ public void forceDisconnect(final String reason) {
}
if (this.isReconnectingDS()) {
- logger.info("Reconnecting system failed to connect");
- uncleanShutdown(reason,
- new MemberDisconnectedException("reconnecting system failed to
connect"));
- return;
- }
-
- try {
- listener.saveConfig();
- } finally {
- new LoggingThread("DisconnectThread", false, () -> {
- lifecycleListener.forcedDisconnect();
- uncleanShutdown(reason, shutdownCause);
- }).start();
+ uncleanShutdownReconnectingDS(reason, shutdownCause);
Review comment:
services.setShutdownCause(shutdownCause);
is different with listener.setShutdownCause(shutdownCause).
They are 2 separate steps in previous code.
services.setShutdownCause() set shutdownCause into Services class only,
while
ClusterDistributionManager.DMListener's setShutdownCause(shutdownCause) will
convert other cause into ForceDisconnectException and set into DM.
We call listener.setShutdownCause() in both if and else is because
listener.setShutdownCause() has to be called after listener.saveConfig() and
lifecycleListener.forcedDisconnect().
It cannot be called around line 1808.
To make thing clearer, maybe we can the listener's method name to be:
setPossibleShutdownCauseToForceDisconnectException(shutdownCause). Then it
will not mixed with services.setShutdownCause().
--
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]
