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

http-keepalive must be set as int from ini #2018

Closed
kbutz opened this issue May 14, 2019 · 5 comments
Closed

http-keepalive must be set as int from ini #2018

kbutz opened this issue May 14, 2019 · 5 comments

Comments

@kbutz
Copy link

kbutz commented May 14, 2019

I'm not sure if this is a bug or if the documentation just needs to be a little more clear here.

While it is noted in the uWSGI config options that the parser for http-keepalive is uwsgi_opt_true, it isn't immediately obvious or clear. Making it worse, there are many examples floating around out there of people suggesting adding http-keepalve=true to their uwsgi.ini config files when the correct usage from

If this isn't a bug where http-keepalive=true should correctly enable keep-alives, I propose updating the Native HTTP documentation w/ an example.

Existing documentation: https://github.com/unbit/uwsgi-docs/blob/master/HTTP.rst

Here is the my .ini config for reference. After a lot of trouble shooting and digging through the uWSGI source, the only change I needed to make for keep-alives to be respected was to change http-keepalive=true to http-keepalive=1.

[uwsgi]

;show-config=true

; don't allow unrecognized options in this file
strict=true

; Set up uwsgi http router on the specified address with http and tcp keepalives enabled
http=0.0.0.0:8080
http-keepalive=1
add-header=Connection: Keep-Alive
http-timeout=90
http-connect-timeout=5
;Enable TCP keepalive
so-keepalive=true
; Set socket send timeout in seconds for SO_SNDTIMEO
so-send-timeout=7200

; Exit if the app isn't found/can't be loaded
need-app=true
manage-script-name=true
mount=/myapp=myapp:app

; Enable a single thread with a top level master process
master=true
enable-threads=true
threads=1
processes=1
; Don't run as root, set uid to run as
uid=12345

; Graceful shutdown on sigterm
hook-master-start=unix_signal:15 gracefully_kill_them_all

; Log memory statistics
memory-report=true
@kbutz
Copy link
Author

kbutz commented Jun 21, 2019

If anyone knows if this is expected behavior or not, I would be happy to make the relevant changes.

If it is expected, I can update the documentation to make it more clear for the next person who wants to enable keep-alives from a wsgi.ini.

If it is unexpected, I can dust off my C skills and work on changing the config setting to a bool.

Thanks!

@xrmx
Copy link
Collaborator

xrmx commented Jun 22, 2019

Code suggests it should be a positive integer, so an hint about the type (seconds) would be nice to be added in help text in plugins/http/http.c.

@kbutz
Copy link
Author

kbutz commented Jun 23, 2019

Thanks for the info! I'll get a PR together soon w/ added documentation here, and an .ini example in the uwsgi-docs repo.

From what I recall during my initial debugging, the value of http-keepalive is used as a bool, and not for any other configuration, like timeout in seconds, or seconds to keep an unused connection pooled. Most documentation shows the keepalives being set with a simple bool flag --http-keepalive

I'll take another look, and add any relevant documentation to the PR.

@kbutz
Copy link
Author

kbutz commented Jun 24, 2019

Also related to: #1637

@kbutz
Copy link
Author

kbutz commented Jul 19, 2019

Closing this issue since #2032 got merged

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

2 participants