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

libevent http fixes #6695

Merged
merged 4 commits into from Sep 21, 2015

Conversation

Projects
None yet
4 participants
@laanwj
Member

laanwj commented Sep 18, 2015

Fix a few rough edges discovered in use of the new libevent http server:;

init: Ignore SIGPIPE

Ignore SIGPIPE on all non-win32 OSes, otherwise an unexpectedly disconnecting
RPC client will terminate the application. This problem was introduced
with the libhttp-based RPC server.

Fixes #6660.

http: Disable libevent debug logging, if not explicitly enabled

Add a option "-debug=libevent" to enable libevent debugging for troubleshooting.
Libevent logging is redirected to our own log.

Addresses stdout spew in #6655.

rpc: Split option -rpctimeout into -rpcservertimeout and -rpcclienttimeout

The two timeouts for the server and client, are essentially different:

  • In the case of the server it should be a lower value to avoid clients
    clogging up connection slots
  • In the case of the client it should be a high value to accomedate slow
    responses from the server, for example for slow queries or when the
    lock is contended

Split the options into -rpcservertimeout and -rpcclienttimeout with
respective defaults of 30 and 900.

Should fix, or make it possible to fix timeout issues in #6655.

init: Ignore SIGPIPE
Ignore SIGPIPE on all non-win32 OSes, otherwise an unexpectedly disconnecting
RPC client will terminate the application. This problem was introduced
with the libhttp-based RPC server.

Fixes #6660.

@laanwj laanwj added the RPC/REST/ZMQ label Sep 18, 2015

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Sep 18, 2015

Member

utACK (plans to test this soon).
Not sure about the insides of getblocktemplate, but hows does the rpctimeout affect the long poll?

Member

jonasschnelli commented Sep 18, 2015

utACK (plans to test this soon).
Not sure about the insides of getblocktemplate, but hows does the rpctimeout affect the long poll?

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 18, 2015

Member

Not sure about the insides of getblocktemplate, but hows does the rpctimeout affect the long poll?

On the server side it doesn't affect that - it doesn't spend time waiting for input from the client.
On the client side, the request will time out if the reply takes longer than rpcclienttimeout.

Member

laanwj commented Sep 18, 2015

Not sure about the insides of getblocktemplate, but hows does the rpctimeout affect the long poll?

On the server side it doesn't affect that - it doesn't spend time waiting for input from the client.
On the client side, the request will time out if the reply takes longer than rpcclienttimeout.

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Sep 18, 2015

Member

Unfortunately I'm 0/2 on testing these. I couldn't recreate the SIGPIPE issue, in master or here, so thats's a start. But I still get what looks like a timeout from getblocktemplate_longpoll.py and I still get errors spewed to stdout when I don't have an IPv6 address.

EDIT: as per IRC:

  • SIGPIPE appears to be an OS X issue, which is why I couldn't recreate.
  • older versions of libevent don't support turning off the function call to turn off debugging
Member

morcos commented Sep 18, 2015

Unfortunately I'm 0/2 on testing these. I couldn't recreate the SIGPIPE issue, in master or here, so thats's a start. But I still get what looks like a timeout from getblocktemplate_longpoll.py and I still get errors spewed to stdout when I don't have an IPv6 address.

EDIT: as per IRC:

  • SIGPIPE appears to be an OS X issue, which is why I couldn't recreate.
  • older versions of libevent don't support turning off the function call to turn off debugging
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 18, 2015

Member

From IRC:

<morcos> regarding the rpc timeouts..  before 6695, you could fix the problem by starting the bitcoind's in the python tests with -rpctimeout=1500 (or whatever)
<morcos> doesn't that imply that somehow it was the timeout on the server end that was the problem?

Looks like the issue is not in bitcoin-cli but in the python http functionality that cannot cope with disconnects between requests (as said by @sdaftuar ). Which would be strange, but I'll take a look at it.

Member

laanwj commented Sep 18, 2015

From IRC:

<morcos> regarding the rpc timeouts..  before 6695, you could fix the problem by starting the bitcoind's in the python tests with -rpctimeout=1500 (or whatever)
<morcos> doesn't that imply that somehow it was the timeout on the server end that was the problem?

Looks like the issue is not in bitcoin-cli but in the python http functionality that cannot cope with disconnects between requests (as said by @sdaftuar ). Which would be strange, but I'll take a look at it.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 18, 2015

Member

The rpcbind.py test hang in #6655 is caused by bitcoin-cli being unable to connect to IPv6 IPs. Not sure why. Also not sure whether this affects all libevent versions. But I can reproduce it with libevent 0x02001500 on Ubuntu 14.04 (which is relatively ancient).

Member

laanwj commented Sep 18, 2015

The rpcbind.py test hang in #6655 is caused by bitcoin-cli being unable to connect to IPv6 IPs. Not sure why. Also not sure whether this affects all libevent versions. But I can reproduce it with libevent 0x02001500 on Ubuntu 14.04 (which is relatively ancient).

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Sep 18, 2015

Member

SIGPIPE appears to be an OS X issue, which is why I couldn't recreate.

I could reproduce it on debian as well. Use a bitcoin-cli command like getblocktemplate or getrawmempool and enter ctrl-C short after sending-off the command. Very hard to reproduce (requires a few tries).
With a script it would probably easy.

The SIGPIPE gets emitted when bitcoind starts to write to a closed socket (ctrl-c will terminate bitcoin-cli which ends up in writing to a closed socket on the server side).

Member

jonasschnelli commented Sep 18, 2015

SIGPIPE appears to be an OS X issue, which is why I couldn't recreate.

I could reproduce it on debian as well. Use a bitcoin-cli command like getblocktemplate or getrawmempool and enter ctrl-C short after sending-off the command. Very hard to reproduce (requires a few tries).
With a script it would probably easy.

The SIGPIPE gets emitted when bitcoind starts to write to a closed socket (ctrl-c will terminate bitcoin-cli which ends up in writing to a closed socket on the server side).

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Sep 18, 2015

Member

@laanwj I know I said I didn't care, but I tracked down the silly warning messages. We need to call event_set_log_callback. I tried it and it works.

screen shot 2015-09-18 at 6 11 17 pm

Member

morcos commented Sep 18, 2015

@laanwj I know I said I didn't care, but I tracked down the silly warning messages. We need to call event_set_log_callback. I tried it and it works.

screen shot 2015-09-18 at 6 11 17 pm

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Sep 19, 2015

Contributor

concept ACK

Contributor

dcousens commented Sep 19, 2015

concept ACK

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 19, 2015

Member

@morcos Ah, that'd allow redirecting the messages to our own log. That makes sense in addition to what is done here.

Edit: done, the option is -debug=libevent now

Member

laanwj commented Sep 19, 2015

@morcos Ah, that'd allow redirecting the messages to our own log. That makes sense in addition to what is done here.

Edit: done, the option is -debug=libevent now

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 19, 2015

Member

The Python BadStatusLine on disconnect issue sounds like https://bugs.python.org/issue3566 "httplib persistent connections violate MUST in RFC2616 sec 8.1.4."

This was fixed in Python 3.5.

I've pushed a commit that should work around it.

Member

laanwj commented Sep 19, 2015

The Python BadStatusLine on disconnect issue sounds like https://bugs.python.org/issue3566 "httplib persistent connections violate MUST in RFC2616 sec 8.1.4."

This was fixed in Python 3.5.

I've pushed a commit that should work around it.

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Sep 20, 2015

Member

ACK (modulo missing newlines)

Member

morcos commented Sep 20, 2015

ACK (modulo missing newlines)

http: Disable libevent debug logging, if not explicitly enabled
Add a option "-debug=libevent" to enable libevent debugging for troubleshooting.
Libevent logging is redirected to our own log.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 21, 2015

Member

Fixed the newline issue (thanks @morcos), removed a spurious change in util.py

Member

laanwj commented Sep 21, 2015

Fixed the newline issue (thanks @morcos), removed a spurious change in util.py

laanwj added some commits Sep 18, 2015

rpc: Split option -rpctimeout into -rpcservertimeout and -rpcclientti…
…meout

The two timeouts for the server and client, are essentially different:

- In the case of the server it should be a lower value to avoid clients
clogging up connection slots

- In the case of the client it should be a high value to accomedate slow
  responses from the server, for example for slow queries or when the
  lock is contended

Split the options into `-rpcservertimeout` and `-rpcclienttimeout` with
respective defaults of 30 and 900.
Make RPC tests cope with server-side timeout between requests
Python's httplib does not graciously handle disconnections from the http server, resulting in BadStatusLine errors.
See https://bugs.python.org/issue3566 "httplib persistent connections violate MUST in RFC2616 sec 8.1.4."

This was fixed in Python 3.5.

Work around it for now.

@laanwj laanwj merged commit ddf98d1 into bitcoin:master Sep 21, 2015

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

laanwj added a commit that referenced this pull request Sep 21, 2015

Merge pull request #6695
ddf98d1 Make RPC tests cope with server-side timeout between requests (Wladimir J. van der Laan)
2190ea6 rpc: Split option -rpctimeout into -rpcservertimeout and -rpcclienttimeout (Wladimir J. van der Laan)
8b2d6ed http: Disable libevent debug logging, if not explicitly enabled (Wladimir J. van der Laan)
5ce43da init: Ignore SIGPIPE (Wladimir J. van der Laan)

@str4d str4d referenced this pull request Mar 22, 2017

Merged

libevent-based http server #2176

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment