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

Close idle channels instead of sending PING frames #1048

Merged
merged 1 commit into from
Jul 6, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -116,12 +116,10 @@ protected void initChannel(final SocketChannel channel) {
clientHandlerBuilder = new TokenAuthenticationApnsClientHandler.TokenAuthenticationApnsClientHandlerBuilder()
.signingKey(clientConfiguration.getSigningKey().get())
.tokenExpiration(clientConfiguration.getTokenExpiration())
.authority(authority)
.idlePingInterval(clientConfiguration.getIdlePingInterval());
.authority(authority);
} else {
clientHandlerBuilder = new ApnsClientHandler.ApnsClientHandlerBuilder()
.authority(authority)
.idlePingInterval(clientConfiguration.getIdlePingInterval());
.authority(authority);
}

clientConfiguration.getFrameLogger().ifPresent(clientHandlerBuilder::frameLogger);
@@ -139,7 +137,7 @@ protected void initChannel(final SocketChannel channel) {

pipeline.addLast(sslHandler);
pipeline.addLast(new FlushConsolidationHandler(FlushConsolidationHandler.DEFAULT_EXPLICIT_FLUSH_AFTER_FLUSHES, true));
pipeline.addLast(new IdleStateHandler(clientConfiguration.getIdlePingInterval().toMillis(), 0, 0, TimeUnit.MILLISECONDS));
pipeline.addLast(new IdleStateHandler(clientConfiguration.getCloseAfterIdleDuration().toMillis(), 0, 0, TimeUnit.MILLISECONDS));
pipeline.addLast(apnsClientHandler);
}
});
Original file line number Diff line number Diff line change
@@ -79,19 +79,19 @@ public class ApnsClientBuilder {
private ProxyHandlerFactory proxyHandlerFactory;

private Duration connectionTimeout;
private Duration idlePingInterval = DEFAULT_IDLE_PING_INTERVAL;
private Duration closeAfterIdleDuration = DEFAULT_CLOSE_AFTER_IDLE_DURATION;
private Duration gracefulShutdownTimeout;

private Http2FrameLogger frameLogger;

private boolean useAlpn = false;

/**
* The default idle time in milliseconds after which the client will send a PING frame to the APNs server.
* The default idle time after which the client will close a connection (which may be reopened later).
*
* @since 0.11
*/
public static final Duration DEFAULT_IDLE_PING_INTERVAL = Duration.ofMinutes(1);
public static final Duration DEFAULT_CLOSE_AFTER_IDLE_DURATION = Duration.ofMinutes(1);

/**
* The hostname for the production APNs gateway.
@@ -476,18 +476,18 @@ public ApnsClientBuilder setConnectionTimeout(final Duration timeout) {
}

/**
* Sets the amount of idle time (in milliseconds) after which the client under construction will send a PING frame
* to the APNs server. By default, clients will send a PING frame after an idle period of
* {@link com.eatthepath.pushy.apns.ApnsClientBuilder#DEFAULT_IDLE_PING_INTERVAL}.
* Sets the amount of idle time after which the client under construction will close a connection (which may be
* reopened later). By default, clients will close connections after an idle time of
* {@link com.eatthepath.pushy.apns.ApnsClientBuilder#DEFAULT_CLOSE_AFTER_IDLE_DURATION}.
*
* @param idlePingInterval the amount of idle time after which the client will send a PING frame
* @param closeAfterIdleDuration the amount of idle time after which the client will close a connection
*
* @return a reference to this builder
*
* @since 0.10
* @since 0.16.0
*/
public ApnsClientBuilder setIdlePingInterval(final Duration idlePingInterval) {
this.idlePingInterval = idlePingInterval;
public ApnsClientBuilder setCloseAfterIdleDuration(final Duration closeAfterIdleDuration) {
this.closeAfterIdleDuration = closeAfterIdleDuration;
return this;
}

@@ -628,7 +628,7 @@ public ApnsClient build() throws SSLException {
this.tokenExpiration,
this.proxyHandlerFactory,
this.connectionTimeout,
this.idlePingInterval,
this.closeAfterIdleDuration,
this.gracefulShutdownTimeout,
this.concurrentConnections,
this.metricsListener,
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ class ApnsClientConfiguration {
private final Duration tokenExpiration;
private final ProxyHandlerFactory proxyHandlerFactory;
private final Duration connectionTimeout;
private final Duration idlePingInterval;
private final Duration closeAfterIdleDuration;
private final Duration gracefulShutdownTimeout;
private final int concurrentConnections;
private final ApnsClientMetricsListener metricsListener;
@@ -59,7 +59,7 @@ public ApnsClientConfiguration(final InetSocketAddress apnsServerAddress,
final Duration tokenExpiration,
final ProxyHandlerFactory proxyHandlerFactory,
final Duration connectionTimeout,
final Duration idlePingInterval,
final Duration closeAfterIdleDuration,
final Duration gracefulShutdownTimeout,
final int concurrentConnections,
final ApnsClientMetricsListener metricsListener,
@@ -72,7 +72,7 @@ public ApnsClientConfiguration(final InetSocketAddress apnsServerAddress,
this.tokenExpiration = tokenExpiration != null ? tokenExpiration : DEFAULT_TOKEN_EXPIRATION;
this.proxyHandlerFactory = proxyHandlerFactory;
this.connectionTimeout = connectionTimeout;
this.idlePingInterval = idlePingInterval;
this.closeAfterIdleDuration = closeAfterIdleDuration;
this.gracefulShutdownTimeout = gracefulShutdownTimeout;
this.concurrentConnections = concurrentConnections;
this.metricsListener = metricsListener;
@@ -107,8 +107,8 @@ public Optional<Duration> getConnectionTimeout() {
return Optional.ofNullable(connectionTimeout);
}

public Duration getIdlePingInterval() {
return idlePingInterval;
public Duration getCloseAfterIdleDuration() {
return closeAfterIdleDuration;
}

public Optional<Duration> getGracefulShutdownTimeout() {
Original file line number Diff line number Diff line change
@@ -38,22 +38,18 @@
import io.netty.handler.timeout.IdleStateEvent;
import io.netty.util.AsciiString;
import io.netty.util.collection.IntObjectHashMap;
import io.netty.util.concurrent.GenericFutureListener;
import io.netty.util.concurrent.Promise;
import io.netty.util.concurrent.PromiseCombiner;
import io.netty.util.concurrent.ScheduledFuture;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.time.Duration;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

class ApnsClientHandler extends Http2ConnectionHandler implements Http2FrameListener, Http2Connection.Listener {

@@ -65,9 +61,6 @@ class ApnsClientHandler extends Http2ConnectionHandler implements Http2FrameList

private final String authority;

private final Duration pingTimeout;
private ScheduledFuture<?> pingTimeoutFuture;

private Throwable connectionErrorCause;

private static final AsciiString APNS_PATH_PREFIX = new AsciiString("/3/device/");
@@ -92,7 +85,6 @@ class ApnsClientHandler extends Http2ConnectionHandler implements Http2FrameList
public static class ApnsClientHandlerBuilder extends AbstractHttp2ConnectionHandlerBuilder<ApnsClientHandler, ApnsClientHandlerBuilder> {

private String authority;
private Duration idlePingInterval;

ApnsClientHandlerBuilder authority(final String authority) {
this.authority = authority;
@@ -103,15 +95,6 @@ String authority() {
return this.authority;
}

Duration idlePingInterval() {
return idlePingInterval;
}

ApnsClientHandlerBuilder idlePingInterval(final Duration idlePingIntervalMillis) {
this.idlePingInterval = idlePingIntervalMillis;
return this;
}

@Override
public ApnsClientHandlerBuilder frameLogger(final Http2FrameLogger frameLogger) {
return super.frameLogger(frameLogger);
@@ -136,7 +119,7 @@ protected boolean encoderEnforceMaxConcurrentStreams() {
public ApnsClientHandler build(final Http2ConnectionDecoder decoder, final Http2ConnectionEncoder encoder, final Http2Settings initialSettings) {
Objects.requireNonNull(this.authority(), "Authority must be set before building an ApnsClientHandler.");

final ApnsClientHandler handler = new ApnsClientHandler(decoder, encoder, initialSettings, this.authority(), this.idlePingInterval());
final ApnsClientHandler handler = new ApnsClientHandler(decoder, encoder, initialSettings, this.authority());
this.frameListener(handler);
return handler;
}
@@ -147,7 +130,7 @@ public ApnsClientHandler build() {
}
}

ApnsClientHandler(final Http2ConnectionDecoder decoder, final Http2ConnectionEncoder encoder, final Http2Settings initialSettings, final String authority, final Duration idlePingInterval) {
ApnsClientHandler(final Http2ConnectionDecoder decoder, final Http2ConnectionEncoder encoder, final Http2Settings initialSettings, final String authority) {
super(decoder, encoder, initialSettings);

this.authority = authority;
@@ -157,8 +140,6 @@ public ApnsClientHandler build() {
this.streamErrorCausePropertyKey = this.connection().newKey();

this.connection().addListener(this);

this.pingTimeout = idlePingInterval.dividedBy(2);
}

@Override
@@ -264,22 +245,8 @@ protected Http2Headers getHeadersForPushNotification(final ApnsPushNotification
@Override
public void userEventTriggered(final ChannelHandlerContext context, final Object event) throws Exception {
if (event instanceof IdleStateEvent) {
log.trace("Sending ping due to inactivity.");

this.encoder().writePing(context, false, System.currentTimeMillis(), context.newPromise()).addListener(
(GenericFutureListener<ChannelFuture>) future -> {
if (!future.isSuccess()) {
log.debug("Failed to write PING frame.", future.cause());
future.channel().close();
}
});

this.pingTimeoutFuture = context.channel().eventLoop().schedule(() -> {
log.debug("Closing channel due to ping timeout.");
context.channel().close();
}, pingTimeout.toMillis(), TimeUnit.MILLISECONDS);

this.flush(context);
log.debug("Closing idle channel.");
context.close();
}

super.userEventTriggered(context, event);
@@ -413,11 +380,6 @@ public void onPingRead(final ChannelHandlerContext ctx, final long pingData) {

@Override
public void onPingAckRead(final ChannelHandlerContext context, final long pingData) {
if (this.pingTimeoutFuture != null) {
this.pingTimeoutFuture.cancel(false);
} else {
log.error("Received PING ACK, but no corresponding outbound PING found.");
}
}

@Override
@@ -508,10 +470,6 @@ protected void onConnectionError(final ChannelHandlerContext context, final bool

@Override
public void channelInactive(final ChannelHandlerContext context) throws Exception {
if (this.pingTimeoutFuture != null) {
this.pingTimeoutFuture.cancel(false);
}

for (final PushNotificationFuture<?, ?> future : this.unattachedResponsePromisesByStreamId.values()) {
future.completeExceptionally(STREAM_CLOSED_BEFORE_REPLY_EXCEPTION);
}
Original file line number Diff line number Diff line change
@@ -81,14 +81,14 @@ public ApnsClientHandler build(final Http2ConnectionDecoder decoder, final Http2
Objects.requireNonNull(this.signingKey(), "Signing key must be set before building a TokenAuthenticationApnsClientHandler.");
Objects.requireNonNull(this.tokenExpiration(), "Token expiration duration must be set before building a TokenAuthenticationApnsClientHandler.");

final ApnsClientHandler handler = new TokenAuthenticationApnsClientHandler(decoder, encoder, initialSettings, this.authority(), this.idlePingInterval(), this.signingKey(), this.tokenExpiration());
final ApnsClientHandler handler = new TokenAuthenticationApnsClientHandler(decoder, encoder, initialSettings, this.authority(), this.signingKey(), this.tokenExpiration());
this.frameListener(handler);
return handler;
}
}

protected TokenAuthenticationApnsClientHandler(final Http2ConnectionDecoder decoder, final Http2ConnectionEncoder encoder, final Http2Settings initialSettings, final String authority, final Duration idlePingInterval, final ApnsSigningKey signingKey, final Duration tokenExpiration) {
super(decoder, encoder, initialSettings, authority, idlePingInterval);
protected TokenAuthenticationApnsClientHandler(final Http2ConnectionDecoder decoder, final Http2ConnectionEncoder encoder, final Http2Settings initialSettings, final String authority, final ApnsSigningKey signingKey, final Duration tokenExpiration) {
super(decoder, encoder, initialSettings, authority);

this.signingKey = Objects.requireNonNull(signingKey, "Signing key must not be null for token-based client handlers.");
this.tokenExpiration = Objects.requireNonNull(tokenExpiration, "Token expiration must not be null for token-based client handlers");