http: Release server before waiting for event base loop exit

promag commented at 2:35 pm on February 7, 2019: member

Call evhttp_free in the event loop to trigger server shutdown, in particular to close connections.

promag commented at 2:38 pm on February 7, 2019: member

@sdaftuar regarding #15305, if you cherry pick commit to your branch you’ll see the test runs in a couple of seconds.

Also, with the following patch:

0--- a/src/httpserver.cpp
1+++ b/src/httpserver.cpp
2@@ -465,6 +465,7 @@ void StopHTTPServer()
3     boundSockets.clear();
4     if (eventBase) {
5         LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n");
6+        event_base_dump_events(eventBase, stderr);
7         // Exit the event loop as soon as there are no active events.
8         event_base_loopexit(eventBase, nullptr);
9         threadHTTP.join();

and running

0test/functional/feature_abortnode.py --nocleanup

you will see (mind the path):

0cat /var/folders/1v/8_69hby54nj2k3n6fywt44x80000gn/T/bitcoin_func_test_o4hopx4l/node0/stderr/tmp*
1: Corrupted block database detected.
2Please restart with -reindex or -reindex-chainstate to recover.
3Inserted events:
4Active events:
5Error: Error: A fatal internal error occurred, see debug.log for details
6Inserted events:
7  0x7fb93d44b560 [fd  14] Read Persist Timeout=1549549849.031699
8Active events:

promag commented at 2:47 pm on February 7, 2019: member

Improves #15309.

promag force-pushed on Feb 7, 2019

promag commented at 3:24 pm on February 7, 2019: member

#15350 was merged, updated OP.

fanquake added the label RPC/REST/ZMQ on Feb 7, 2019

laanwj commented at 12:54 pm on February 8, 2019: member

We just can’t keep “fixing” this, can we?

promag renamed this:
~~http: Exit the event loop as soon as there are no active events~~
wip: http: Exit the event loop as soon as there are no active events
on Feb 8, 2019

promag commented at 1:20 pm on February 8, 2019: member

This is a different issue, idle connections are stalling shutdown. We could decrease the timeout duration but I don’t think that’s a fix.

This is incomplete.

ryanofsky commented at 7:40 pm on February 8, 2019: member

We just can’t keep “fixing” this, can we?

The libevent HTTP interface is kind of crazy, so I don’t think it’s surprising that fixing this requires some trial and error. But it would be good to have tests for each issue that gets fixed to avoid going in circles.

If it would be possible to specify what should happen on shutdown for connections in different states (close headers? truncated responses? timeouts?) and write a more comprehensive client test that would of course be great, too.

promag commented at 7:51 pm on February 8, 2019: member

@ryanofsky agree, and test/functional/feature_shutdown.py was added with that in mind. I’ll add test for this change.

BTW, I think the case introduced by @sdaftuar in #15305 is new because the shutdown isn’t triggered by stop command.

MarcoFalke added this to the milestone 0.18.0 on Feb 13, 2019

MarcoFalke removed this from the milestone 0.18.0 on Feb 18, 2019

MarcoFalke commented at 3:53 pm on February 18, 2019: member

Not sure if we should pull this into 0.18 at this point. Removing for now

promag commented at 4:21 pm on February 18, 2019: member

@MarcoFalke sure I agree. The issue is on shutdown so it shouldn’t bother too much to wait for connections to timeout - beside our functional tests.

promag force-pushed on Mar 11, 2019

MarcoFalke commented at 6:13 pm on July 29, 2019: member

What is the status of this?

It has nothing to do with #16405, right?

promag commented at 6:31 pm on July 31, 2019: member

It has nothing to do with #16405, right? @MarcoFalke no I mean right. Now that #15305 is merged, I’ll pick this again.

promag commented at 5:35 pm on August 2, 2019: member

Currently in master 3a3d8b835

0test/functional/feature_abortnode.py --nocleanup
1
22019-08-02T17:31:41.878000Z TestFramework (INFO): Initializing test directory /var/folders/dc/zhlby57d0vb9yrw5chl5wx6h0000gn/T/bitcoin_func_test_prjmeyz1
32019-08-02T17:31:42.898000Z TestFramework (INFO): Waiting for crash
42019-08-02T17:32:13.050000Z TestFramework (INFO): Node crashed - now verifying restart fails
52019-08-02T17:32:13.359000Z TestFramework (INFO): Stopping nodes
62019-08-02T17:32:13.725000Z TestFramework (WARNING): Not cleaning up dir /var/folders/dc/zhlby57d0vb9yrw5chl5wx6h0000gn/T/bitcoin_func_test_prjmeyz1
72019-08-02T17:32:13.725000Z TestFramework (INFO): Tests successful

It takes 30 seconds to timeout and only then the event base quits.

This PR fixes it but IMO in a wrong way. I will update shortly.

promag force-pushed on Aug 3, 2019

Sjors commented at 1:33 pm on August 14, 2019: member

When I run the functional test suite on a RAM disk it’s bottlenecked by the slowest running tests. I looked at some of those tests and saw they were stuck on Waiting for HTTP event thread to exit. This also happens without --use-cli, e.g. for wallet_multiwallet.py.

Cherry-picking this commit speeds up wallet_multiwallet.py --usecli from 60-120s to 40s on my machine.

DrahtBot commented at 8:29 pm on May 18, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19420 (http: Track active requests and wait for last to finish by promag)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Needs rebase on May 20, 2020

promag renamed this:
~~wip: http: Exit the event loop as soon as there are no active events~~
http: Exit the event loop as soon as there are no active events
on Jun 29, 2020

promag force-pushed on Jun 29, 2020

promag renamed this:
~~http: Exit the event loop as soon as there are no active events~~
http: Relese server before waiting for event base loop exit
on Jun 29, 2020

DrahtBot removed the label Needs rebase on Jun 29, 2020

promag force-pushed on Jun 30, 2020

http: Relese server before waiting for event base loop exit d044b7b086

promag force-pushed on Jun 30, 2020

promag commented at 10:45 am on June 30, 2020: member

Now test/functional/feature_abortnode.py runs pretty quick, it doesn’t have to wait for timeouts.

MarcoFalke renamed this:
~~http: Relese server before waiting for event base loop exit~~
http: Release server before waiting for event base loop exit
on Jun 30, 2020

promag commented at 11:13 am on July 1, 2020: member

For now closing in favor of #19420.

promag closed this on Jul 1, 2020

promag deleted the branch on Jul 1, 2020

DrahtBot locked this on Feb 15, 2022

http: Release server before waiting for event base loop exit #15363

Conflicts