gesterzhou commented on a change in pull request #6827:
URL: https://github.com/apache/geode/pull/6827#discussion_r717054624
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MembershipListener.java
##########
@@ -71,4 +71,7 @@ void memberSuspect(ID suspect, ID whoSuspected,
*/
void saveConfig();
+ default Throwable setShutdownCause(Throwable t) {
Review comment:
I can add javadoc for this method.
Maybe the method name can be changed to
setPossibleShutdownCauseToForceDisconnectException(shutdownCause)
The method signature is from the refactoring above. Since the original code
expects to convert throwable into ForceDisconnectionException, thus this
setShutdownCause() should return a Throwable.
From listener point of view, we don't need to return anything. We did this
way is only to avoid duplicated code.
Of cause, I can complete revoked the code changes in
ClusterDistributionManager.java, then I can change to void
setShutdownCause(Throwable t) here, because in GMSMembership.java, we don't
need setShutdownCause() to return anything.
However, in our code, there will be 2 places with very similar code, one in
ClusterDistributionManager, one in GMSMembership. That's what we want to
avoid.
Conceptually, the whole fix's idea is to move setShutdownCause() a little
earlier in force disconnect case. So it makes sense to do the refactoring in
ClusterDistributionManager.DMListener into a method, then let listener to make
use of this method.
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
##########
@@ -1775,6 +1775,24 @@ public void memberSuspected(ID initiator, ID suspect,
String reason) {
handleOrDeferSuspect(s);
}
+ void uncleanShutdownReconnectingDS(String reason, Exception shutdownCause)
{
+ logger.info("Reconnecting system failed to connect");
+ listener.setShutdownCause(shutdownCause);
+ uncleanShutdown(reason,
+ new MemberDisconnectedException("reconnecting system failed to
connect"));
+ }
+
+ void uncleanShutdownDS(String reason, Exception shutdownCause) {
+ try {
+ listener.saveConfig();
+ } finally {
+ new LoggingThread("DisconnectThread", false, () -> {
+ lifecycleListener.forcedDisconnect();
+ listener.setShutdownCause(shutdownCause);
+ uncleanShutdown(reason, shutdownCause);
Review comment:
We did not change to use logging thread here. The original code called
uncleanShutdown() in logging thread.
Our fix here is only to add listener.setShutdownCause(shutdownCause) in
both isReconnectingDS() and not reconnecting case.
--
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]
