[
https://issues.apache.org/jira/browse/GEODE-8385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17166958#comment-17166958
]
ASF GitHub Bot commented on GEODE-8385:
---------------------------------------
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:
[email protected]
> hang recovering from disk with cyclic dependencies
> --------------------------------------------------
>
> Key: GEODE-8385
> URL: https://issues.apache.org/jira/browse/GEODE-8385
> Project: Geode
> Issue Type: Bug
> Components: membership, persistence
> Reporter: Bruce J Schuchardt
> Assignee: Bruce J Schuchardt
> Priority: Major
> Fix For: 1.13.0
>
>
> In a test cluster using replicated persistent Regions all of the servers were
> shut down and restarted. The restart hung showing a cycle in disk store
> dependencies.
> {noformat}
> [info 2020/05/29 03:02:36.635 PDT <Thread-18> tid=0x8f] Region /Region_14 has
> potentially stale data. It is waiting for another online member to recover
> the latest data.My persistent id:
> DiskStore ID: a175354a-d27d-4575-9916-16fd7ff7ea67 Name:
> persistgemfire4_host1_4194 Location:
> /10.32.110.100:/var/vcap/data/rundir/concRecoverAllV4O41/concRecoverAll-0529-024642/vm_5_persist4_disk_1
> Members with potentially new data:[
> DiskStore ID: 2d77752e-507d-4425-a382-a5856c61938f Name:
> persistgemfire10_host1_4208 Location:
> /10.32.110.100:/var/vcap/data/rundir/concRecoverAllV4O41/concRecoverAll-0529-024642/vm_2_persist10_disk_1]
> Use the gfsh show missing-disk-stores command to see all disk stores that are
> being waited on by other members.
> {noformat}
> After looking at the logs for all members, the "members with potentially new
> data" for each member were found to be:
> {noformat}
> Member | Members with potentially new data
> --------+----------------------------------
> 1 | all
> 2 | 4
> 3 | 4
> 4 | 10
> 5 | 2, 3, 4, 8, 10
> 6 | 2, 3, 4, 5, 7, 8, 10
> 7 | 3, 4, 10
> 8 | 3, 4, 10
> 9 | 2, 3, 4, 5, 7, 8, 10
> 10 | 3
> {noformat}
> It appears that there is a cycle in this "waiting for another online member"
> graph between 3 > 4 > 10 > 3.
> The problem seems to have cropped up after the fix for GEODE-7196 was merged.
> That changed the timing of member-departed notifications such that a server
> might close a Region's Persistence Advisor before getting notification that
> another server was shutting down. We used to do this notification upon
> receipt of a ShutdownMessage but now we only do it when the membership view
> has changed.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
