Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ConnectionTransportException during a Master/Node fault detection ping during Discovery #6686

Closed
wants to merge 1 commit into from

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Jul 2, 2014

Both the Master and Node fault detection register themselves to be notified when a node disconnects to be able to respond to it accordingly. As such, when a ConnectionTransportException was raised on a ping request, it was not handled as it is already handled somewhere else. However, this does introduce a racing condition, if the disconnect happen during a period where there is no current master (minimum_master_node breach) at which time the fault detection is not active. In this case, we will only discover the disconnect error during the ping request, so we have to respond accordingly.

…t detection ping

Both the Master and Node fault detection register themselves to be notified when a node disconnects to be able to respond to it accordingly. As such, when a ConnectionTransportException was raised on a ping request, it was not handled it as it is already handled somewhere else. However, this does introduce a racing condition, if the disconnect  happen during a period where there is no current master (minimum_master_node breach) at which time the fault detection is not active. This case we will only discover the disconnect error during the ping request, so we have to respond accordingly.
NodeFD nodeFD = nodesFD.get(node);
if (nodeFD != null) {
if (!nodeFD.running) {
return;
}
if (exp instanceof ConnectTransportException) {
// ignore this one, we already handle it by registering a connection listener
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this change, this comment is obsolete.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. left overs. will remove.

@martijnvg
Copy link
Member

Left one small comment, other than that LGTM.

@kimchy
Copy link
Member

kimchy commented Jul 2, 2014

same here, LGTM

@bleskes bleskes changed the title [Infra] Handle ConnectionTransportException during a Master/Node fault detection ping [Discovery] Handle ConnectionTransportException during a Master/Node fault detection ping Jul 2, 2014
@bleskes bleskes closed this in 8909a77 Jul 2, 2014
bleskes added a commit that referenced this pull request Jul 2, 2014
…fault detection ping

Both the Master and Node fault detection register themselves to be notified when a node disconnects to be able to respond to it accordingly. As such, when a ConnectionTransportException was raised on a ping request, it was not handled as it is already handled somewhere else. However, this does introduce a racing condition, if the disconnect  happen during a period where there is no current master (minimum_master_node breach) at which time the fault detection is not active. In this case, we will only discover the disconnect error during the ping request, so we have to respond accordingly.

Closes #6686
@bleskes bleskes deleted the ping_disconnect_failure branch July 2, 2014 18:55
@bleskes bleskes removed the review label Jul 2, 2014
bleskes added a commit that referenced this pull request Jul 3, 2014
…sconnects

The change introduced in #6686 checks for ConnectionTransportException during pinging. However, transport layer wraps it in  SendRequestTransportException
bleskes added a commit that referenced this pull request Jul 3, 2014
…sconnects

The change introduced in #6686 checks for ConnectionTransportException during pinging. However, transport layer wraps it in  SendRequestTransportException
@clintongormley clintongormley changed the title [Discovery] Handle ConnectionTransportException during a Master/Node fault detection ping Resiliency: Handle ConnectionTransportException during a Master/Node fault detection ping during Discovery Jul 16, 2014
@clintongormley clintongormley added the :Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure label Jun 7, 2015
@clintongormley clintongormley changed the title Resiliency: Handle ConnectionTransportException during a Master/Node fault detection ping during Discovery Handle ConnectionTransportException during a Master/Node fault detection ping during Discovery Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure resiliency v1.3.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants