You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:55:22 UTC

[GitHub] [geode] bschuchardt opened a new pull request #5403: GEODE-8385: hang recovering from disk with cyclic dependencies

bschuchardt opened a new pull request #5403:
URL: https://github.com/apache/geode/pull/5403


   This restores the point at which we notify membership listeners of
   departures.  We used to do this (in 1.12 and earlier) when a ShutdownMessage
   is received instead of waiting for a new membership view announcing the departure.
   Membership views can take some time to form and install, which can cause
   persistent (disk store) views to be updated later than they used to be.
   
   In the case of this ticket the disk store of one member was being
   closed while another was shutting down.  The member closing its disk
   store did not see the view announcing that shutdown until most of its
   disk store regions had closed their persistence advisors.  This left the
   disk store thinking that the other member was still up at the time it
   was closed.
   
   Thank you for submitting a contribution to Apache Geode.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   - [ ] Does `gradlew build` run cleanly?
   
   - [ ] Have you written or updated unit tests to verify your changes?
   
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   
   ### Note:
   Please ensure that once the PR is submitted, check Concourse for build issues and
   submit an update to your PR as soon as possible. If you need help, please send an
   email to dev@geode.apache.org.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5403: GEODE-8385: hang recovering from disk with cyclic dependencies

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5403:
URL: https://github.com/apache/geode/pull/5403#discussion_r461788593



##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,39 +1848,56 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason);

Review comment:
       Why this is not `handleManagerDeparture(theId, false, reason, true)`? `handleManagerDeparture(theId, false, reason)` will not update the `stats` in line 1887.

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,39 +1848,56 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason);
   }
 
+  /*
+   * handleManagerDeparted may be invoked multiple times for a member identifier.
+   * We allow this and inform listeners on each invocation, but only perform some
+   * actions (such as decrementing the node count) if the change came from a
+   * membership view.
+   */
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason) {
+    handleManagerDeparture(theId, memberCrashed, reason, false);
+  }
 
+  private void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);
+
     int vmType = theId.getVmKind();
     if (vmType == ADMIN_ONLY_DM_TYPE) {
-      removeUnfinishedStartup(theId, true);
-      handleConsoleShutdown(theId, p_crashed, p_reason);
+      handleConsoleShutdown(theId, memberCrashed, reason);
       return;
     }
 
-    removeUnfinishedStartup(theId, true);
-
-    if (removeManager(theId, p_crashed, p_reason)) {
-      if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-        stats.incNodes(-1);
-      }
-      String msg;
-      if (p_crashed && !shouldInhibitMembershipWarnings()) {
-        msg =
-            "Member at {} unexpectedly left the distributed cache: {}";
-        addMemberEvent(new MemberCrashedEvent(theId, p_reason));
-      } else {
-        msg =
-            "Member at {} gracefully left the distributed cache: {}";
-        addMemberEvent(new MemberDepartedEvent(theId, p_reason));
-      }
-      logger.info(msg, new Object[] {theId, prettifyReason(p_reason)});
+    if (logger.isDebugEnabled()) {
+      logger.debug(
+          "DistributionManager: removing member <{}>; crashed {}; reason = {} fromView = {}", theId,
+          memberCrashed, prettifyReason(reason), fromViewChange);
+    }
+    removeHostedLocators(theId);
+    redundancyZones.remove(theId);
 
+    if (fromViewChange && theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
+      stats.incNodes(-1);
+    }
+    String msg;
+    if (memberCrashed && !shouldInhibitMembershipWarnings()) {
+      msg =
+          "Member at {} unexpectedly left the distributed cache: {}";
+      addMemberEvent(new MemberCrashedEvent(theId, reason));
+    } else {
+      msg =
+          "Member at {} gracefully left the distributed cache: {}";
+      addMemberEvent(new MemberDepartedEvent(theId, reason));
+    }
+    if (fromViewChange) {
+      logger.info(msg, new Object[] {theId, prettifyReason(reason)});

Review comment:
       Would it better to move the `logger.info` out of the `if` statement? So regardless the value of `fromViewChange`, it logs the member departure. It will be helpful when analyzing the logs. 

##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
##########
@@ -688,7 +688,11 @@ private void removeWithViewLock(ID dm, boolean crashed, String reason) {
       return; // Explicit deletion, no upcall.
     }
 
-    listener.memberDeparted(dm, crashed, reason);
+    if (!shutdownMembers.containsKey(dm)) {

Review comment:
       πŸ‘ 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] bschuchardt merged pull request #5403: GEODE-8385: hang recovering from disk with cyclic dependencies

Posted by GitBox <gi...@apache.org>.
bschuchardt merged pull request #5403:
URL: https://github.com/apache/geode/pull/5403


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] Bill commented on a change in pull request #5403: GEODE-8385: hang recovering from disk with cyclic dependencies

Posted by GitBox <gi...@apache.org>.
Bill commented on a change in pull request #5403:
URL: https://github.com/apache/geode/pull/5403#discussion_r461217750



##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,41 +1862,54 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason, false);
   }
 
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
-
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);
+
     int vmType = theId.getVmKind();
     if (vmType == ADMIN_ONLY_DM_TYPE) {
-      removeUnfinishedStartup(theId, true);
-      handleConsoleShutdown(theId, p_crashed, p_reason);
+      handleConsoleShutdown(theId, memberCrashed, reason);
       return;
     }
 
-    removeUnfinishedStartup(theId, true);
-
-    if (removeManager(theId, p_crashed, p_reason)) {
-      if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-        stats.incNodes(-1);
+    if (!fromViewChange) {
+      if (!isCurrentMember(theId)) {
+        // this is notification from a shutdown message received from a member that is no longer
+        // part of the cluster
+        return;
       }
-      String msg;
-      if (p_crashed && !shouldInhibitMembershipWarnings()) {
-        msg =
-            "Member at {} unexpectedly left the distributed cache: {}";
-        addMemberEvent(new MemberCrashedEvent(theId, p_reason));
-      } else {
-        msg =
-            "Member at {} gracefully left the distributed cache: {}";
-        addMemberEvent(new MemberDepartedEvent(theId, p_reason));
+      // else this is from a shutdown message so continue & notify listeners
+    } else {
+      if (!memberCrashed) {
+        // member left the view normally - we've already received a shutdown message and notified
+        // listeners, so there's nothing more to do here
+        return;
       }
-      logger.info(msg, new Object[] {theId, prettifyReason(p_reason)});
+    }
 
-      executors.handleManagerDeparture(theId);
+    removeManager(theId, memberCrashed, reason);
+    if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
+      stats.incNodes(-1);
+    }
+    String msg;
+    if (memberCrashed && !shouldInhibitMembershipWarnings()) {
+      msg =
+          "Member at {} unexpectedly left the distributed cache: {}";
+      addMemberEvent(new MemberCrashedEvent(theId, reason));
+    } else {
+      msg =
+          "Member at {} gracefully left the distributed cache: {}";
+      addMemberEvent(new MemberDepartedEvent(theId, reason));
     }
+    logger.info(msg, new Object[] {theId, prettifyReason(reason)});
+
+    executors.handleManagerDeparture(theId);

Review comment:
       βœ“ ok to this block of codeβ€”I confirmed it's the same as the old block that was under the conditional but no conditional is needed now that `removeManager()` returns `void` (and it used to always return `true`) βœ“

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -2366,10 +2375,10 @@ public void memberDeparted(InternalDistributedMember theId, boolean crashed, Str
           message.setReason(reason); // added for #37950
           handleIncomingDMsg(message);
         }
-        dm.handleManagerDeparture(theId, crashed, reason);
       } catch (DistributedSystemDisconnectedException se) {
         // let's not get huffy about it
       }
+      dm.handleManagerDeparture(theId, crashed, reason, true);

Review comment:
       why did this statement move? should it be in a `finally` block?

##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java
##########
@@ -399,11 +399,11 @@ public int hashCode() {
   public String toString() {
     StringBuilder sb = new StringBuilder(100);
 
-    sb.append("MemberData[");
+    sb.append("GMSMember[");

Review comment:
       Why did the name change to `"GMSMember"` here? It was `"MemberData"` which kinda made sense because `GMSMemberData` isa `MemberData`. I could see leaving it alone or maybe making it `GMSMemberData`.

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1784,19 +1784,15 @@ private String prettifyReason(String r) {
   /**
    * Returns true if id was removed. Returns false if it was not in the list of managers.
    */
-  private boolean removeManager(InternalDistributedMember theId, boolean crashed, String p_reason) {
-    String reason = p_reason;
-
-    reason = prettifyReason(reason);
+  private void removeManager(InternalDistributedMember theId, boolean crashed, String p_reason) {

Review comment:
       comment lies now that this returns `void`

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,41 +1848,45 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason, false);
   }
 
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
-
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);

Review comment:
       nice

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,41 +1848,45 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason, false);
   }
 
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
-
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);
+
     int vmType = theId.getVmKind();
     if (vmType == ADMIN_ONLY_DM_TYPE) {
-      removeUnfinishedStartup(theId, true);
-      handleConsoleShutdown(theId, p_crashed, p_reason);
+      handleConsoleShutdown(theId, memberCrashed, reason);
       return;
     }
 
-    removeUnfinishedStartup(theId, true);
-
-    if (removeManager(theId, p_crashed, p_reason)) {
-      if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-        stats.incNodes(-1);
-      }
-      String msg;
-      if (p_crashed && !shouldInhibitMembershipWarnings()) {
-        msg =
-            "Member at {} unexpectedly left the distributed cache: {}";
-        addMemberEvent(new MemberCrashedEvent(theId, p_reason));
-      } else {
-        msg =
-            "Member at {} gracefully left the distributed cache: {}";
-        addMemberEvent(new MemberDepartedEvent(theId, p_reason));
-      }
-      logger.info(msg, new Object[] {theId, prettifyReason(p_reason)});
+    if (logger.isDebugEnabled()) {
+      logger.debug("DistributionManager: removing member <{}>; crashed {}; reason = {}", theId,
+          memberCrashed, prettifyReason(reason));
+    }
+    removeHostedLocators(theId);
+    redundancyZones.remove(theId);

Review comment:
       βœ“ inlined `removeManager()`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] bschuchardt commented on a change in pull request #5403: GEODE-8385: hang recovering from disk with cyclic dependencies

Posted by GitBox <gi...@apache.org>.
bschuchardt commented on a change in pull request #5403:
URL: https://github.com/apache/geode/pull/5403#discussion_r461613391



##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java
##########
@@ -399,11 +399,11 @@ public int hashCode() {
   public String toString() {
     StringBuilder sb = new StringBuilder(100);
 
-    sb.append("MemberData[");
+    sb.append("GMSMember[");

Review comment:
       I think that's an artifact of my doing this work on an old SHA and then rebasing onto develop.  I'll revert that change

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -2366,10 +2375,10 @@ public void memberDeparted(InternalDistributedMember theId, boolean crashed, Str
           message.setReason(reason); // added for #37950
           handleIncomingDMsg(message);
         }
-        dm.handleManagerDeparture(theId, crashed, reason);
       } catch (DistributedSystemDisconnectedException se) {
         // let's not get huffy about it
       }
+      dm.handleManagerDeparture(theId, crashed, reason, true);

Review comment:
       that change was unnecessary - I'll revert it.

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1784,19 +1784,15 @@ private String prettifyReason(String r) {
   /**
    * Returns true if id was removed. Returns false if it was not in the list of managers.
    */
-  private boolean removeManager(InternalDistributedMember theId, boolean crashed, String p_reason) {
-    String reason = p_reason;
-
-    reason = prettifyReason(reason);
+  private void removeManager(InternalDistributedMember theId, boolean crashed, String p_reason) {

Review comment:
       I've in-lined this method.  Its name no longer made sense and it was only used by handleManagerDeparture()

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,39 +1848,56 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason);
   }
 
+  /*
+   * handleManagerDeparted may be invoked multiple times for a member identifier.
+   * We allow this and inform listeners on each invocation, but only perform some
+   * actions (such as decrementing the node count) if the change came from a
+   * membership view.
+   */
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason) {
+    handleManagerDeparture(theId, memberCrashed, reason, false);
+  }
 
+  private void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);
+
     int vmType = theId.getVmKind();
     if (vmType == ADMIN_ONLY_DM_TYPE) {
-      removeUnfinishedStartup(theId, true);
-      handleConsoleShutdown(theId, p_crashed, p_reason);
+      handleConsoleShutdown(theId, memberCrashed, reason);
       return;
     }
 
-    removeUnfinishedStartup(theId, true);
-
-    if (removeManager(theId, p_crashed, p_reason)) {
-      if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-        stats.incNodes(-1);
-      }
-      String msg;
-      if (p_crashed && !shouldInhibitMembershipWarnings()) {
-        msg =
-            "Member at {} unexpectedly left the distributed cache: {}";
-        addMemberEvent(new MemberCrashedEvent(theId, p_reason));
-      } else {
-        msg =
-            "Member at {} gracefully left the distributed cache: {}";
-        addMemberEvent(new MemberDepartedEvent(theId, p_reason));
-      }
-      logger.info(msg, new Object[] {theId, prettifyReason(p_reason)});
+    if (logger.isDebugEnabled()) {
+      logger.debug(
+          "DistributionManager: removing member <{}>; crashed {}; reason = {} fromView = {}", theId,
+          memberCrashed, prettifyReason(reason), fromViewChange);
+    }
+    removeHostedLocators(theId);
+    redundancyZones.remove(theId);
 
+    if (fromViewChange && theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
+      stats.incNodes(-1);
+    }
+    String msg;
+    if (memberCrashed && !shouldInhibitMembershipWarnings()) {
+      msg =
+          "Member at {} unexpectedly left the distributed cache: {}";
+      addMemberEvent(new MemberCrashedEvent(theId, reason));
+    } else {
+      msg =
+          "Member at {} gracefully left the distributed cache: {}";
+      addMemberEvent(new MemberDepartedEvent(theId, reason));
+    }
+    if (fromViewChange) {
+      logger.info(msg, new Object[] {theId, prettifyReason(reason)});

Review comment:
       If I move it out of the `if` statement it will log the departure on every invocation of this method, and I don't want to do that.  I think it will be confusing.

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,39 +1848,56 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason);

Review comment:
       Sorry, I don't understand the question.  We want to decrement the node count once, but this method will be invoked multiple times for the same departing node.  If we restrict the decrement to view-changes it will only be decremented once because we only notify memberDeparted once for a member that's left the view.

##########
File path: geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1866,39 +1848,56 @@ public void handleConsoleShutdown(InternalDistributedMember theId, boolean crash
   void shutdownMessageReceived(InternalDistributedMember theId, String reason) {
     removeHostedLocators(theId);
     distribution.shutdownMessageReceived(theId, reason);
+    handleManagerDeparture(theId, false, reason);
   }
 
+  /*
+   * handleManagerDeparted may be invoked multiple times for a member identifier.
+   * We allow this and inform listeners on each invocation, but only perform some
+   * actions (such as decrementing the node count) if the change came from a
+   * membership view.
+   */
   @Override
-  public void handleManagerDeparture(InternalDistributedMember theId, boolean p_crashed,
-      String p_reason) {
+  public void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason) {
+    handleManagerDeparture(theId, memberCrashed, reason, false);
+  }
 
+  private void handleManagerDeparture(InternalDistributedMember theId, boolean memberCrashed,
+      String reason, boolean fromViewChange) {
     alertingService.removeAlertListener(theId);
 
+    removeUnfinishedStartup(theId, true);
+
     int vmType = theId.getVmKind();
     if (vmType == ADMIN_ONLY_DM_TYPE) {
-      removeUnfinishedStartup(theId, true);
-      handleConsoleShutdown(theId, p_crashed, p_reason);
+      handleConsoleShutdown(theId, memberCrashed, reason);
       return;
     }
 
-    removeUnfinishedStartup(theId, true);
-
-    if (removeManager(theId, p_crashed, p_reason)) {
-      if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-        stats.incNodes(-1);
-      }
-      String msg;
-      if (p_crashed && !shouldInhibitMembershipWarnings()) {
-        msg =
-            "Member at {} unexpectedly left the distributed cache: {}";
-        addMemberEvent(new MemberCrashedEvent(theId, p_reason));
-      } else {
-        msg =
-            "Member at {} gracefully left the distributed cache: {}";
-        addMemberEvent(new MemberDepartedEvent(theId, p_reason));
-      }
-      logger.info(msg, new Object[] {theId, prettifyReason(p_reason)});
+    if (logger.isDebugEnabled()) {
+      logger.debug(
+          "DistributionManager: removing member <{}>; crashed {}; reason = {} fromView = {}", theId,
+          memberCrashed, prettifyReason(reason), fromViewChange);
+    }
+    removeHostedLocators(theId);
+    redundancyZones.remove(theId);
 
+    if (fromViewChange && theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
+      stats.incNodes(-1);
+    }
+    String msg;
+    if (memberCrashed && !shouldInhibitMembershipWarnings()) {
+      msg =
+          "Member at {} unexpectedly left the distributed cache: {}";
+      addMemberEvent(new MemberCrashedEvent(theId, reason));
+    } else {
+      msg =
+          "Member at {} gracefully left the distributed cache: {}";
+      addMemberEvent(new MemberDepartedEvent(theId, reason));
+    }
+    if (fromViewChange) {
+      logger.info(msg, new Object[] {theId, prettifyReason(reason)});

Review comment:
       I've simplified the method now, so it no longer has fromViewChange.  We'll be notifying listeners of departures when we get a shutdown message.  The subsequent view change won't trigger listener invocation.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org