Thriftpy/gunicorn_thrift

It seems -w parameter is very important but not in Readme.md?

Closed this issue · 6 comments

h3l commented

I just run command like

gunicorn_thrift tests.thriftpy_app:app -k thriftpy_sync \
  --thrift-protocol-factory \
    thriftpy.protocol:TCyBinaryProtocolFactory \
  --thrift-transport-factory \
    thriftpy.transport:TCyBufferedTransportFactory

,when I run on my computer, works well.
But on a server, some request always be hang on a long time.
i can't figure out which part got problem, thriftpy ? thrift_connector? gunicorn_thrift or even sqlalchemy.
finally, I found that one Process or called one worker only work for one connection. so i run into gunicorn documents find -w out.

so could you please add this parameter to Readme.md. so, someone else will not run in this trouble.

btw, could you please just share how to choose best client timeout and service timeout?

thanks.

wooparadog commented

-w parameter is a parameter inherited from gunicorn. As you say, it's very very important to sync servers.

We are internally using gevent worker to make io asynchronous, but gevent introduces a hell lot of problems. I'll add some instructions regarding how to choose gunicorn workers.

btw, could you please just share how to choose best client timeout and service timeout?

This depends on the worker type.

  • gevent(we are using): server_client_timeout: 20min. client keepalive should be lower than 20min (19min 50s as such)
  • sync: because one worker can only serve one connection, I recommend setting server client timeout as low as possible to release worker for other clients. The criteria should be the upstream's average response time. This makes rpc work in short connections, which is actually fine for low throughput

And come to think of it, we maybe should implement thread worker for better concurrency for apps without gevent. But I don't have time to do that currently. 😢

h3l commented

First of all, thank you very much.
And after you introduce, the sync worker type seems not very reasonable, Thread Pool may be a better design. But i didn't write this type of code before, so I need to do some work to prepare it.

aawilson commented

The sync worker type is pretty much exactly as reasonable as it is in gunicorn in general. Check out https://docs.gunicorn.org/en/stable/design.html for more information about the trade-offs between sync and async workers, and when to use them

aawilson commented

@wooparadog

We are internally using gevent worker to make io asynchronous, but gevent introduces a hell lot of problems.

Are you referring to general issues around designing around gevent, or specific issues within gunicorn_thrift? If it's the latter, I've got some time and desire to crank on them if you want to lay them out somewhere.

wooparadog commented
  • 👍1
wooparadog commented

Closing for now. Feel free to reopen.