Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Fix intermittent rpc_net issue #20299

Merged
merged 1 commit into from
Nov 4, 2020
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 4, 2020

The test fails because getpeerinfo and getnettotals are not synchronised, so a wait_until is needed for each RPC (separately).

Fixes https://cirrus-ci.com/task/4663366629195776?command=ci#L5034

@jnewbery
Copy link
Contributor

jnewbery commented Nov 4, 2020

utACK face870

Thanks for fixing this!

I don't quite understand the "get the id from the peer info and then get the peer info from the id" step. Could be simplified with:

--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -113,11 +113,10 @@ class NetTest(BitcoinTestFramework):
         self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytessent'] >= net_totals_before['totalbytessent'] + 32 * 2), timeout=1)
         self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytesrecv'] >= net_totals_before['totalbytesrecv'] + 32 * 2), timeout=1)
 
-        for peer_id in [p['id'] for p in peer_info_before]:
-            before = next(p for p in peer_info_before if p['id'] == peer_id)
-            after = lambda: next(p for p in self.nodes[0].getpeerinfo() if p['id'] == peer_id)
-            self.wait_until(lambda: after()['bytesrecv_per_msg'].get('pong', 0) >= before['bytesrecv_per_msg'].get('pong', 0) + 32, timeout=1)
-            self.wait_until(lambda: after()['bytessent_per_msg'].get('ping', 0) >= before['bytessent_per_msg'].get('ping', 0) + 32, timeout=1)
+        for peer_before in peer_info_before:
+            peer_after = lambda: next(p for p in self.nodes[0].getpeerinfo() if p['id'] == peer_before['id'])
+            self.wait_until(lambda: peer_after()['bytesrecv_per_msg'].get('pong', 0) >= peer_before['bytesrecv_per_msg'].get('pong', 0) + 32, timeout=1)
+            self.wait_until(lambda: peer_after()['bytessent_per_msg'].get('ping', 0) >= peer_before['bytessent_per_msg'].get('ping', 0) + 32, timeout=1)
 
     def test_getnetworkinfo(self):
         self.log.info("Test getnetworkinfo")

@jnewbery
Copy link
Contributor

jnewbery commented Nov 4, 2020

utACK fa2ecad

@maflcko maflcko merged commit ed9f547 into bitcoin:master Nov 4, 2020
@maflcko maflcko deleted the 2011-testNet branch November 4, 2020 16:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 5, 2020
fa2ecad test: Fix intermittent rpc_net issue (MarcoFalke)

Pull request description:

  The test fails because getpeerinfo and getnettotals are not synchronised, so a `wait_until` is needed for each RPC (separately).

  Fixes https://cirrus-ci.com/task/4663366629195776?command=ci#L5034

ACKs for top commit:
  jnewbery:
    utACK fa2ecad

Tree-SHA512: 5ea7128801aab8dbe3d9e6737545ff4ee770e4a9c5a2096ba2339a688424f1879ccba6bf8bcb219983acf86eb28af06fc629586613e7fe28aeffadd2c98633e8
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 21, 2021
Summary:
[[bitcoin/bitcoin#20258 | core#20258]]:
> The `CNode` member `nSendBytes` is incremented under the node's lock `cs_vSend`. However, `RecordBytesSent` is not. An RPC thread that acquires the `cs_vSend` lock puts the message handler thread on hold. While the thread is on hold, `getnettotals` returns "stale" values or values that don't add up.
>
> We make no guarantees about consistency between RPC calls.

[[bitcoin/bitcoin#20299 | core#20299]]
> The test fails intermittently because getpeerinfo and getnettotals are not synchronised, so a wait_until is needed for each RPC (separately).

This is a backport of [[bitcoin/bitcoin#20258 | core#20258]] and [[bitcoin/bitcoin#20299 | core#20299]]

Test Plan: `test/functional/test_runner.py rpc_net.py`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10706
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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.

None yet

3 participants