Allow to disable automatically sending PING acks. (#9338)

Motivation:

There are situations where the user may want to be more flexible when to send the PING acks for various reasons or be able to attach a listener to the future that is used for the ping ack. To be able to do so we should allow to manage the acks manually.

Modifications:

- Add constructor to DefaultHttp2ConnectionDecoder that allows to disable the automatically sending of ping acks (default is to send automatically to not break users)
- Add methods to AbstractHttp2ConnectionHandlerBuilder (and sub-classes) to either enable ot disable auto acks for pings
- Make DefaultHttp2PingFrame constructor public that allows to write acks.
- Add unit test

Result:

More flexible way of handling acks.
This commit is contained in:
Norman Maurer 2019-07-12 18:15:06 +02:00 committed by GitHub
parent 5384bbcf85
commit 906fc02b3f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 87 additions and 11 deletions

View File

@ -106,6 +106,7 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder<T extends Http2Conne
private Boolean encoderIgnoreMaxHeaderListSize;
private Http2PromisedRequestVerifier promisedRequestVerifier = ALWAYS_VERIFY;
private boolean autoAckSettingsFrame = true;
private boolean autoAckPingFrame = true;
/**
* Sets the {@link Http2Settings} to use for the initial connection settings exchange.
@ -399,6 +400,24 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder<T extends Http2Conne
return autoAckSettingsFrame;
}
/**
* Determine if PING frame should automatically be acknowledged or not.
* @return this.
*/
protected B autoAckPingFrame(boolean autoAckPingFrame) {
enforceNonCodecConstraints("autoAckPingFrame");
this.autoAckPingFrame = autoAckPingFrame;
return self();
}
/**
* Determine if the PING frames should be automatically acknowledged or not.
* @return {@code true} if the PING frames should be automatically acknowledged.
*/
protected boolean isAutoAckPingFrame() {
return autoAckPingFrame;
}
/**
* Determine if the {@link Channel#close()} should be coupled with goaway and graceful close.
* @param decoupleCloseAndGoAway {@code true} to make {@link Channel#close()} directly close the underlying
@ -463,7 +482,7 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder<T extends Http2Conne
}
DefaultHttp2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, reader,
promisedRequestVerifier(), isAutoAckSettingsFrame());
promisedRequestVerifier(), isAutoAckSettingsFrame(), isAutoAckPingFrame());
return buildFromCodec(decoder, encoder);
}

View File

@ -58,6 +58,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
private Http2FrameListener listener;
private final Http2PromisedRequestVerifier requestVerifier;
private final Http2SettingsReceivedConsumer settingsReceivedConsumer;
private final boolean autoAckPing;
public DefaultHttp2ConnectionDecoder(Http2Connection connection,
Http2ConnectionEncoder encoder,
@ -89,6 +90,31 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
Http2FrameReader frameReader,
Http2PromisedRequestVerifier requestVerifier,
boolean autoAckSettings) {
this(connection, encoder, frameReader, requestVerifier, autoAckSettings, true);
}
/**
* Create a new instance.
* @param connection The {@link Http2Connection} associated with this decoder.
* @param encoder The {@link Http2ConnectionEncoder} associated with this decoder.
* @param frameReader Responsible for reading/parsing the raw frames. As opposed to this object which applies
* h2 semantics on top of the frames.
* @param requestVerifier Determines if push promised streams are valid.
* @param autoAckSettings {@code false} to disable automatically applying and sending settings acknowledge frame.
* The {@code Http2ConnectionEncoder} is expected to be an instance of
* {@link Http2SettingsReceivedConsumer} and will apply the earliest received but not yet
* ACKed SETTINGS when writing the SETTINGS ACKs. {@code true} to enable automatically
* applying and sending settings acknowledge frame.
* @param autoAckPing {@code false} to disable automatically sending ping acknowledge frame. {@code true} to enable
* automatically sending ping ack frame.
*/
public DefaultHttp2ConnectionDecoder(Http2Connection connection,
Http2ConnectionEncoder encoder,
Http2FrameReader frameReader,
Http2PromisedRequestVerifier requestVerifier,
boolean autoAckSettings,
boolean autoAckPing) {
this.autoAckPing = autoAckPing;
if (autoAckSettings) {
settingsReceivedConsumer = null;
} else {
@ -455,10 +481,10 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
@Override
public void onPingRead(ChannelHandlerContext ctx, long data) throws Http2Exception {
// Send an ack back to the remote client.
// Need to retain the buffer here since it will be released after the write completes.
encoder.writePing(ctx, true, data, ctx.newPromise());
if (autoAckPing) {
// Send an ack back to the remote client.
encoder.writePing(ctx, true, data, ctx.newPromise());
}
listener.onPingRead(ctx, data);
}

View File

@ -32,10 +32,7 @@ public class DefaultHttp2PingFrame implements Http2PingFrame {
this(content, false);
}
/**
* A user cannot send a ping ack, as this is done automatically when a ping is received.
*/
DefaultHttp2PingFrame(long content, boolean ack) {
public DefaultHttp2PingFrame(long content, boolean ack) {
this.content = content;
this.ack = ack;
}

View File

@ -147,6 +147,11 @@ public class Http2FrameCodecBuilder extends
return super.autoAckSettingsFrame(autoAckSettings);
}
@Override
public Http2FrameCodecBuilder autoAckPingFrame(boolean autoAckPingFrame) {
return super.autoAckPingFrame(autoAckPingFrame);
}
@Override
public Http2FrameCodecBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) {
return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway);
@ -176,7 +181,7 @@ public class Http2FrameCodecBuilder extends
encoder = new StreamBufferingEncoder(encoder);
}
Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader,
promisedRequestVerifier(), isAutoAckSettingsFrame());
promisedRequestVerifier(), isAutoAckSettingsFrame(), isAutoAckPingFrame());
return build(decoder, encoder, initialSettings());
}

View File

@ -176,6 +176,11 @@ public class Http2MultiplexCodecBuilder
return super.autoAckSettingsFrame(autoAckSettings);
}
@Override
public Http2MultiplexCodecBuilder autoAckPingFrame(boolean autoAckPingFrame) {
return super.autoAckPingFrame(autoAckPingFrame);
}
@Override
public Http2MultiplexCodecBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) {
return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway);
@ -202,7 +207,7 @@ public class Http2MultiplexCodecBuilder
encoder = new StreamBufferingEncoder(encoder);
}
Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader,
promisedRequestVerifier(), isAutoAckSettingsFrame());
promisedRequestVerifier(), isAutoAckSettingsFrame(), isAutoAckPingFrame());
return build(decoder, encoder, initialSettings());
}

View File

@ -759,6 +759,30 @@ public class Http2FrameCodecTest {
assertEquals(expectedStreams, activeStreams);
}
@Test
public void autoAckPingTrue() throws Exception {
setUp(Http2FrameCodecBuilder.forServer().autoAckPingFrame(true), new Http2Settings());
frameInboundWriter.writeInboundPing(false, 8);
Http2PingFrame frame = inboundHandler.readInbound();
assertFalse(frame.ack());
assertEquals(8, frame.content());
verify(frameWriter).writePing(eqFrameCodecCtx(), eq(true), eq(8L), anyChannelPromise());
}
@Test
public void autoAckPingFalse() throws Exception {
setUp(Http2FrameCodecBuilder.forServer().autoAckPingFrame(false), new Http2Settings());
frameInboundWriter.writeInboundPing(false, 8);
verify(frameWriter, never()).writePing(eqFrameCodecCtx(), eq(true), eq(8L), anyChannelPromise());
Http2PingFrame frame = inboundHandler.readInbound();
assertFalse(frame.ack());
assertEquals(8, frame.content());
// Now ack the frame manually.
channel.writeAndFlush(new DefaultHttp2PingFrame(8, true));
verify(frameWriter).writePing(eqFrameCodecCtx(), eq(true), eq(8L), anyChannelPromise());
}
@Test
public void streamShouldBeOpenInListener() {
final Http2FrameStream stream2 = frameCodec.newStream();