From 16a5c18cea7330bd68dc9d2f768eb518af88795b Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Tue, 13 Jan 2015 17:43:39 -0800 Subject: [PATCH 1/3] Add a -rpckeepalive and disable RPC use of HTTP persistent connections. 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. --- src/init.cpp | 1 + src/rpcserver.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index d6f1e1cb9..f851fcbbc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -380,6 +380,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += " -rpcport= " + strprintf(_("Listen for JSON-RPC connections on (default: %u or testnet: %u)"), 8332, 18332) + "\n"; strUsage += " -rpcallowip= " + _("Allow JSON-RPC connections from specified source. Valid for 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= " + 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"; diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index a070ab5bb..9ebacdbe2 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -953,7 +953,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 From 56c1093dae0c523f9f643f00c67414691272a983 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 14 Jan 2015 12:45:11 +0100 Subject: [PATCH 2/3] fix tests for #5655 --- qa/rpc-tests/httpbasics.py | 3 +++ qa/rpc-tests/test_framework.py | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/httpbasics.py b/qa/rpc-tests/httpbasics.py index 85e85e7f0..7a9857a77 100755 --- a/qa/rpc-tests/httpbasics.py +++ b/qa/rpc-tests/httpbasics.py @@ -21,6 +21,9 @@ except ImportError: import urlparse class HTTPBasicsTest (BitcoinTestFramework): + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, extra_args=[['-rpckeepalive']]*4) + def run_test(self): ################################################# diff --git a/qa/rpc-tests/test_framework.py b/qa/rpc-tests/test_framework.py index 6c4ec073c..4c8a11b82 100755 --- a/qa/rpc-tests/test_framework.py +++ b/qa/rpc-tests/test_framework.py @@ -33,8 +33,11 @@ class BitcoinTestFramework(object): 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 From 1dd8ee72afc26191da51d8d3a5590eab7c9368f6 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 14 Jan 2015 16:36:48 +0100 Subject: [PATCH 3/3] improve tests for #5655 --- qa/rpc-tests/httpbasics.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/httpbasics.py b/qa/rpc-tests/httpbasics.py index 7a9857a77..8cbf1d7f4 100755 --- a/qa/rpc-tests/httpbasics.py +++ b/qa/rpc-tests/httpbasics.py @@ -22,7 +22,7 @@ except ImportError: class HTTPBasicsTest (BitcoinTestFramework): def setup_nodes(self): - return start_nodes(4, self.options.tmpdir, extra_args=[['-rpckeepalive']]*4) + return start_nodes(4, self.options.tmpdir, extra_args=[['-rpckeepalive=1'], ['-rpckeepalive=0'], [], []]) def run_test(self): @@ -74,6 +74,29 @@ class HTTPBasicsTest (BitcoinTestFramework): 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__': HTTPBasicsTest ().main ()