Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using cmux with MatchWithWriters consumes too much CPU #42

Closed
sadlil opened this issue Jan 27, 2017 · 11 comments
Closed

Using cmux with MatchWithWriters consumes too much CPU #42

sadlil opened this issue Jan 27, 2017 · 11 comments

Comments

@sadlil
Copy link

sadlil commented Jan 27, 2017

We are using cmux with gRPC and grpc-gateway like this example. . But we found out that using
grpcl := m.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc")) consumes too much cpu.

$ ~> ps axo pid,etime,%cpu,%mem,cmd | grep 'server-combine' | grep -v grep 7054 00:07 123 0.1 server-combine

Using m.Match(cmux.HTTP2HeaderField("content-type", "application/grpc")) is ok. but if try to use the L28 this takes too much CPU.

Is there any fix for this?

@sadlil
Copy link
Author

sadlil commented Jan 31, 2017

ping @soheilhy

@soheilhy
Copy link
Owner

Please attach a pprof callgraph and I'll take a look. MatchWithWriters has higher CPU usage as it's doing a lot more work. It's not recommended to use it unless you really have broken clients that need this matcher.

@tamalsaha
Copy link

cmux-callgraph.pdf

@soheilhy , I work with @sadlil . Here is a callgraph of this issue. It will be great if you can take a look.

@enm10k
Copy link

enm10k commented Dec 4, 2017

I have the same problem on latest master branch: bb79a83465015a27a175925ebd155e660f55e9f1 .
After I stopped using MatchWithWriters, high CPU utilization was solved.
Is anyone working on this?

@soheilhy
Copy link
Owner

soheilhy commented Dec 4, 2017

I'm not aware of any new issues. Do you have a profile or a call graph?

@enm10k
Copy link

enm10k commented Dec 4, 2017

Do you have a profile or a call graph?

Not now. I will try to create a PoC.
Please wait a few days.

@talgendler
Copy link

@soheilhy Perhaps when you send empty settings the clients can't handle that correctly, one can see that when debugging the server with GODEBUG=http2debug=2
Those 4 lines of code are in infinite loop

2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: wrote SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: read SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: wrote SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: read SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: wrote SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: read SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: wrote SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: read SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: wrote SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: read SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: wrote SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: read SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: wrote SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: read SETTINGS len=0
2018/01/29 17:19:31 http2: Framer 0xc420264000: wrote SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: read SETTINGS flags=ACK len=0
2018/01/29 17:19:31 http2: Framer 0xc4201fc2a0: wrote SETTINGS len=0

Both client and server are in go, the grpc-gateway is actually creating a local connection to the server so when it translates REST API calls the code call gRPC endpoint locally.

soheilhy added a commit that referenced this issue Jan 29, 2018
Reported-by talgendler in Issue #42
@soheilhy
Copy link
Owner

I think you've found the bug. Thank you.

Can you give the following patch a try? I think it should fix the issue.
e496b35

@talgendler
Copy link

@soheilhy That solves it.
Thanks

@soheilhy
Copy link
Owner

Great thank you for the confirmation.

soheilhy added a commit that referenced this issue Jan 29, 2018
Reported-by talgendler in Issue #42
@soheilhy
Copy link
Owner

I'm going to mark this bug as fixed. Please open if it flares up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants