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
This commit is contained in:
Menghan Li
2017-07-05 16:51:14 -07:00
committed by GitHub
parent 0100e4262c
commit 41d9b6ea2a
3 changed files with 9 additions and 5 deletions

View File

@ -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()

View File

@ -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 {

View File

@ -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
}