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

rgw/http: add timeout to http client #34414

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

yuvalif
Copy link
Contributor

@yuvalif yuvalif commented Apr 6, 2020

some more http client related fixes:

  • prevent "Expect: 100-continue" from being sent when not needed
  • fixed a small issue with the wait on the condition, where the done value was not checked. This is needed to fix "spurious wakeups"
  • CURLOPT_INFILESIZE was set with a pointer value and not the actual size

Following Issues are not blockers for notification testing (e.g. #34293), and could be addressed later:

  • According to this setting the post field should be sufficient to prevent from the Expect: 100-continue to be sent. However, tests showed the the field is sent regardless. So, to prevent it it was removed explicitly from the header
  • If a server respect that field, and actually answer with 100 Continue result code, the RGW treat that as an -EIO error.If the server ignore that, there is no issue
  • if uploaded file size is more than 2GB, different option should be used

Signed-off-by: Yuval Lifshitz yuvalif@yahoo.com

Copy link
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the POST stuff looks right 👍 there's also CURLOPT_POST, but sounds like that would make it harder to reuse the handle for later requests

src/rgw/rgw_http_client.cc Outdated Show resolved Hide resolved
src/rgw/rgw_http_client.cc Outdated Show resolved Hide resolved
@tchaikov
Copy link
Contributor

tchaikov commented Apr 7, 2020

jenkins, retest this please.

also, prevent "Expect: 100-continue" from being sent
when not needed

Signed-off-by: Yuval Lifshitz <yuvalif@yahoo.com>
@yuvalif yuvalif force-pushed the add_timeout_to_http_client branch from 5266b5f to dd49cc8 Compare April 7, 2020 09:23
@yuvalif yuvalif added needs-qa and removed DNM labels Apr 7, 2020
@yuvalif
Copy link
Contributor Author

yuvalif commented Apr 7, 2020

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