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

Schedule transport ping interval #10189

Merged
merged 1 commit into from Mar 21, 2015
Merged

Conversation

kimchy
Copy link
Member

@kimchy kimchy commented Mar 20, 2015

Sometimes, when using transport client for example, through a load balancer, there is a need to send a scheduled ping message to keep each channel alive.

Add support for transport.ping_schedule, which controls the schedule (-1 for disabled) at which a ping message will be sent. For transport client case, it gets enabled automatically since almost always this is the desired behavior.

We use the same 6 bytes header format for the ping message, with ES header and -1 for data length for ping message, and simply continue to process the next messages once this is encountered.

if (lifecycle.stoppedOrClosed()) {
return;
}
for (Map.Entry<DiscoveryNode, NodeChannels> entry : connectedNodes.entrySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you assign the key and the value here before we use it? it's way easier to read

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

@s1monw
Copy link
Contributor

s1monw commented Mar 20, 2015

I left some comments

@s1monw s1monw self-assigned this Mar 20, 2015
@kimchy
Copy link
Member Author

kimchy commented Mar 20, 2015

@s1monw addressed your comments, ready for another round

DiscoveryNode node = entry.getKey();
NodeChannels channels = entry.getValue();
// we only support the ping message format since 1.6
if (node.version().onOrAfter(Version.V_1_6_0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please make sure you remove the conditional on master

@s1monw
Copy link
Contributor

s1monw commented Mar 20, 2015

left one comment LGTM otherwise

Sometimes, when using transport client for example, through a load balancer, there is a need to send a scheduled ping message to keep each channel alive.

Add support for `transport.ping_schedule`, which controls the schedule (-1 for disabled) at which a ping message will be sent. For transport client case, it gets enabled automatically since almost always this is the desired behavior.

We use the same 6 bytes header format for the ping message, with ES header and -1 for data length for ping message, and simply continue to process the next messages once this is encountered.

closes elastic#10189
@kimchy kimchy merged commit 0cc13b9 into elastic:master Mar 21, 2015
kimchy added a commit that referenced this pull request Mar 21, 2015
Sometimes, when using transport client for example, through a load balancer, there is a need to send a scheduled ping message to keep each channel alive.

Add support for `transport.ping_schedule`, which controls the schedule (-1 for disabled) at which a ping message will be sent. For transport client case, it gets enabled automatically since almost always this is the desired behavior.

We use the same 6 bytes header format for the ping message, with ES header and -1 for data length for ping message, and simply continue to process the next messages once this is encountered.

closes #10189
@kimchy kimchy deleted the transport_ping branch March 21, 2015 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Network Http and internode communication implementations >enhancement v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants