From 41d9b6ea2a6335f3a22074ed35c0542c9da1baf4 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Wed, 5 Jul 2017 16:51:14 -0700 Subject: [PATCH] Do not flush NewStream header on client side for unary RPCs and streaming RPCs with requests. (#1343) If it's not client streaming, we should already have the request to be sent, so we don't flush the header. If it's client streaming, the user may never send a request or send it any time soon, so we ask the transport to flush the header. And flush header even without metadata --- stream.go | 6 +++++- transport/http2_client.go | 4 +--- transport/transport.go | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/stream.go b/stream.go index ac8481f35..30374afa6 100644 --- a/stream.go +++ b/stream.go @@ -130,7 +130,11 @@ func newClientStream(ctx context.Context, desc *StreamDesc, cc *ClientConn, meth callHdr := &transport.CallHdr{ Host: cc.authority, Method: method, - Flush: desc.ServerStreams && desc.ClientStreams, + // If it's not client streaming, we should already have the request to be sent, + // so we don't flush the header. + // If it's client streaming, the user may never send a request or send it any + // time soon, so we ask the transport to flush the header. + Flush: desc.ClientStreams, } if cc.dopts.cp != nil { callHdr.SendCompress = cc.dopts.cp.Type() diff --git a/transport/http2_client.go b/transport/http2_client.go index 3579d6cf5..09bc4f4a5 100644 --- a/transport/http2_client.go +++ b/transport/http2_client.go @@ -465,11 +465,9 @@ func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Strea t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)}) } var ( - hasMD bool endHeaders bool ) if md, ok := metadata.FromOutgoingContext(ctx); ok { - hasMD = true for k, vv := range md { // HTTP doesn't allow you to set pseudoheaders after non pseudoheaders were set. if isReservedHeader(k) { @@ -501,7 +499,7 @@ func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Strea endHeaders = true } var flush bool - if endHeaders && (hasMD || callHdr.Flush) { + if callHdr.Flush && endHeaders { flush = true } if first { diff --git a/transport/transport.go b/transport/transport.go index 27f58d079..14eb1627f 100644 --- a/transport/transport.go +++ b/transport/transport.go @@ -539,8 +539,10 @@ type CallHdr struct { // Flush indicates whether a new stream command should be sent // to the peer without waiting for the first data. This is - // only a hint. The transport may modify the flush decision + // only a hint. + // If it's true, the transport may modify the flush decision // for performance purposes. + // If it's false, new stream will never be flushed. Flush bool }