-
Notifications
You must be signed in to change notification settings - Fork 166
Make HTTP's maximum open delay configurable #2532
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
Conversation
abh3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is all about avoiding behaviour changes and using consistent handling of directives. It also winds up being less code.
|
@abh3 - I believe all your review comments have been addressed in the latest branch revision and requested a new review in the GitHub interface. Thanks! |
abh3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks.
|
@amadio - question about things tagged I understand |
The tags don't say much about the state of the work per se. It works as a reminder for me not to forget to look at it when releasing that particular release, and that the current plan is to include it into that particular release, unless something prevents it from happening (could be because it's not ready yet, or some other reason, like needing to move the release earlier to push out important fixes). In any case, if a change tagged for 5.9 doesn't make, we'd tag it to the next release where we intend to include it. As for branches, |
By default, the bridge will retry operations internally until the maximum delay is hit and then return an error. By default, the maximum delay is 3600 seconds; this is well beyond the timeouts of a typical HTTP client. This makes the delay configurable for HTTP with the following: ``` http.maxdelay <seconds> ``` as in ``` http.maxdelay 10 ``` It also tunes down the default delay to 30s. Note: the response time can still be greater than 30s; this just limits how long the bridge will process "wait" responses for.
|
Hi @bbockelm . I happened to also see some not desirable behvior around the wait retry, while testing fix(es) for #2602. We do not cancel or break out of the redrive incase the http client has gone away (the redrive job will wait for the delay period, reissue its XrdXrootdProtocol reqeust, and if the result is still kxr_wait another redrive is scheduled etc.). If there was a situation where say, Open was returning kxr_wait(some small value) for a period of e.g. 5 minutes, and there was a e.g. a population of 100 http clients reqeusting a file with a client-side timeout of 10 seconds with immediate retry, we chould have 3000 redrive jobs scheduled (and network fd's open). I haven't found a proposal for properly detecting that the http client has gone. We could detect, e.g. POLLRDHUP from poll to indicate client gone, but that seems not a safe assuption, as an http client could have shutdown(fd,SHUT_WR) its connection, but still be reading for the request result on the half open conneciton. |
By default, the bridge will retry operations internally until the maximum delay is hit and then return an error. By default, the maximum delay is 3600 seconds; this is well beyond the timeouts of a typical HTTP client.
This makes the delay configurable for HTTP with the following:
as in
It also tunes down the default delay to 30s.
Note: the response time can still be greater than 30s; this just limits how long the bridge will process "wait" responses for.