Skip to content

Commit

Permalink
Add a -rpckeepalive and disable RPC use of HTTP persistent connections.
Browse files Browse the repository at this point in the history
It turns out that some miners have been staying with old versions of
 Bitcoin Core because their software  behaves poorly with persistent
 connections and the Bitcoin Core thread and connection limits.

What happens is that underlying  HTTP libraries leave connections open
 invisibly to their users and then the user runs into the default four
 thread limit.  This looks like Bitcoin Core is unresponsive to RPC.

There are many things that should be improved in Bitcoin Core's behavior
 here, e.g. supporting more concurrent connections, not tying up threads
 for idle connections, disconnecting kept-alive  connections when limits
 are reached, etc. All are fairly big, risky changes.

Disabling keep-alive is a simple workaround. It's often not easy to turn
 off the keep-alive support in the client where it may be buried in some
 platform library.

If you are one of the few who really needs persistent connections you
 probably know that you want them and can find a switch; while if you
 don't and the misbehavior is hitting you it is hard to discover the
 source of your problems is keepalive related.  Given that it is best
 to default to off until they're handled better.

Github-Merge: #5655
Rebased-From: 16a5c18 56c1093 1dd8ee7
  • Loading branch information
gmaxwell authored and laanwj committed Jan 15, 2015
1 parent 249bf0e commit aaf55d2
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
30 changes: 28 additions & 2 deletions qa/rpc-tests/httpbasics.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
except ImportError:
import urlparse

class RESTTest (BitcoinTestFramework):
class HTTPBasicsTest (BitcoinTestFramework):
def setup_nodes(self):
return start_nodes(4, self.options.tmpdir, extra_args=[['-rpckeepalive=1'], ['-rpckeepalive=0'], [], []])

def run_test(self):

#################################################
Expand Down Expand Up @@ -71,6 +74,29 @@ def run_test(self):
assert_equal('"error":null' in out1, True)
assert_equal(conn.sock!=None, False) #now the connection must be closed after the response

#node1 (2nd node) is running with disabled keep-alive option
urlNode1 = urlparse.urlparse(self.nodes[1].url)
authpair = urlNode1.username + ':' + urlNode1.password
headers = {"Authorization": "Basic " + base64.b64encode(authpair)}

conn = httplib.HTTPConnection(urlNode1.hostname, urlNode1.port)
conn.connect()
conn.request('GET', '/', '{"method": "getbestblockhash"}', headers)
out1 = conn.getresponse().read();
assert_equal('"error":null' in out1, True)
assert_equal(conn.sock!=None, False) #connection must be closed because keep-alive was set to false

#node2 (third node) is running with standard keep-alive parameters which means keep-alive is off
urlNode2 = urlparse.urlparse(self.nodes[2].url)
authpair = urlNode2.username + ':' + urlNode2.password
headers = {"Authorization": "Basic " + base64.b64encode(authpair)}

conn = httplib.HTTPConnection(urlNode2.hostname, urlNode2.port)
conn.connect()
conn.request('GET', '/', '{"method": "getbestblockhash"}', headers)
out1 = conn.getresponse().read();
assert_equal('"error":null' in out1, True)
assert_equal(conn.sock!=None, False) #connection must be closed because bitcoind should use keep-alive by default

if __name__ == '__main__':
RESTTest ().main ()
HTTPBasicsTest ().main ()
5 changes: 4 additions & 1 deletion qa/rpc-tests/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ def setup_chain(self):
print("Initializing test directory "+self.options.tmpdir)
initialize_chain(self.options.tmpdir)

def setup_nodes(self):
return start_nodes(4, self.options.tmpdir)

def setup_network(self, split = False):
self.nodes = start_nodes(4, self.options.tmpdir)
self.nodes = self.setup_nodes()

# Connect the nodes as a "chain". This allows us
# to split the network between nodes 1 and 2 to get
Expand Down
1 change: 1 addition & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += " -rpcport=<port> " + strprintf(_("Listen for JSON-RPC connections on <port> (default: %u or testnet: %u)"), 8332, 18332) + "\n";
strUsage += " -rpcallowip=<ip> " + _("Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times") + "\n";
strUsage += " -rpcthreads=<n> " + strprintf(_("Set the number of threads to service RPC calls (default: %d)"), 4) + "\n";
strUsage += " -rpckeepalive " + strprintf(_("RPC support for HTTP persistent connections (default: %d)"), 0) + "\n";

strUsage += "\n" + _("RPC SSL options: (see the Bitcoin Wiki for SSL setup instructions)") + "\n";
strUsage += " -rpcssl " + _("Use OpenSSL (https) for JSON-RPC connections") + "\n";
Expand Down
2 changes: 1 addition & 1 deletion src/rpcserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ void ServiceConnection(AcceptedConnection *conn)
ReadHTTPMessage(conn->stream(), mapHeaders, strRequest, nProto, MAX_SIZE);

// HTTP Keep-Alive is false; close connection immediately
if (mapHeaders["connection"] == "close")
if ((mapHeaders["connection"] == "close") || (!GetBoolArg("-rpckeepalive", false)))
fRun = false;

// Process via JSON-RPC API
Expand Down

0 comments on commit aaf55d2

Please sign in to comment.