rgw: add enable_keep_alive option#41824
rgw: add enable_keep_alive option#41824a9QrX3Lu wants to merge 1 commit intoceph:masterfrom a9QrX3Lu:patch-tcp-alive
Conversation
This commit adds an enable_keep_alive option to control the keep_alive for the beast frontend. Fixes: https://tracker.ceph.com/issues/48402 Signed-off-by: Zulai Wang <zl31wang@gmail.com>
|
jenkins test make check |
| :Description: Optional value to enable connection to keep alive, either | ||
| "yes" or "no". It allows clients to reuse TCP connections | ||
| for subsequent HTTP requests, which improves performance. |
There was a problem hiding this comment.
this description confuses http keepalive with tcp keepalive
the frontend already supports http keepalive based on the http Connection header. see handle_connection() in rgw_asio_frontend.cc, which loops on parser.keep_alive()
this pr is enabling SO_KEEPALIVE on the socket, which controls tcp's keepalive. this differs from civetweb's enable_keep_alive option which controls http keepalive. is that really the intent here?
There was a problem hiding this comment.
You're right. I'll close this PR.
There was a problem hiding this comment.
Hi, I opened the issue https://tracker.ceph.com/issues/48402 and I can say that tcp keepalive is actually the intent here. So this PR looked like a fix for my issue, only the documentation part is wrong. Can this be reopened?
There was a problem hiding this comment.
hi @r0ss3, i added some thoughts to https://tracker.ceph.com/issues/48402, shall we continue the discussion there?
This commit adds an enable_keep_alive option to control the keep_alive for
the beast frontend.
Fixes: https://tracker.ceph.com/issues/48402
Signed-off-by: Zulai Wang zl31wang@gmail.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox