Avoid flushing headers when the server returns a single response (#9314)

* core,netty: avoid flushing the headers on the server side when the server has a single response.
This commit is contained in:
amirhadadi 2023-10-09 19:43:10 +03:00 committed by GitHub
parent aec9a56eaf
commit 8c0545564f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 43 additions and 43 deletions

View File

@ -77,7 +77,7 @@ final class MultiMessageServerStream implements ServerStream {
}
@Override
public void writeHeaders(Metadata headers) {
public void writeHeaders(Metadata headers, boolean flush) {
try {
synchronized (outbound) {
outbound.sendHeaders(headers);

View File

@ -80,7 +80,7 @@ final class SingleMessageServerStream implements ServerStream {
}
@Override
public void writeHeaders(Metadata headers) {
public void writeHeaders(Metadata headers, boolean flush) {
pendingHeaders = headers;
}

View File

@ -41,7 +41,7 @@ public abstract class AbstractServerStream extends AbstractStream
*
* @param headers the headers to be sent to client.
*/
void writeHeaders(Metadata headers);
void writeHeaders(Metadata headers, boolean flush);
/**
* Sends an outbound frame to the remote end point.
@ -96,11 +96,11 @@ public abstract class AbstractServerStream extends AbstractStream
}
@Override
public final void writeHeaders(Metadata headers) {
public final void writeHeaders(Metadata headers, boolean flush) {
Preconditions.checkNotNull(headers, "headers");
headersSent = true;
abstractServerStreamSink().writeHeaders(headers);
abstractServerStreamSink().writeHeaders(headers, flush);
}
@Override

View File

@ -142,7 +142,7 @@ final class ServerCallImpl<ReqT, RespT> extends ServerCall<ReqT, RespT> {
// Don't check if sendMessage has been called, since it requires that sendHeaders was already
// called.
sendHeadersCalled = true;
stream.writeHeaders(headers);
stream.writeHeaders(headers, !getMethodDescriptor().getType().serverSendsOneMessage());
}
@Override

View File

@ -36,7 +36,7 @@ public interface ServerStream extends Stream {
*
* @param headers to send to client.
*/
void writeHeaders(Metadata headers);
void writeHeaders(Metadata headers, boolean flush);
/**
* Closes the stream for both reading and writing. A status code of

View File

@ -285,28 +285,28 @@ public class AbstractServerStreamTest {
public void writeHeaders_failsOnNullHeaders() {
thrown.expect(NullPointerException.class);
stream.writeHeaders(null);
stream.writeHeaders(null, true);
}
@Test
public void writeHeaders() {
Metadata headers = new Metadata();
stream.writeHeaders(headers);
verify(sink).writeHeaders(same(headers));
stream.writeHeaders(headers, true);
verify(sink).writeHeaders(same(headers), eq(true));
}
@Test
public void writeMessage_dontWriteDuplicateHeaders() {
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
stream.writeMessage(new ByteArrayInputStream(new byte[]{}));
// Make sure it wasn't called twice
verify(sink).writeHeaders(any(Metadata.class));
verify(sink).writeHeaders(any(Metadata.class), eq(true));
}
@Test
public void writeMessage_ignoreIfFramerClosed() {
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
stream.endOfMessages();
reset(sink);
@ -317,7 +317,7 @@ public class AbstractServerStreamTest {
@Test
public void writeMessage() {
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
stream.writeMessage(new ByteArrayInputStream(new byte[]{}));
stream.flush();
@ -327,7 +327,7 @@ public class AbstractServerStreamTest {
@Test
public void writeMessage_closesStream() throws Exception {
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
InputStream input = mock(InputStream.class, delegatesTo(new ByteArrayInputStream(new byte[1])));
stream.writeMessage(input);
verify(input).close();

View File

@ -153,7 +153,7 @@ public class ServerCallImplTest {
call.sendHeaders(headers);
verify(stream).writeHeaders(headers);
verify(stream).writeHeaders(headers, false);
}
@Test
@ -162,7 +162,7 @@ public class ServerCallImplTest {
headers.put(CONTENT_LENGTH_KEY, "123");
call.sendHeaders(headers);
verify(stream).writeHeaders(headers);
verify(stream).writeHeaders(headers, false);
assertNull(headers.get(CONTENT_LENGTH_KEY));
}

View File

@ -681,7 +681,7 @@ public class ServerImplTest {
Metadata responseHeaders = new Metadata();
responseHeaders.put(metadataKey, "response value");
call.sendHeaders(responseHeaders);
verify(stream).writeHeaders(responseHeaders);
verify(stream).writeHeaders(responseHeaders, true);
verify(stream).setCompressor(isA(Compressor.class));
call.sendMessage(firstResponse);

View File

@ -290,7 +290,7 @@ public abstract class AbstractTransportTest {
stream.flush();
stream.cancel(Status.CANCELLED);
stream.flush();
serverStreamCreation.stream.writeHeaders(new Metadata());
serverStreamCreation.stream.writeHeaders(new Metadata(), true);
serverStreamCreation.stream.flush();
serverStreamCreation.stream.writeMessage(methodDescriptor.streamResponse("bar"));
serverStreamCreation.stream.flush();
@ -308,7 +308,7 @@ public abstract class AbstractTransportTest {
stream.start(mockClientStreamListener2);
serverStreamCreation
= serverTransportListener.takeStreamOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
serverStreamCreation.stream.writeHeaders(new Metadata());
serverStreamCreation.stream.writeHeaders(new Metadata(), true);
serverStreamCreation.stream.flush();
verify(mockClientStreamListener2, timeout(TIMEOUT_MS)).headersRead(any(Metadata.class));
@ -468,7 +468,7 @@ public abstract class AbstractTransportTest {
// Try to "flush" out any listener notifications on client and server. This also ensures that
// the stream still functions.
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
clientStream.halfClose();
assertNotNull(clientStreamListener.headers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS));
assertTrue(serverStreamListener.awaitHalfClosed(TIMEOUT_MS, TimeUnit.MILLISECONDS));
@ -886,7 +886,7 @@ public abstract class AbstractTransportTest {
serverHeaders.put(binaryKey, "dup,value");
Metadata serverHeadersCopy = new Metadata();
serverHeadersCopy.merge(serverHeaders);
serverStream.writeHeaders(serverHeaders);
serverStream.writeHeaders(serverHeaders, true);
Metadata headers = clientStreamListener.headers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS);
assertNotNull(headers);
assertAsciiMetadataValuesEqual(serverHeadersCopy.getAll(asciiKey), headers.getAll(asciiKey));
@ -1010,7 +1010,7 @@ public abstract class AbstractTransportTest {
clientStream.halfClose();
assertTrue(serverStreamListener.awaitHalfClosed(TIMEOUT_MS, TimeUnit.MILLISECONDS));
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
assertNotNull(clientStreamListener.headers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS));
Status status = Status.OK.withDescription("Nice talking to you");
@ -1047,7 +1047,7 @@ public abstract class AbstractTransportTest {
ServerStream serverStream = serverStreamCreation.stream;
ServerStreamListenerBase serverStreamListener = serverStreamCreation.listener;
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
assertNotNull(clientStreamListener.headers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS));
Status strippedStatus = Status.OK.withDescription("Hello. Goodbye.");
@ -1273,7 +1273,7 @@ public abstract class AbstractTransportTest {
assertTrue(serverStreamListener.awaitOnReadyAndDrain(TIMEOUT_MS, TimeUnit.MILLISECONDS));
assertTrue(serverStream.isReady());
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
serverStream.writeMessage(methodDescriptor.streamRequest("foo"));
serverStream.flush();
@ -1362,7 +1362,7 @@ public abstract class AbstractTransportTest {
ServerStream serverStream = serverStreamCreation.stream;
ServerStreamListenerBase serverStreamListener = serverStreamCreation.listener;
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
String largeMessage;
{
@ -1559,7 +1559,7 @@ public abstract class AbstractTransportTest {
assertNotNull(clientStreamListener.trailers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS));
// Ensure that for a closed ServerStream, interactions are noops
server.stream.writeHeaders(new Metadata());
server.stream.writeHeaders(new Metadata(), true);
server.stream.writeMessage(methodDescriptor.streamResponse("response"));
server.stream.close(Status.INTERNAL, new Metadata());
@ -1868,7 +1868,7 @@ public abstract class AbstractTransportTest {
assertEquals(0, clientBefore.lastMessageReceivedTimeNanos);
clientStream.request(1);
serverStream.writeHeaders(new Metadata());
serverStream.writeHeaders(new Metadata(), true);
serverStream.writeMessage(methodDescriptor.streamResponse("response"));
serverStream.flush();
verifyMessageCountAndClose(clientStreamListener.messageQueue, 1);
@ -1984,7 +1984,7 @@ public abstract class AbstractTransportTest {
= serverTransportListener.takeStreamOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
serverStreamCreation.stream.request(1);
serverStreamCreation.stream.writeHeaders(tooLargeMetadata);
serverStreamCreation.stream.writeHeaders(tooLargeMetadata, true);
serverStreamCreation.stream.writeMessage(methodDescriptor.streamResponse("response"));
serverStreamCreation.stream.close(Status.OK, new Metadata());
@ -2029,7 +2029,7 @@ public abstract class AbstractTransportTest {
= serverTransportListener.takeStreamOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
serverStreamCreation.stream.request(1);
serverStreamCreation.stream.writeHeaders(new Metadata());
serverStreamCreation.stream.writeHeaders(new Metadata(), true);
serverStreamCreation.stream.writeMessage(methodDescriptor.streamResponse("response"));
serverStreamCreation.stream.close(Status.OK, tooLargeMetadata);

View File

@ -546,7 +546,7 @@ final class InProcessTransport implements ServerTransport, ConnectionClientTrans
}
@Override
public void writeHeaders(Metadata headers) {
public void writeHeaders(Metadata headers, boolean flush) {
if (clientMaxInboundMetadataSize != Integer.MAX_VALUE) {
int metadataSize = metadataSize(headers);
if (metadataSize > clientMaxInboundMetadataSize) {

View File

@ -94,13 +94,13 @@ class NettyServerStream extends AbstractServerStream {
private class Sink implements AbstractServerStream.Sink {
@Override
public void writeHeaders(Metadata headers) {
public void writeHeaders(Metadata headers, boolean flush) {
try (TaskCloseable ignore = PerfMark.traceTask("NettyServerStream$Sink.writeHeaders")) {
writeQueue.enqueue(
SendResponseHeadersCommand.createHeaders(
transportState(),
Utils.convertServerHeaders(headers)),
true);
flush);
}
}

View File

@ -923,7 +923,7 @@ public class NettyClientTransportTest {
public void streamCreated(ServerStream stream, String method, Metadata headers) {
EchoServerStreamListener listener = new EchoServerStreamListener(stream, headers);
stream.setListener(listener);
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
stream.request(1);
streamListeners.add(listener);
}

View File

@ -104,7 +104,7 @@ public class NettyServerStreamTest extends NettyStreamTestBase<NettyServerStream
.status(Utils.STATUS_OK)
.set(Utils.CONTENT_TYPE_HEADER, Utils.CONTENT_TYPE_GRPC));
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
ArgumentCaptor<SendResponseHeadersCommand> sendHeadersCap =
ArgumentCaptor.forClass(SendResponseHeadersCommand.class);
@ -130,7 +130,7 @@ public class NettyServerStreamTest extends NettyStreamTestBase<NettyServerStream
ListMultimap<CharSequence, CharSequence> expectedHeaders =
ImmutableListMultimap.copyOf(Utils.convertServerHeaders(headers));
stream().writeHeaders(headers);
stream().writeHeaders(headers, true);
ArgumentCaptor<SendResponseHeadersCommand> sendHeadersCap =
ArgumentCaptor.forClass(SendResponseHeadersCommand.class);
@ -300,7 +300,7 @@ public class NettyServerStreamTest extends NettyStreamTestBase<NettyServerStream
@Override
protected void sendHeadersIfServer() {
stream.writeHeaders(new Metadata());
stream.writeHeaders(new Metadata(), true);
}
@Override

View File

@ -83,7 +83,7 @@ class OkHttpServerStream extends AbstractServerStream {
class Sink implements AbstractServerStream.Sink {
@Override
public void writeHeaders(Metadata metadata) {
public void writeHeaders(Metadata metadata, boolean flush) {
try (TaskCloseable ignore =
PerfMark.traceTask("OkHttpServerStream$Sink.writeHeaders")) {
List<Header> responseHeaders = Headers.createResponseHeaders(metadata);

View File

@ -371,7 +371,7 @@ public class OkHttpServerTransportTest {
assertThat(streamListener.messages.pop()).isEqualTo("Hello server");
assertThat(streamListener.halfClosedCalled).isTrue();
streamListener.stream.writeHeaders(metadata("User-Data", "best data"));
streamListener.stream.writeHeaders(metadata("User-Data", "best data"), false);
streamListener.stream.writeMessage(new ByteArrayInputStream("Howdy client".getBytes(UTF_8)));
streamListener.stream.close(Status.OK, metadata("End-Metadata", "bye"));
@ -434,7 +434,7 @@ public class OkHttpServerTransportTest {
assertThat(streamListener.messages.pop()).isEqualTo("Hello server");
assertThat(streamListener.halfClosedCalled).isTrue();
streamListener.stream.writeHeaders(new Metadata());
streamListener.stream.writeHeaders(new Metadata(), false);
streamListener.stream.writeMessage(new ByteArrayInputStream("Howdy client".getBytes(UTF_8)));
streamListener.stream.flush();
@ -1034,7 +1034,7 @@ public class OkHttpServerTransportTest {
streamListener.stream.request(1);
pingPong();
assertThat(streamListener.messages.pop()).isEqualTo("Hello Server Pad Me!");
streamListener.stream.writeHeaders(metadata("User-Data", "best data"));
streamListener.stream.writeHeaders(metadata("User-Data", "best data"), false);
streamListener.stream.writeMessage(new ByteArrayInputStream("Howdy client".getBytes(UTF_8)));
List<Header> responseHeaders = Arrays.asList(
new Header(":status", "200"),
@ -1211,7 +1211,7 @@ public class OkHttpServerTransportTest {
pingPong();
assertThat(streamListener.messages.pop()).isEqualTo("Hello server");
streamListener.stream.writeHeaders(metadata("User-Data", "best data"));
streamListener.stream.writeHeaders(metadata("User-Data", "best data"), false);
streamListener.stream.writeMessage(new ByteArrayInputStream("Howdy client".getBytes(UTF_8)));
streamListener.stream.flush();
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();

View File

@ -225,7 +225,7 @@ final class ServletServerStream extends AbstractServerStream {
final TrailerSupplier trailerSupplier = new TrailerSupplier();
@Override
public void writeHeaders(Metadata headers) {
public void writeHeaders(Metadata headers, boolean flush) {
writeHeadersToServletResponse(headers);
resp.setTrailerFields(trailerSupplier);
try {