From c29ad76daea0d17637fefcab4dbfd9ef8cb84ebc Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Wed, 2 Sep 2020 14:11:34 -0400 Subject: [PATCH] Explain test value for flow control window --- .../test/java/io/grpc/internal/AbstractTransportTest.java | 6 ++++++ .../grpc/testing/integration/NettyClientInteropServlet.java | 3 ++- .../io/grpc/testing/integration/AbstractInteropTest.java | 6 ++++++ .../java/io/grpc/testing/integration/TestServiceClient.java | 2 +- .../testing/integration/Http2NettyLocalChannelTest.java | 4 ++-- .../java/io/grpc/testing/integration/Http2NettyTest.java | 4 ++-- .../java/io/grpc/testing/integration/Http2OkHttpTest.java | 2 +- netty/src/test/java/io/grpc/netty/NettyTransportTest.java | 6 +++--- .../src/test/java/io/grpc/okhttp/OkHttpTransportTest.java | 4 ++-- 9 files changed, 25 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/AbstractTransportTest.java b/core/src/test/java/io/grpc/internal/AbstractTransportTest.java index cca994c105..aa9f269de2 100644 --- a/core/src/test/java/io/grpc/internal/AbstractTransportTest.java +++ b/core/src/test/java/io/grpc/internal/AbstractTransportTest.java @@ -89,6 +89,12 @@ import org.mockito.InOrder; /** Standard unit tests for {@link ClientTransport}s and {@link ServerTransport}s. */ @RunWith(JUnit4.class) public abstract class AbstractTransportTest { + /** + * Use a small flow control to help detect flow control bugs. Don't use 64KiB to test + * SETTINGS/WINDOW_UPDATE exchange. + */ + public static final int TEST_FLOW_CONTROL_WINDOW = 65 * 1024; + private static final int TIMEOUT_MS = 5000; private static final Attributes.Key ADDITIONAL_TRANSPORT_ATTR_KEY = diff --git a/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java b/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java index b42f9aef31..2a9c195841 100644 --- a/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java +++ b/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java @@ -134,7 +134,8 @@ public final class NettyClientInteropServlet extends HttpServlet { ManagedChannelBuilder.forTarget(INTEROP_TEST_ADDRESS) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); assertTrue(builder instanceof NettyChannelBuilder); - ((NettyChannelBuilder) builder).flowControlWindow(65 * 1024); + ((NettyChannelBuilder) builder) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW); return builder; } diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java index 46122d5f25..643d984bad 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java @@ -148,6 +148,12 @@ public abstract class AbstractInteropTest { /** Must be at least {@link #unaryPayloadLength()}, plus some to account for encoding overhead. */ public static final int MAX_MESSAGE_SIZE = 16 * 1024 * 1024; + /** + * Use a small flow control to help detect flow control bugs. Don't use 64KiB to test + * SETTINGS/WINDOW_UPDATE exchange. + */ + public static final int TEST_FLOW_CONTROL_WINDOW = 65 * 1024; + private static final FakeTagger tagger = new FakeTagger(); private static final FakeTagContextBinarySerializer tagContextBinarySerializer = new FakeTagContextBinarySerializer(); diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java index a0a484a64a..ff4c6ca06b 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java @@ -416,7 +416,7 @@ public class TestServiceClient { } NettyChannelBuilder nettyBuilder = NettyChannelBuilder.forAddress(serverHost, serverPort) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .negotiationType(useTls ? NegotiationType.TLS : (useH2cUpgrade ? NegotiationType.PLAINTEXT_UPGRADE : NegotiationType.PLAINTEXT)) .sslContext(sslContext); diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java index 8ed7dc7690..4494d620b1 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java @@ -41,7 +41,7 @@ public class Http2NettyLocalChannelTest extends AbstractInteropTest { protected AbstractServerImplBuilder getServerBuilder() { return NettyServerBuilder .forAddress(new LocalAddress("in-process-1")) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .channelType(LocalServerChannel.class) .workerEventLoopGroup(eventLoopGroup) @@ -55,7 +55,7 @@ public class Http2NettyLocalChannelTest extends AbstractInteropTest { .negotiationType(NegotiationType.PLAINTEXT) .channelType(LocalChannel.class) .eventLoopGroup(eventLoopGroup) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); // Disable the default census stats interceptor, use testing interceptor instead. InternalNettyChannelBuilder.setStatsEnabled(builder, false); diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java index 353180cbaf..9053a1fff6 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java @@ -45,7 +45,7 @@ public class Http2NettyTest extends AbstractInteropTest { // Starts the server with HTTPS. try { return NettyServerBuilder.forPort(0) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(GrpcSslContexts .forServer(TestUtils.loadCert("server1.pem"), TestUtils.loadCert("server1.key")) @@ -63,7 +63,7 @@ public class Http2NettyTest extends AbstractInteropTest { try { NettyChannelBuilder builder = NettyChannelBuilder .forAddress(TestUtils.testServerAddress((InetSocketAddress) getListenAddress())) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(GrpcSslContexts .forClient() diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java index fc6e600789..ea4ba5877c 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java @@ -78,7 +78,7 @@ public class Http2OkHttpTest extends AbstractInteropTest { GrpcSslContexts.configure(contextBuilder, sslProvider); contextBuilder.ciphers(TestUtils.preferredTestCiphers(), SupportedCipherSuiteFilter.INSTANCE); return NettyServerBuilder.forPort(0) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractInteropTest.TEST_FLOW_CONTROL_WINDOW) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(contextBuilder.build()); } catch (IOException ex) { diff --git a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java index 182fd81d9d..7280ba6494 100644 --- a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java @@ -47,7 +47,7 @@ public class NettyTransportTest extends AbstractTransportTest { private final ClientTransportFactory clientFactory = NettyChannelBuilder // Although specified here, address is ignored because we never call build. .forAddress("localhost", 0) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractTransportTest.TEST_FLOW_CONTROL_WINDOW) .negotiationType(NegotiationType.PLAINTEXT) .setTransportTracerFactory(fakeClockTransportTracer) .buildTransportFactory(); @@ -67,7 +67,7 @@ public class NettyTransportTest extends AbstractTransportTest { List streamTracerFactories) { return NettyServerBuilder .forAddress(new InetSocketAddress("localhost", 0)) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractTransportTest.TEST_FLOW_CONTROL_WINDOW) .setTransportTracerFactory(fakeClockTransportTracer) .buildTransportServers(streamTracerFactories); } @@ -77,7 +77,7 @@ public class NettyTransportTest extends AbstractTransportTest { int port, List streamTracerFactories) { return NettyServerBuilder .forAddress(new InetSocketAddress("localhost", port)) - .flowControlWindow(65 * 1024) + .flowControlWindow(AbstractTransportTest.TEST_FLOW_CONTROL_WINDOW) .setTransportTracerFactory(fakeClockTransportTracer) .buildTransportServers(streamTracerFactories); } diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java index ac6b1122b9..8e99b549a3 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java @@ -56,7 +56,7 @@ public class OkHttpTransportTest extends AbstractTransportTest { return AccessProtectedHack.serverBuilderBuildTransportServer( NettyServerBuilder .forPort(0) - .flowControlWindow(65 * 1024), + .flowControlWindow(AbstractTransportTest.TEST_FLOW_CONTROL_WINDOW), streamTracerFactories, fakeClockTransportTracer); } @@ -67,7 +67,7 @@ public class OkHttpTransportTest extends AbstractTransportTest { return AccessProtectedHack.serverBuilderBuildTransportServer( NettyServerBuilder .forAddress(new InetSocketAddress(port)) - .flowControlWindow(65 * 1024), + .flowControlWindow(AbstractTransportTest.TEST_FLOW_CONTROL_WINDOW), streamTracerFactories, fakeClockTransportTracer); }