Skip to content

Configurable HTTP/2 PING ACK timeout #662

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.

Sign up for GitHub

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

Open
arturobernalg wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Configurable HTTP/2 PING ACK timeout #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 31 additions & 3 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.http2.frame.FrameConsts;
import org.apache.hc.core5.util.Args;
import org.apache.hc.core5.util.Timeout;

/**
* HTTP/2 protocol configuration.
Expand All @@ -51,10 +52,12 @@ public class H2Config {
private final int maxHeaderListSize;
private final boolean compressionEnabled;
private final int maxContinuations;
private final Timeout pingAckTimeout;

H2Config(final int headerTableSize, final boolean pushEnabled, final int maxConcurrentStreams,
final int initialWindowSize, final int maxFrameSize, final int maxHeaderListSize,
final boolean compressionEnabled, final int maxContinuations) {
final boolean compressionEnabled, final int maxContinuations,
final Timeout pingAckTimeout) {
super();
this.headerTableSize = headerTableSize;
this.pushEnabled = pushEnabled;
Expand All @@ -64,6 +67,7 @@ public class H2Config {
this.maxHeaderListSize = maxHeaderListSize;
this.compressionEnabled = compressionEnabled;
this.maxContinuations = maxContinuations;
this.pingAckTimeout = pingAckTimeout;
}

public int getHeaderTableSize() {
Expand Down Expand Up @@ -98,6 +102,15 @@ public int getMaxContinuations() {
return maxContinuations;
}

/**
* Returns the timeout for waiting for a PING ACK during idle connection validation.
*
* @since 5.5
*/
public Timeout getPingAckTimeout() {
return pingAckTimeout;
}

@Override
public String toString() {
final StringBuilder builder = new StringBuilder();
Expand All @@ -109,6 +122,7 @@ public String toString() {
.append(", maxHeaderListSize=").append(this.maxHeaderListSize)
.append(", compressionEnabled=").append(this.compressionEnabled)
.append(", maxContinuations=").append(this.maxContinuations)
.append(", pingAckTimeout=").append(this.pingAckTimeout)
.append("]");
return builder.toString();
}
Expand Down Expand Up @@ -142,7 +156,8 @@ public static H2Config.Builder copy(final H2Config config) {
.setInitialWindowSize(config.getInitialWindowSize())
.setMaxFrameSize(config.getMaxFrameSize())
.setMaxHeaderListSize(config.getMaxHeaderListSize())
.setCompressionEnabled(config.isCompressionEnabled());
.setCompressionEnabled(config.isCompressionEnabled())
.setPingAckTimeout(config.getPingAckTimeout());
}

public static class Builder {
Expand All @@ -155,6 +170,7 @@ public static class Builder {
private int maxHeaderListSize;
private boolean compressionEnabled;
private int maxContinuations;
private Timeout pingAckTimeout;

Builder() {
this.headerTableSize = INIT_HEADER_TABLE_SIZE * 2;
Expand All @@ -165,6 +181,7 @@ public static class Builder {
this.maxHeaderListSize = FrameConsts.MAX_FRAME_SIZE;
this.compressionEnabled = true;
this.maxContinuations = 100;
this.pingAckTimeout = Timeout.ofSeconds(5);
}

public Builder setHeaderTableSize(final int headerTableSize) {
Expand Down Expand Up @@ -216,6 +233,16 @@ public Builder setMaxContinuations(final int maxContinuations) {
return this;
}

/**
* Sets the timeout for waiting for a PING ACK during idle connection validation.
*
* @since 5.5
*/
public Builder setPingAckTimeout(final Timeout pingAckTimeout) {
this.pingAckTimeout = Args.notNull(pingAckTimeout, "PING ACK timeout");
return this;
}

public H2Config build() {
return new H2Config(
headerTableSize,
Expand All @@ -225,7 +252,8 @@ public H2Config build() {
maxFrameSize,
maxHeaderListSize,
compressionEnabled,
maxContinuations);
maxContinuations,
pingAckTimeout);
}

}
Expand Down
2 changes: 1 addition & 1 deletion ...ore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ public final void onOutput() throws HttpException, IOException {
final boolean hasBeenIdleTooLong = t > 0 && System.currentTimeMillis() - lastActivityTime > t;
if (hasBeenIdleTooLong && ioSession.hasCommands() && pingHandlers.isEmpty()) {
final Timeout socketTimeout = ioSession.getSocketTimeout();
ioSession.setSocketTimeout(Timeout.ofSeconds(5));
ioSession.setSocketTimeout(localConfig.getPingAckTimeout());
executePing(new PingCommand(new BasicPingHandler(result -> {
// restore timeout
ioSession.setSocketTimeout(socketTimeout);
Expand Down
11 changes: 7 additions & 4 deletions httpcore5-h2/src/test/java/org/apache/hc/core5/http2/config/H2ConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,46 +32,48 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.apache.hc.core5.util.Timeout;
import org.junit.jupiter.api.Test;

class H2ConfigTest {

@Test
void builder() {
// Create and start requester
final H2Config h2Config = H2Config.custom()
.setPushEnabled(false)
.build();
assertNotNull(h2Config);
assertEquals(Timeout.ofSeconds(5), h2Config.getPingAckTimeout());
}

@Test
void checkValues() {
// Create and start requester
final H2Config h2Config = H2Config.custom()
.setHeaderTableSize(1)
.setMaxConcurrentStreams(1)
.setMaxFrameSize(16384)
.setPushEnabled(true)
.setCompressionEnabled(true)
.setPingAckTimeout(Timeout.ofSeconds(10))
.build();

assertEquals(1, h2Config.getHeaderTableSize());
assertEquals(1, h2Config.getMaxConcurrentStreams());
assertEquals(16384, h2Config.getMaxFrameSize());
assertTrue(h2Config.isPushEnabled());
assertTrue(h2Config.isCompressionEnabled());
assertEquals(Timeout.ofSeconds(10), h2Config.getPingAckTimeout());
}

@Test
void copy() {
// Create and start requester
final H2Config h2Config = H2Config.custom()
.setHeaderTableSize(1)
.setMaxConcurrentStreams(1)
.setMaxFrameSize(16384)
.setPushEnabled(true)
.setCompressionEnabled(true)
.setPingAckTimeout(Timeout.ofSeconds(15))
.build();

final H2Config.Builder builder = H2Config.copy(h2Config);
Expand All @@ -82,7 +84,8 @@ void copy() {
() -> assertEquals(h2Config.getInitialWindowSize(), h2Config2.getInitialWindowSize()),
() -> assertEquals(h2Config.getMaxConcurrentStreams(), h2Config2.getMaxConcurrentStreams()),
() -> assertEquals(h2Config.getMaxFrameSize(), h2Config2.getMaxFrameSize()),
() -> assertEquals(h2Config.getMaxHeaderListSize(), h2Config2.getMaxHeaderListSize())
() -> assertEquals(h2Config.getMaxHeaderListSize(), h2Config2.getMaxHeaderListSize()),
() -> assertEquals(h2Config.getPingAckTimeout(), h2Config2.getPingAckTimeout())
);

}
Expand Down
44 changes: 44 additions & 0 deletions ...-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,50 @@ void testValidateAfterInactivityPingAckRestoresPreviousTimeout() throws Exceptio
Mockito.verify(protocolIOSession, Mockito.atLeastOnce()).setSocketTimeout(ArgumentMatchers.eq(previousTimeout));
}

@Test
void testValidateAfterInactivityUsesConfiguredPingAckTimeout() throws Exception {
final List<byte[]> writes = new ArrayList<>();
Mockito.when(protocolIOSession.write(ArgumentMatchers.any(ByteBuffer.class)))
.thenAnswer(inv -> {
final ByteBuffer b = inv.getArgument(0, ByteBuffer.class);
final byte[] copy = new byte[b.remaining()];
b.get(copy);
writes.add(copy);
return copy.length;
});
Mockito.doNothing().when(protocolIOSession).setEvent(ArgumentMatchers.anyInt());
Mockito.doNothing().when(protocolIOSession).clearEvent(ArgumentMatchers.anyInt());

Mockito.when(protocolIOSession.hasCommands()).thenReturn(true);
Mockito.when(protocolIOSession.getSocketTimeout()).thenReturn(Timeout.ofSeconds(30));

final Timeout customPingAckTimeout = Timeout.ofSeconds(15);
final H2Config h2Config = H2Config.custom()
.setPingAckTimeout(customPingAckTimeout)
.build();
final Timeout validateAfterInactivity = Timeout.ofMilliseconds(1);

final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession, FRAME_FACTORY, StreamIdGenerator.ODD,
httpProcessor, CharCodingConfig.DEFAULT, h2Config, h2StreamListener, () -> streamHandler,
validateAfterInactivity);

mux.onConnect();
completeSettingsHandshake(mux);

writes.clear();
makeMuxIdle(mux, validateAfterInactivity);

mux.onOutput();

Mockito.verify(protocolIOSession, Mockito.atLeastOnce())
.setSocketTimeout(ArgumentMatchers.eq(customPingAckTimeout));

final List<FrameStub> frames = parseFrames(concat(writes));
Assertions.assertTrue(frames.stream().anyMatch(f -> f.isPing() && !f.isAck()),
"Must emit pre-flight PING");
}

@Test
void testKeepAliveAckTimeoutShutsDownAndFailsStreams() throws Exception {
final List<byte[]> writes = new ArrayList<>();
Expand Down
Loading