Skip to content

Commit

Permalink
Do not flush NewStream header on client side for unary RPCs and strea…
Browse files Browse the repository at this point in the history
…ming RPCs with requests. (grpc#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
  • Loading branch information
menghanl authored Jul 5, 2017
1 parent 0100e42 commit 41d9b6e
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
6 changes: 5 additions & 1 deletion stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 1 addition & 3 deletions transport/http2_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 41d9b6e

Please sign in to comment.