Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Jun 8, 2018

The portseed_offset is no longer needed in the test runner, since we already kill leftover processes (see #12904). This "fixes" #10869 because we deterministically pick ports starting at 11000

@maflcko maflcko requested a review from jnewbery June 8, 2018 17:34
@fanquake fanquake added the Tests label Jun 9, 2018
@laanwj
Copy link
Member

laanwj commented Jun 11, 2018

utACK fa6edfe I think making the port range more deterministic is, overall, a good thing. We don't support multiple test_runner instances at the same time, and if we did, it would be better as an explicit option to change the port offset.

@laanwj laanwj merged commit fa6edfe into bitcoin:master Jun 11, 2018
laanwj added a commit that referenced this pull request Jun 11, 2018
fa6edfe qa: Remove portseed_offset from test runner (MarcoFalke)

Pull request description:

  The portseed_offset is no longer needed in the test runner, since we already kill leftover processes (see #12904). This "fixes" #10869 because we deterministically pick ports starting at 11000

Tree-SHA512: 1ee22e19e02acd3afadc7c6a2b391fd3b5cfcec22c0fe194f3207251e7b1264a04e47d90a3ff8be4aca7d0ec33219a2f5855076acb3565291767939bc2f2fa17
@maflcko maflcko deleted the Mf1806-qaPortseedOffset branch June 11, 2018 12:41
@laanwj
Copy link
Member

laanwj commented Jun 11, 2018

Something that might be useful is to do an initial sweep over all the ports that are going to be used, so if something is in the way, there's an early warning/error.

UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jun 28, 2021
fa6edfe qa: Remove portseed_offset from test runner (MarcoFalke)

Pull request description:

  The portseed_offset is no longer needed in the test runner, since we already kill leftover processes (see bitcoin#12904). This "fixes" bitcoin#10869 because we deterministically pick ports starting at 11000

Tree-SHA512: 1ee22e19e02acd3afadc7c6a2b391fd3b5cfcec22c0fe194f3207251e7b1264a04e47d90a3ff8be4aca7d0ec33219a2f5855076acb3565291767939bc2f2fa17
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 8, 2021
Summary:
PR description:
> The portseed_offset is no longer needed in the test runner, since we already kill leftover processes

The pseudo-random portseed causes port collisions with other daemons. Core devs had issues with port 11211 (memcached), and we are seeing issues with 21325 (trezord).
This backport is a first step in avoiding random failures, by making the ports more deterministic again.

We cannot use the number of tests as a unique seed for Bitcoin ABC like Core does, because we run tests in parallel, so use the test number instead.

This is a backport of [[bitcoin/bitcoin#13421 | core#13421]]

Test Plan: `for i in {1..4}; do ninja check-functional; done`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D9759
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants