Move back to a mostly-single-threaded http2 transport by ctiller · Pull Request #8008 · grpc/grpc

ctiller · 2016-09-07T03:18:16Z

Combiner locks (refactored here) end up providing most of the gains we had from a complicated chttp2 threading model: drop the complicated book-keeping, get simpler, faster code.

@yang-g - please take chttp2 (grab @markdroth into the review if you want to split it up)
@vjpai - please look at the new & improved combiner locks, and iomgr/{closure,exec_ctx} changes
@sreecha - please look at workqueue changes (particularly the merge of that into the polling engine)
@dgquintas - channel changes, src/core/lib/support, src/core/lib/profiling, src/core/lib/surface, src/core/lib/transport

This change is

…low that request

fix write buffer ruby test

ctiller · 2016-10-17T20:48:09Z

Unless there are violent objections: I plan to bombs-away later this afternoon assuming tests look good.

There have been some minor edits since LGTM's came in:
@yang-g may want a quick look at df0f365 936f1ea 1409dbf
@vjpai may want a quick look at c2dd2a2 b019d61
@rjshade may want a quick look at 68cf8ce

kpayson64 · 2016-10-18T00:09:39Z

About the Python failure:

Nothing wrong is jumping out to me with the test. Basically we have a call waiting on a server response, then call grpc_completion_queue_next() in a background thread, then cancel the call from the main thread. The test hangs waiting for grpc_completion_queue_next() to return.

This seems like it should not hang, and in order for grpc_completion_queue_next() to return, grpc_wakeup_fd_wakeup() would somehow need to be called. It doesn't look like that is the case:

kpayson@kpayson0:~/grpc$ gdb --args py27/bin/python2.7-dbg src/python/grpcio_tests/tests/unit/_cython/_read_some_but_not_all_responses_test.py
GNU gdb (GDB) 7.9-gg19
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-grtev4-linux-gnu".
Type "show configuration" for configuration details.

<http://go/gdb-home FAQ: http://go/gdb-faq Email: c-toolchain-team IRC: gdb>
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Reading symbols from py27/bin/python2.7-dbg...done.
Unable to determine compiler version.
Skipping loading of libstdc++ pretty-printers for now.
Non-google3 binary detected.
(gdb) handle SIG40 noprint  
Signal        Stop  Print   Pass to program Description
SIG40         No    No  Yes     Real-time event 40
(gdb) break grpc_wakeup_fd_wakeup
Function "grpc_wakeup_fd_wakeup" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (grpc_wakeup_fd_wakeup) pending.
(gdb) break grpc_call_cancel
Function "grpc_call_cancel" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 2 (grpc_call_cancel) pending.
(gdb) run
Starting program: /usr/local/google/home/kpayson/grpc/py27/bin/python2.7-dbg src/python/grpcio_tests/tests/unit/_cython/_read_some_but_not_all_responses_test.py
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/grte/v4/lib64/libthread_db.so.1".

Breakpoint 1, grpc_wakeup_fd_wakeup (fd_info=0x7ffff5e871c8 <polling_island_wakeup_fd>)
    at src/core/lib/iomgr/wakeup_fd_posix.c:87
87    if (cv_wakeup_fds_enabled) {
(gdb) c
Continuing.
testReadSomeButNotAllResponses (__main__.ReadSomeButNotAllResponsesTest) ... [New Thread 0x7ffff27a9700 (LWP 62210)]
[Thread 0x7ffff27a9700 (LWP 62210) exited]
[New Thread 0x7ffff2faa700 (LWP 62209)]
[New Thread 0x7ffff37ab700 (LWP 62208)]
[New Thread 0x7ffff3fac700 (LWP 62207)]

Breakpoint 1, grpc_wakeup_fd_wakeup (fd_info=0xb2a398) at src/core/lib/iomgr/wakeup_fd_posix.c:87
87    if (cv_wakeup_fds_enabled) {
(gdb) c
Continuing.

Breakpoint 1, grpc_wakeup_fd_wakeup (fd_info=0xb2a398) at src/core/lib/iomgr/wakeup_fd_posix.c:87
87    if (cv_wakeup_fds_enabled) {
(gdb) c
Continuing.

Breakpoint 1, grpc_wakeup_fd_wakeup (fd_info=0xb2a398) at src/core/lib/iomgr/wakeup_fd_posix.c:87
87    if (cv_wakeup_fds_enabled) {
(gdb) c
Continuing.
[Thread 0x7ffff2faa700 (LWP 62209) exited]

Breakpoint 1, grpc_wakeup_fd_wakeup (fd_info=0x7fffe4000f38) at src/core/lib/iomgr/wakeup_fd_posix.c:87
87    if (cv_wakeup_fds_enabled) {
(gdb) c
Continuing.

Breakpoint 2, grpc_call_cancel (call=0xd47350, reserved=0x0) at src/core/lib/surface/call.c:733
733   GRPC_API_TRACE("grpc_call_cancel(call=%p, reserved=%p)", 2, (call, reserved));
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
sem_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_wait.S:85
85  ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_wait.S: No such file or directory.
(gdb)

ctiller · 2016-10-18T16:52:00Z

Manually verified alarming failed tests (and re-verified dbg)

thelinuxfoundation · 2019-01-26T04:44:34Z

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

User @rjshade isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf

Regards,
CLA GitHub bot

rjshade and others added 30 commits July 15, 2016 15:16

Extends the payload test to use large random bodies.

2449e19

Fixing int conversion warnings

bd10faa

Just create 1 MB strings, no argument.

61aed92

Explicitly add terminating null character.

c6f7767

Add byte_buffer_eq_slice method

d5d851b

progress

a47563c

Merge branch 'grand-unified-closures' into offload

1e1cd50

Implement is_finished for cqs

2f42fde

Fixup compilation

5f70fc6

Timing nuances

ca04562

Merge branch 'grand-unified-closures' into offload

99bef20

Initial pass reifying read and global locks

67eb59e

Fix typo

2679e4c

Merge github.com:grpc/grpc into merge-parse

378ffd8

Fixes for memory misuse

83200a4

Fix ping

73ba74b

Merge github.com:grpc/grpc into offload

7da613e

Merge branch 'offload' into merge-parse

1bfeb26

Merge branch 'merge-parse' of github.com:ctiller/grpc into merge-parse

8c2d75d

Fix typo

09f9792

Make failure to initialize a filter cause the server to silently swal…

8e21465

…low that request

Simplifications

13e4bf8

Eliminate dual stream maps

796f525

Eliminate lock during initialization

922f810

Fix refcounting bugs

f0e1119

Add tracer for pending tags

4e41e36

Fix bugs, make it easier to find them

49c644c

Merge branch 'merge-parse' of github.com:ctiller/grpc into merge-parse

f0e6554

Merge branch 'merge-parse' of github.com:ctiller/grpc into merge-parse

d2e1614

Remove global list of streams from transport

032b515

fix write buffer ruby test

f6e14b7

apolcyn mentioned this pull request Oct 13, 2016

fix write buffer ruby test ctiller/grpc#38

Merged

Merge pull request #38 from apolcyn/fix_write_buffer_test_on_pr

d688a6d

fix write buffer ruby test

sreecha approved these changes Oct 13, 2016

View reviewed changes

ctiller added 10 commits October 13, 2016 17:37

Fix bad stream terminators

df0f365

Merge branch 'direct-calls' of github.com:ctiller/grpc into direct-calls

99b923d

Revert changes to jobset.py

47284be

Revert spam

ef91e56

Fix potential crash

c2dd2a2

Merge github.com:grpc/grpc into direct-calls

fd77de6

Fix some test failures

936f1ea

clang-format

6cdbf6c

Merge github.com:grpc/grpc into direct-calls

7d2f0b0

Merge branch 'direct-calls' of github.com:ctiller/grpc into direct-calls

bcadfc3

ctiller mentioned this pull request Oct 17, 2016

Out of order stream messages Python stress test #7785

Closed

ctiller and others added 4 commits October 17, 2016 13:11

Better debug strings

1409dbf

Fix benchmark shutdown

b019d61

Merge github.com:grpc/grpc into direct-calls

8c94df1

Fix payload tests under windows

68cf8ce

Delambdaify

14c836f

ctiller force-pushed the direct-calls branch from 8c01ea8 to 14c836f Compare October 18, 2016 16:06

Fix wakeup bug

a81c311

ctiller merged commit 1a62ec8 into grpc:master Oct 18, 2016

lock bot locked as resolved and limited conversation to collaborators Jan 26, 2019

lock bot unassigned dgquintas and sreecha Jan 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move back to a mostly-single-threaded http2 transport#8008

Move back to a mostly-single-threaded http2 transport#8008
ctiller merged 158 commits intogrpc:masterfrom
ctiller:direct-calls

ctiller commented Sep 7, 2016 •

edited

Loading

Uh oh!

ctiller commented Oct 17, 2016

Uh oh!

kpayson64 commented Oct 18, 2016 •

edited

Loading

Uh oh!

ctiller commented Oct 18, 2016

Uh oh!

thelinuxfoundation commented Jan 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Conversation

ctiller commented Sep 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctiller commented Oct 17, 2016

Uh oh!

kpayson64 commented Oct 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctiller commented Oct 18, 2016

Uh oh!

thelinuxfoundation commented Jan 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

ctiller commented Sep 7, 2016 •

edited

Loading

kpayson64 commented Oct 18, 2016 •

edited

Loading