Skip to content

Commit

Permalink
Make keepalive pings bidirectional and optimizable (elastic#35441)
Browse files Browse the repository at this point in the history
This is related to elastic#34405 and a follow-up to elastic#34753. It makes a number
of changes to our current keepalive pings.

The ping interval configuration is moved to the ConnectionProfile.

The server channel now responds to pings. This makes the keepalive
pings bidirectional.

On the client-side, the pings can now be optimized away. What this
means is that if the channel has received a message or sent a message
since the last pinging round, the ping is not sent for this round.
  • Loading branch information
Tim-Brooks committed Nov 29, 2018
1 parent 8d58334 commit ecf4503
Show file tree
Hide file tree
Showing 23 changed files with 918 additions and 474 deletions.
Expand Up @@ -35,12 +35,15 @@
public class Netty4TcpChannel implements TcpChannel {

private final Channel channel;
private final boolean isServer;
private final String profile;
private final CompletableContext<Void> connectContext;
private final CompletableContext<Void> closeContext = new CompletableContext<>();
private final ChannelStats stats = new ChannelStats();

Netty4TcpChannel(Channel channel, String profile, @Nullable ChannelFuture connectFuture) {
Netty4TcpChannel(Channel channel, boolean isServer, String profile, @Nullable ChannelFuture connectFuture) {
this.channel = channel;
this.isServer = isServer;
this.profile = profile;
this.connectContext = new CompletableContext<>();
this.channel.closeFuture().addListener(f -> {
Expand Down Expand Up @@ -77,6 +80,11 @@ public void close() {
channel.close();
}

@Override
public boolean isServerChannel() {
return isServer;
}

@Override
public String getProfile() {
return profile;
Expand All @@ -92,6 +100,11 @@ public void addConnectListener(ActionListener<Void> listener) {
connectContext.addListener(ActionListener.toBiConsumer(listener));
}

@Override
public ChannelStats getChannelStats() {
return stats;
}

@Override
public boolean isOpen() {
return channel.isOpen();
Expand Down
Expand Up @@ -232,7 +232,7 @@ protected Netty4TcpChannel initiateChannel(DiscoveryNode node) throws IOExceptio
}
addClosedExceptionLogger(channel);

Netty4TcpChannel nettyChannel = new Netty4TcpChannel(channel, "default", connectFuture);
Netty4TcpChannel nettyChannel = new Netty4TcpChannel(channel, false, "default", connectFuture);
channel.attr(CHANNEL_KEY).set(nettyChannel);

return nettyChannel;
Expand All @@ -246,14 +246,6 @@ protected Netty4TcpServerChannel bind(String name, InetSocketAddress address) {
return esChannel;
}

long successfulPingCount() {
return successfulPings.count();
}

long failedPingCount() {
return failedPings.count();
}

@Override
@SuppressForbidden(reason = "debug")
protected void stopInternal() {
Expand Down Expand Up @@ -297,8 +289,7 @@ protected ServerChannelInitializer(String name) {
@Override
protected void initChannel(Channel ch) throws Exception {
addClosedExceptionLogger(ch);
Netty4TcpChannel nettyTcpChannel = new Netty4TcpChannel(ch, name, ch.newSucceededFuture());

Netty4TcpChannel nettyTcpChannel = new Netty4TcpChannel(ch, true, name, ch.newSucceededFuture());
ch.attr(CHANNEL_KEY).set(nettyTcpChannel);
serverAcceptedChannel(nettyTcpChannel);
ch.pipeline().addLast("logging", new ESLoggingHandler());
Expand Down

This file was deleted.

29 changes: 29 additions & 0 deletions server/src/main/java/org/elasticsearch/common/AsyncBiFunction.java
@@ -0,0 +1,29 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.common;

import org.elasticsearch.action.ActionListener;

/**
* A {@link java.util.function.BiFunction}-like interface designed to be used with asynchronous executions.
*/
public interface AsyncBiFunction<T,U,C> {

void apply(T t, U u, ActionListener<C> listener);
}
Expand Up @@ -27,10 +27,7 @@
import org.elasticsearch.common.component.Lifecycle;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.AbstractLifecycleRunnable;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
import org.elasticsearch.common.util.concurrent.KeyedLock;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -53,32 +50,28 @@
* the connection when the connection manager is closed.
*/
public class ConnectionManager implements Closeable {

private static final Logger logger = LogManager.getLogger(ConnectionManager.class);

private final ConcurrentMap<DiscoveryNode, Transport.Connection> connectedNodes = ConcurrentCollections.newConcurrentMap();
private final KeyedLock<String> connectionLock = new KeyedLock<>();
private final Transport transport;
private final ThreadPool threadPool;
private final TimeValue pingSchedule;
private final ConnectionProfile defaultProfile;
private final Lifecycle lifecycle = new Lifecycle();
private final AtomicBoolean closed = new AtomicBoolean(false);
private final ReadWriteLock closeLock = new ReentrantReadWriteLock();
private final DelegatingNodeConnectionListener connectionListener = new DelegatingNodeConnectionListener();

public ConnectionManager(Settings settings, Transport transport, ThreadPool threadPool) {
this(settings, transport, threadPool, TcpTransport.PING_SCHEDULE.get(settings));
this(ConnectionProfile.buildDefaultConnectionProfile(settings), transport, threadPool);
}

public ConnectionManager(Settings settings, Transport transport, ThreadPool threadPool, TimeValue pingSchedule) {
public ConnectionManager(ConnectionProfile connectionProfile, Transport transport, ThreadPool threadPool) {
this.transport = transport;
this.threadPool = threadPool;
this.pingSchedule = pingSchedule;
this.defaultProfile = ConnectionProfile.buildDefaultConnectionProfile(settings);
this.defaultProfile = connectionProfile;
this.lifecycle.moveToStarted();
if (pingSchedule.millis() > 0) {
threadPool.schedule(pingSchedule, ThreadPool.Names.GENERIC, new ScheduledPing());
}
}

public void addListener(TransportConnectionListener listener) {
Expand Down Expand Up @@ -251,47 +244,8 @@ private void ensureOpen() {
}
}

TimeValue getPingSchedule() {
return pingSchedule;
}

private class ScheduledPing extends AbstractLifecycleRunnable {

private ScheduledPing() {
super(lifecycle, logger);
}

@Override
protected void doRunInLifecycle() {
for (Map.Entry<DiscoveryNode, Transport.Connection> entry : connectedNodes.entrySet()) {
Transport.Connection connection = entry.getValue();
if (connection.sendPing() == false) {
logger.warn("attempted to send ping to connection without support for pings [{}]", connection);
}
}
}

@Override
protected void onAfterInLifecycle() {
try {
threadPool.schedule(pingSchedule, ThreadPool.Names.GENERIC, this);
} catch (EsRejectedExecutionException ex) {
if (ex.isExecutorShutdown()) {
logger.debug("couldn't schedule new ping execution, executor is shutting down", ex);
} else {
throw ex;
}
}
}

@Override
public void onFailure(Exception e) {
if (lifecycle.stoppedOrClosed()) {
logger.trace("failed to send ping transport message", e);
} else {
logger.warn("failed to send ping transport message", e);
}
}
ConnectionProfile getConnectionProfile() {
return defaultProfile;
}

private static final class DelegatingNodeConnectionListener implements TransportConnectionListener {
Expand Down

0 comments on commit ecf4503

Please sign in to comment.