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

jewel: rgw: work around curl_multi_wait bug with non-blocking reads #11627

Merged
10 commits merged into from Nov 11, 2016

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Oct 24, 2016

pulls in some earlier fixes to resolve merge conflicts

Backport tracker: http://tracker.ceph.com/issues/17343
http://tracker.ceph.com/issues/17846

@cbodley cbodley added the rgw label Oct 24, 2016
@cbodley cbodley added this to the jewel milestone Oct 24, 2016
@cbodley
Copy link
Contributor Author

cbodley commented Oct 24, 2016

closing until #11630 merges to master - it should be backported at the same time

@cbodley cbodley closed this Oct 24, 2016
@cbodley cbodley reopened this Oct 26, 2016
@ghost ghost changed the base branch from jewel to jewel-next November 9, 2016 09:57
@ghost
Copy link

ghost commented Nov 9, 2016

@cbodley could you please rebase against jewel-next ? A number of backports have been merged since the last check and it would be good to get another make check with a more recent rebase.

@ghost ghost self-assigned this Nov 9, 2016
@ghost ghost added the bug-fix label Nov 9, 2016
cbodley and others added 9 commits November 9, 2016 10:29
closes a potential race between pipe creation and
RGWHTTPManager::reqs_thread_entry()'s access of thread_pipe

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 9161e9f)
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit c93959b)
when HAVE_CURL_MULTI_WAIT is 0, the pipe fd is never added to the
readfds for select(), so FD_ISSET() is always false. this prevents us
from ever trying to read from the fd, and the pipe's buffer eventually
fills up and deadlocks callers of RGWHTTPManager::signal_thread() when
they try to write to the pipe

Fixes: http://tracker.ceph.com/issues/16368

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 75897f8)
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 2080984)
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit f2f5cdf)
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 1101fcc)
Fixes: http://tracker.ceph.com/issues/15915
Fixes: http://tracker.ceph.com/issues/16695

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 0359be6)
Signed-off-by: John Coyle <dx9err@gmail.com>
(cherry picked from commit 00f4554)
Reported-by: Ken Dreyer <kdreyer@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit dcff120)
@cbodley
Copy link
Contributor Author

cbodley commented Nov 9, 2016

thanks @dachary, rebased and waiting on checks

@ghost
Copy link

ghost commented Nov 9, 2016

jenkins test this please (general jenkins failure)

@ghost
Copy link

ghost commented Nov 9, 2016

@cbodley

CXX      rgw/librgw_la-rgw_metadata.lo
rgw/rgw_http_client.cc:365:8: error: 'once_flag' in namespace 'std' does not name a type
 static std::once_flag detect_flag;
        ^
rgw/rgw_http_client.cc: In member function 'int RGWHTTPManager::set_threaded()':
rgw/rgw_http_client.cc:805:3: error: 'call_once' is not a member of 'std'
   std::call_once(detect_flag, detect_curl_multi_wait_bug, cct,
   ^
rgw/rgw_http_client.cc:805:18: error: 'detect_flag' was not declared in this scope
   std::call_once(detect_flag, detect_curl_multi_wait_bug, cct,
                  ^
make[3]: *** [rgw/librgw_la-rgw_http_client.lo] Error 1

@ghost ghost changed the title jewel: rgw: work around curl_multi_wait bug with non-blocking reads DNM: jewel: rgw: work around curl_multi_wait bug with non-blocking reads Nov 9, 2016
@cbodley
Copy link
Contributor Author

cbodley commented Nov 9, 2016

@dachary eek, interesting.. it must be getting the <mutex> header from somewhere else in master. i added a commit for #include <mutex>, hopefully that helps

@ghost ghost changed the title DNM: jewel: rgw: work around curl_multi_wait bug with non-blocking reads jewel: rgw: work around curl_multi_wait bug with non-blocking reads Nov 9, 2016
@ghost
Copy link

ghost commented Nov 9, 2016

@cbodley yeah !

@ghost
Copy link

ghost commented Nov 9, 2016

@cbodley could you please amend the commit message of 47abc13 to explain why it has to be a commit unique to jewel and not a cherry-pick -x of a commit from master ?

this fix was added directly to the jewel branch rather than backporting
from master, because the code on master compiles without this specific
include - it's likely included by another header, but backporting would
involve pulling in unrelated changes

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Nov 9, 2016

@dachary added this:

this fix was added directly to the jewel branch rather than backporting
from master, because the code on master compiles without this specific
include - it's likely included by another header, but backporting would
involve pulling in unrelated changes

sound okay?

ghost pushed a commit that referenced this pull request Nov 9, 2016
…g with non-blocking reads

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost
Copy link

ghost commented Nov 9, 2016

@cbodley yes, thanks !

@ghost
Copy link

ghost commented Nov 9, 2016

hey jenkins, do you best this time, test this please (jenkins general failure)

@cbodley
Copy link
Contributor Author

cbodley commented Nov 10, 2016

well done jenkins, i knew you had it in you! :)

@ghost
Copy link

ghost commented Nov 11, 2016

It passed the rgw suite http://tracker.ceph.com/issues/17851#note-3 . Note that it will not be in 10.2.4, reason why it targets jewel-next.

@ghost ghost merged commit cfca2a4 into ceph:jewel-next Nov 11, 2016
@cbodley cbodley deleted the wip-17343 branch November 11, 2016 14:20
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants