gRPC channels blocking indefinitely and not respecting deadlines on network disconnect by kpayson64 · Pull Request #15983 · grpc/grpc

kpayson64 · 2018-07-11T01:54:06Z

The chttp2 transport would not execute write closures until a write occurred, even if the stream was cancelled.

This PR separates the write callback closure lists based on streams, so they can be run when a cancellation comes in.

grpc-testing · 2018-07-11T01:57:27Z

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +0.0%     +10 [None]                                                                               +1.31Ki  +0.0%
  +1.7%    +102 src/core/ext/transport/chttp2/transport/writing.cc                                      +102  +1.7%
       +61%    +118 grpc_chttp2_end_write                                                                   +118   +61%
      [NEW]    +117 report_stall(grpc_chttp2_transport*, grpc_chttp2_stream*, char const*) [clone .part.    +117  [NEW]
       +44%     +12 [Unmapped]                                                                               +12   +44%
  +0.3%     +80 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                              +80  +0.3%
      +3.1%     +75 grpc_chttp2_cancel_stream                                                                +75  +3.1%
      +2.9%     +17 grpc_chttp2_complete_closure_step                                                        +17  +2.9%

  +0.0%    +192 TOTAL                                                                                +1.48Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-11T02:06:23Z

[trickle] No significant performance differences

grpc-testing · 2018-07-11T22:38:56Z

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +0.0%     +34 [None]                                                                               +1.62Ki  +0.0%
  +0.5%    +160 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                             +160  +0.5%
      +8.3%     +87 grpc_chttp2_mark_stream_closed                                                           +87  +8.3%
      +3.1%     +75 grpc_chttp2_cancel_stream                                                                +75  +3.1%
      +2.9%     +17 grpc_chttp2_complete_closure_step                                                        +17  +2.9%
  +1.7%    +102 src/core/ext/transport/chttp2/transport/writing.cc                                      +102  +1.7%
       +61%    +118 grpc_chttp2_end_write                                                                   +118   +61%
      [NEW]    +117 report_stall(grpc_chttp2_transport*, grpc_chttp2_stream*, char const*) [clone .part.    +117  [NEW]
       +44%     +12 [Unmapped]                                                                               +12   +44%

  +0.0%    +296 TOTAL                                                                                +1.88Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-11T22:46:47Z

[trickle] No significant performance differences

grpc-testing · 2018-07-11T23:31:15Z

[microbenchmarks] Performance differences noted:
Benchmark                         atm_add_per_iteration
--------------------------------  -----------------------
BM_TransportStreamSend/134217728  -19%
BM_TransportStreamSend/16777216   -19%
BM_TransportStreamSend/2097152    -19%

grpc-testing · 2018-07-12T17:30:27Z

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +0.0%     +34 [None]                                                                               +1.61Ki  +0.0%
  +0.5%    +160 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                             +160  +0.5%
      +8.3%     +87 grpc_chttp2_mark_stream_closed                                                           +87  +8.3%
      +3.1%     +75 grpc_chttp2_cancel_stream                                                                +75  +3.1%
      +2.9%     +17 grpc_chttp2_complete_closure_step                                                        +17  +2.9%
  +1.7%    +102 src/core/ext/transport/chttp2/transport/writing.cc                                      +102  +1.7%
       +61%    +118 grpc_chttp2_end_write                                                                   +118   +61%
      [NEW]    +117 report_stall(grpc_chttp2_transport*, grpc_chttp2_stream*, char const*) [clone .part.    +117  [NEW]
       +44%     +12 [Unmapped]                                                                               +12   +44%

  +0.0%    +296 TOTAL                                                                                +1.87Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-12T17:37:23Z

[trickle] No significant performance differences

grpc-testing · 2018-07-12T18:23:15Z

Corrupt JSON data (indicates timeout or crash): 
    bm_chttp2_transport.BM_TransportStreamSend_2097152.counters.new: 1


[microbenchmarks] Performance differences noted:
Benchmark                         atm_add_per_iteration
--------------------------------  -----------------------
BM_TransportStreamSend/134217728  -19%
BM_TransportStreamSend/16777216   -19%
BM_TransportStreamSend/2097152    -19%

grpc-testing · 2018-07-12T23:18:07Z

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +0.0%     +18 [None]                                                                               +1.67Ki  +0.0%
  +0.5%    +144 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                             +144  +0.5%
      +3.1%     +75 grpc_chttp2_cancel_stream                                                                +75  +3.1%
      +6.9%     +72 grpc_chttp2_mark_stream_closed                                                           +72  +6.9%
      +2.9%     +17 grpc_chttp2_complete_closure_step                                                        +17  +2.9%
  +1.7%    +102 src/core/ext/transport/chttp2/transport/writing.cc                                      +102  +1.7%
       +61%    +118 grpc_chttp2_end_write                                                                   +118   +61%
      [NEW]    +117 report_stall(grpc_chttp2_transport*, grpc_chttp2_stream*, char const*) [clone .part.    +117  [NEW]
       +44%     +12 [Unmapped]                                                                               +12   +44%

  +0.0%    +264 TOTAL                                                                                +1.91Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-12T23:25:25Z

[trickle] No significant performance differences

grpc-testing · 2018-07-13T00:09:24Z

[microbenchmarks] Performance differences noted:
Benchmark                                                                                  atm_add_per_iteration    atm_cas_per_iteration    nows_per_iteration
-----------------------------------------------------------------------------------------  -----------------------  -----------------------  --------------------
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                              +4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                              +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                           +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                           +4%
BM_TransportStreamSend/134217728                                                           -19%
BM_TransportStreamSend/16777216                                                            -19%
BM_TransportStreamSend/2097152                                                             -19%
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                  -14%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                               -14%
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/0/2097152                                                                                    -4%

yashykt

I've just got a few questions

yashykt · 2018-07-13T20:01:20Z

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

so.. run the closure immediately if there is an error (just thinking to myself).. though documentation might help

Added a note.

yashykt · 2018-07-13T20:07:23Z

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

shouldn't be needed right? remove_stream already has it

Good observation, I've removed it.

yashykt · 2018-07-13T20:08:01Z

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

same here. remove_stream should run everything

yashykt · 2018-07-13T20:08:45Z

src/core/ext/transport/chttp2/transport/writing.cc

we don't need to check for stream_became_writable anymore?

stream_became_writable is somewhat of a misnomer, it seems to mean a stream was unblocked by flow control for a message.

I guess my question is - was it a mistake to be there earlier?

Before grpc_chttp2_end_write would only have an effect for streams that were unblocked by flow control, so it wasn't an issue, just kind of a misnomer of writable.

Now we need to execute the new callback list, so we need to make sure it happens every time.

hmm? but wait. this should again have the same problem. If we run the closures only on grpc_chttp2_begin_write, then it would only be called when there is another write or the previous write completed. Isn't it the same issue all over again?

We flush the closures on both remove_stream and grpc_chttp2_end_write. In the cancellation case (the case that was broken before), remove_stream will be called, and that will take care of flushing the pending write ops.

Added back this check. PTAL

yashykt · 2018-07-13T20:11:05Z

src/core/ext/transport/chttp2/transport/internal.h

I'm guessing this is memset to zero and hence does not need to be inited

I copied the pattern of the old per-transport run_after_write, and that one is not initialized either.

grpc-testing · 2018-07-14T00:08:27Z

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +0.0%     +50 [None]                                                                                 +1010  +0.0%
  +1.7%    +102 src/core/ext/transport/chttp2/transport/writing.cc                                      +102  +1.7%
       +61%    +118 grpc_chttp2_end_write                                                                   +118   +61%
      [NEW]    +117 report_stall(grpc_chttp2_transport*, grpc_chttp2_stream*, char const*) [clone .part.    +117  [NEW]
       +44%     +12 [Unmapped]                                                                               +12   +44%
  +0.3%     +80 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                              +80  +0.3%
      +6.9%     +72 grpc_chttp2_mark_stream_closed                                                           +72  +6.9%
      +2.9%     +17 grpc_chttp2_complete_closure_step                                                        +17  +2.9%

  +0.0%    +232 TOTAL                                                                                +1.16Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-14T00:17:29Z

[trickle] No significant performance differences

grpc-testing · 2018-07-14T00:59:44Z

[microbenchmarks] Performance differences noted:
Benchmark                                                                                  atm_add_per_iteration    atm_cas_per_iteration    cpu_time    nows_per_iteration    real_time
-----------------------------------------------------------------------------------------  -----------------------  -----------------------  ----------  --------------------  -----------
BM_StreamingPingPong<InProcess, NoOpMutator, NoOpMutator>/32768/2                                                                            -5%                               -5%
BM_StreamingPingPong<MinInProcess, NoOpMutator, NoOpMutator>/262144/1                                                                        -7%                               -7%
BM_StreamingPingPong<MinInProcess, NoOpMutator, NoOpMutator>/262144/2                                                                        -4%                               -4%
BM_StreamingPingPongMsgs<InProcess, NoOpMutator, NoOpMutator>/32768                                                                          -9%                               -9%
BM_StreamingPingPongMsgs<MinInProcess, NoOpMutator, NoOpMutator>/32768                                                                       -4%                               -4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                              +4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                              +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                           +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                           +4%
BM_TransportStreamSend/134217728                                                           -19%
BM_TransportStreamSend/16777216                                                            -19%
BM_TransportStreamSend/2097152                                                             -19%
BM_UnaryPingPong<InProcess, NoOpMutator, NoOpMutator>/0/262144                                                                               -4%                               -4%
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                              -14%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/16777216                                                                                +7%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                           -15%

grpc-testing · 2018-07-16T23:34:41Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +18 [None]                                                         +874  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.3%     +80 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +80  +0.3%
      +6.9%     +72 grpc_chttp2_mark_stream_closed                                  +72  +6.9%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%

  +0.0%    +232 TOTAL                                                       +1.06Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-16T23:37:29Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +18 [None]                                                         +874  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.3%     +80 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +80  +0.3%
      +6.9%     +72 grpc_chttp2_mark_stream_closed                                  +72  +6.9%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%

  +0.0%    +232 TOTAL                                                       +1.06Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-16T23:41:36Z

[trickle] No significant performance differences

grpc-testing · 2018-07-16T23:45:44Z

[trickle] No significant performance differences

grpc-testing · 2018-07-17T00:29:41Z

[microbenchmarks] Performance differences noted:
Benchmark                                                                                  atm_add_per_iteration    atm_cas_per_iteration    nows_per_iteration
-----------------------------------------------------------------------------------------  -----------------------  -----------------------  --------------------
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                              +4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                              +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                           +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                           +4%
BM_TransportStreamSend/134217728                                                           -19%
BM_TransportStreamSend/16777216                                                            -19%
BM_TransportStreamSend/2097152                                                             -19%
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                  -12%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                               -13%
BM_UnaryPingPong<MinTCP, NoOpMutator, NoOpMutator>/0/2097152                                                                                 -7%

grpc-testing · 2018-07-17T00:57:31Z

[microbenchmarks] Performance differences noted:
Benchmark                                                                                        atm_add_per_iteration    atm_cas_per_iteration    nows_per_iteration
-----------------------------------------------------------------------------------------------  -----------------------  -----------------------  --------------------
BM_StreamingPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/16777216/1                                                                         -10%
BM_StreamingPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/1                                                                          -31%
BM_StreamingPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2                                                                          -22%
BM_StreamingPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/16777216/1                                                                      -13%
BM_StreamingPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/1                                                                       -31%
BM_StreamingPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2                                                                       -22%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                                    +4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                                    +4%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/1/0                                                       -14%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2/0                                                       -22%
BM_StreamingPingPongWithCoalescingApi<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2/1                                                       -22%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/0                                 +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/0/0/1                                 +4%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/1/0                                                    -15%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2/0                                                    -20%
BM_StreamingPingPongWithCoalescingApi<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2/1                                                    -22%
BM_TransportStreamSend/134217728                                                                 -19%
BM_TransportStreamSend/16777216                                                                  -19%
BM_TransportStreamSend/2097152                                                                   -19%
BM_UnaryPingPong<InProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                        -13%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/16777216/16777216                                                                   -5%
BM_UnaryPingPong<MinInProcessCHTTP2, NoOpMutator, NoOpMutator>/2097152/2097152                                                                     -14%

grpc-testing · 2018-07-19T23:15:03Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +42 [None]                                                         +458  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.1%     +16 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +16  +0.1%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%
      +1.0%     +11 grpc_chttp2_mark_stream_closed                                  +11  +1.0%
       +14%      +7 grpc_chttp2_mark_stream_writable                                 +7   +14%

  +0.0%    +192 TOTAL                                                          +608  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE                                    FILE SIZE
 ++++++++++++++ GROWING                      ++++++++++++++
  +4.3%      +4 src/cpp/common/version_cc.cc      +4  +4.3%
      +8.7%      +4 grpc::Version[abi:cxx11]          +4  +8.7%

 -+-+-+-+-+-+-+ MIXED                        +-+-+-+-+-+-+-
  -0.0%      -4 [None]                           +12  +0.0%

  [ = ]       0 TOTAL                            +16  +0.0%

grpc-testing · 2018-07-19T23:19:19Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +26 [None]                                                         +682  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.1%     +32 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +32  +0.1%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%
      +1.0%     +11 grpc_chttp2_mark_stream_closed                                  +11  +1.0%
       +14%      +7 grpc_chttp2_mark_stream_writable                                 +7   +14%

  +0.0%    +192 TOTAL                                                          +848  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-20T01:18:46Z

Corrupt JSON data (indicates timeout or crash): 
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_1_16777216_67108864.opt.new: 4
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_1_16777216_262144.opt.new: 4
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_16777216_16777216_262144.opt.new: 4
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_16777216_16777216_67108864.opt.new: 4
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_16777216_16777216_4194304.opt.new: 4
    bm_fullstack_trickle.BM_PumpUnbalancedUnary_Trickle_1_16777216_4194304.opt.new: 4


[trickle] No significant performance differences

grpc-testing · 2018-07-23T22:38:04Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +10 [None]                                                         +834  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.4%    +112 src/core/ext/transport/chttp2/transport/chttp2_transport.cc    +112  +0.4%
      +9.4%     +99 grpc_chttp2_mark_stream_closed                                  +99  +9.4%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%
       +14%      +7 grpc_chttp2_mark_stream_writable                                 +7   +14%

  +0.0%    +256 TOTAL                                                       +1.05Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-23T23:45:22Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +26 [None]                                                         +866  +0.0%
  +2.3%    +134 src/core/ext/transport/chttp2/transport/writing.cc             +134  +2.3%
       +61%    +118 grpc_chttp2_end_write                                          +118   +61%
      +0.3%     +16 grpc_chttp2_begin_write                                         +16  +0.3%
  +0.4%    +128 src/core/ext/transport/chttp2/transport/chttp2_transport.cc    +128  +0.4%
      +9.4%     +99 grpc_chttp2_mark_stream_closed                                  +99  +9.4%
      +2.9%     +17 grpc_chttp2_complete_closure_step                               +17  +2.9%
       +32%     +16 grpc_chttp2_mark_stream_writable                                +16   +32%

  +0.0%    +288 TOTAL                                                       +1.10Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-23T23:54:05Z

[trickle] No significant performance differences

grpc-testing · 2018-07-24T01:04:57Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.1%    +632 [None]                                                      +2.55Ki  +0.0%
  +6.9%    +128 src/core/ext/transport/chttp2/transport/stream_lists.cc        +128  +6.9%
      [NEW]     +40 grpc_chttp2_list_add_waiting_for_write_stream                   +40  [NEW]
      [NEW]     +40 grpc_chttp2_list_remove_waiting_for_write_stream                +40  [NEW]
       +17%     +22 [Unmapped]                                                      +22   +17%
      +8.5%     +16 stream_list_id_string                                           +16  +8.5%
      [NEW]     +10 grpc_chttp2_list_pop_waiting_for_write_stream                   +10  [NEW]
  +1.8%    +104 src/core/ext/transport/chttp2/transport/writing.cc             +104  +1.8%
       +54%    +104 grpc_chttp2_end_write                                          +104   +54%
  +0.1%     +32 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +32  +0.1%
       +11%     +62 grpc_chttp2_complete_closure_step                               +62   +11%
      +1.2%     +29 grpc_chttp2_cancel_stream                                       +29  +1.2%
      +1.8%     +19 grpc_chttp2_mark_stream_closed                                  +19  +1.8%

  +0.1%    +896 TOTAL                                                       +2.80Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-24T01:11:57Z

[trickle] No significant performance differences

grpc-testing · 2018-07-24T18:22:46Z

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.1%    +648 [None]                                                      +2.45Ki  +0.0%
  +0.6%    +176 src/core/ext/transport/chttp2/transport/chttp2_transport.cc    +176  +0.6%
       +27%     +95 set_write_state                                                 +95   +27%
      +6.7%     +39 grpc_chttp2_complete_closure_step                               +39  +6.7%
      +1.2%     +29 grpc_chttp2_cancel_stream                                       +29  +1.2%
      +1.5%     +10 [Unmapped]                                                      +10  +1.5%
      +2.8%      +8 grpc_chttp2_add_ping_strike                                      +8  +2.8%
  +6.9%    +128 src/core/ext/transport/chttp2/transport/stream_lists.cc        +128  +6.9%
      [NEW]     +40 grpc_chttp2_list_add_waiting_for_write_stream                   +40  [NEW]
      [NEW]     +40 grpc_chttp2_list_remove_waiting_for_write_stream                +40  [NEW]
       +17%     +22 [Unmapped]                                                      +22   +17%
      +8.5%     +16 stream_list_id_string                                           +16  +8.5%
      [NEW]     +10 grpc_chttp2_list_pop_waiting_for_write_stream                   +10  [NEW]

  +0.1%    +952 TOTAL                                                       +2.75Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-07-24T18:30:35Z

[trickle] No significant performance differences

grpc-testing · 2018-07-24T19:16:19Z

[microbenchmarks] Performance differences noted:
Benchmark                                                                       atm_add_per_iteration
------------------------------------------------------------------------------  -----------------------
BM_StreamCreateSendInitialMetadataDestroy<RepresentativeClientInitialMetadata>  +5%
BM_TransportStreamSend/0                                                        +14%
BM_TransportStreamSend/1                                                        +12%
BM_TransportStreamSend/134217728                                                +9%
BM_TransportStreamSend/16777216                                                 +9%
BM_TransportStreamSend/2097152                                                  +9%
BM_TransportStreamSend/262144                                                   +12%
BM_TransportStreamSend/32768                                                    +12%
BM_TransportStreamSend/4096                                                     +12%
BM_TransportStreamSend/512                                                      +12%
BM_TransportStreamSend/64                                                       +12%
BM_TransportStreamSend/8                                                        +12%

yashykt

LGTM! Thanks for doing this!

…cting deadlines on network disconnect

Revert #15983 - gRPC channels blocking indefinitely and not respecting deadlines on network disconnect

kpayson64 mentioned this pull request Jul 11, 2018

gRPC channels blocking indefinitely and not respecting deadlines on network disconnect #15889

Closed

kpayson64 force-pushed the write_stalling_fix branch from 480182e to f9b1856 Compare July 11, 2018 22:34

kpayson64 added the kokoro:force-run label Jul 12, 2018

kokoro-team removed the kokoro:force-run label Jul 12, 2018

kpayson64 requested a review from yashykt July 13, 2018 17:53

yashykt reviewed Jul 13, 2018

View reviewed changes

kpayson64 force-pushed the write_stalling_fix branch from aecea1d to 66311d2 Compare July 19, 2018 23:11

kpayson64 changed the base branch from master to v1.14.x July 19, 2018 23:13

kpayson64 force-pushed the write_stalling_fix branch from 66311d2 to f4c9151 Compare July 19, 2018 23:15

Cancel pending write closures on stream cancellation

5562dd5

kpayson64 force-pushed the write_stalling_fix branch from d3949a5 to 5562dd5 Compare July 24, 2018 01:01

Bug fixes

5c1e47a

yashykt approved these changes Jul 24, 2018

View reviewed changes

kpayson64 merged commit f268d71 into grpc:v1.14.x Jul 24, 2018

kpayson64 added the release notes: yes Indicates if PR needs to be in release notes label Jul 26, 2018

srini100 changed the title ~~Write stalling fix~~ gRPC channels blocking indefinitely and not respecting deadlines on network disconnect Jul 27, 2018

srini100 added the lang/core label Jul 27, 2018

srini100 mentioned this pull request Aug 3, 2018

Revert "Immediately run write closures for failed stream" #16243

Merged

srini100 added a commit to srini100/grpc that referenced this pull request Aug 7, 2018

Revert grpc#15983 - gRPC channels blocking indefinitely and not respe…

9490969

…cting deadlines on network disconnect

srini100 mentioned this pull request Aug 7, 2018

Revert #15983 - gRPC channels blocking indefinitely and not respecting deadlines on network disconnect #16276

Merged

yashykt mentioned this pull request Aug 7, 2018

Revert "gRPC channels blocking indefinitely and not respecting deadlines on network disconnect" #16277

Closed

srini100 added a commit that referenced this pull request Aug 8, 2018

Merge pull request #16276 from srini100/v1.14.x

d8020cb

Revert #15983 - gRPC channels blocking indefinitely and not respecting deadlines on network disconnect

srini100 mentioned this pull request Aug 10, 2018

Merge from v1.14.x #16321

Merged

lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018

Conversation

kpayson64 commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 11, 2018

Uh oh!

grpc-testing commented Jul 12, 2018

Uh oh!

grpc-testing commented Jul 12, 2018

Uh oh!

grpc-testing commented Jul 12, 2018

Uh oh!

grpc-testing commented Jul 12, 2018

Uh oh!

grpc-testing commented Jul 12, 2018

Uh oh!

grpc-testing commented Jul 13, 2018

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented Jul 14, 2018

Uh oh!

grpc-testing commented Jul 14, 2018

Uh oh!

grpc-testing commented Jul 14, 2018

Uh oh!

grpc-testing commented Jul 16, 2018

Uh oh!

grpc-testing commented Jul 16, 2018

Uh oh!

grpc-testing commented Jul 16, 2018

Uh oh!

grpc-testing commented Jul 16, 2018

Uh oh!

grpc-testing commented Jul 17, 2018

Uh oh!

grpc-testing commented Jul 17, 2018

Uh oh!

grpc-testing commented Jul 19, 2018

Uh oh!

grpc-testing commented Jul 19, 2018

Uh oh!