Page MenuHomePhabricator

Merge #14733: P2P: Make peer timeout configurable, speed up very slow test and ensure correct code path tested.
ClosedPublic

Authored by jasonbcox on Apr 24 2020, 17:42.

Details

Summary

48b37db50 make peertimeout a debug argument, remove error message translation (Zain Iqbal Allarakhia)
8042bbfbf p2p: allow p2ptimeout to be configurable, speed up slow test (Zain Iqbal Allarakhia)

Pull request description:

**Summary:**

1. _Primary_: Adds a `debug_only=true` flag for peertimeout, defaults to 60 sec., the current hard-coded setting.
2. _Secondary_: Drastically speeds up `p2p_timeout.py` test.
3. _Secondary_: Tests that the correct code path is being tested by adding log assertions to the test.

**Rationale:**

- P2P timeout was hard-coded: make it explicitly specified and configurable, instead of a magic number.
- Addresses #13518; `p2p_timeout.py` takes 4 sec. to run instead of 61 sec.
- Makes `p2p_timeout.py` more explicit. Previously, we relied on a comment to inform us of the timeout amount being tested. Now it is specified directly in the test via passing in the new arg; `-peertimeout=3`.
- Opens us up to testing more P2P scenarios; oftentimes slow tests are the reason we don't test.

**Locally verified changes:**

_With Proposed Change (4.7 sec.):_
```
$ time ./test/functional/p2p_timeouts.py
2018-11-19T00:04:19.077000Z TestFramework (INFO): Initializing test directory /tmp/testhja7g2n7
2018-11-19T00:04:23.479000Z TestFramework (INFO): Stopping nodes
2018-11-19T00:04:23.683000Z TestFramework (INFO): Cleaning up /tmp/testhja7g2n7 on exit
2018-11-19T00:04:23.683000Z TestFramework (INFO): Tests successful

real    0m4.743s
```

_Currently  on master (62.8 sec.):_
```
$ time ./test/functional/p2p_timeouts.py
2018-11-19T00:06:10.948000Z TestFramework (INFO): Initializing test directory /tmp/test6mo6k21h
2018-11-19T00:07:13.376000Z TestFramework (INFO): Stopping nodes
2018-11-19T00:07:13.631000Z TestFramework (INFO): Cleaning up /tmp/test6mo6k21h on exit
2018-11-19T00:07:13.631000Z TestFramework (INFO): Tests successful

real    1m2.836s
```

_Error message demonstrated for new argument `-peertimeout`:_
```
$ ./bitcoind -peertimeout=-5
...
Error: peertimeout cannot be configured with a negative value.
```

Tree-SHA512: ff7a244ebea54c4059407bf4fb86465714e6a79cef5d2bcaa22cfe831a81761aaf597ba4d5172fc2ec12266f54712216fc41b5d24849e5d9dab39ba6f09e3a2a

Backport of Core PR14733

Test Plan
ninja check
test_runner.py
test_runner.py p2p_timeouts  # verify it completes in ~4 seconds

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pr14733
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10381
Build 18583: Default Diff Build & Tests
Build 18582: arc lint + arc unit

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Apr 24 2020, 20:54