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

Fix rare failures in new_outbound_peer_connections_are_rate_limited test #2450

Closed
teor2345 opened this issue Jul 6, 2021 · 1 comment
Closed
Labels
C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jul 6, 2021

Description

In rare cases, the new_outbound_peer_connections_are_rate_limited proptest fails:

all candidates should obey the minimum rate-limit: now: Instant { t: 4629.9003711s } min: Instant { t: 4629.9022902s }

https://github.com/ZcashFoundation/zebra/runs/3003578821#step:10:1734

Fixing this bug is a low priority, unless it occurs much more frequently.

Steps to Reproduce

It is unclear why the test fails, because the assertions don't log the candidate number, or the times of the previous candidates.

There is no proptest seed, because the test uses assert!, rather than prop_assert! (#2271). But there are no times in the generated test case, so this failure might not be reproducible.

It's possible that this failure is caused by high CPU load on the test machine, or a non-monotonic system clock.

Windows 2021-07-07

Zebra Logs

---- peer_set::candidate_set::tests::prop::new_outbound_peer_connections_are_rate_limited stdout ----

The application panicked (crashed).
Message:  all candidates should obey the minimum rate-limit: now: Instant { t: 4629.9003711s } min: Instant { t: 4629.9022902s }
Location: zebra-network\src\peer_set\candidate_set\tests\prop.rs:158

https://github.com/ZcashFoundation/zebra/runs/3003578821#step:10:1734

Message:  Test failed: all candidates should obey the minimum rate-limit: now: Instant { t: 4629.9003711s } min: Instant { t: 4629.9022902s }; minimal failing input: peers = [NewAlternate { addr: 96.204.16.147:6036, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 127.0.0.1:12331, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 222.128.102.9:38733, untrusted_services: NODE_NETWORK }, NewAlternate { addr: [4df5:6aab:ede6:713c:ad42:96b:f179:1ae]:10900, untrusted_services: NODE_NETWORK }, NewAlternate { addr: [c694:b778:5ef2:f095:14af:db77:a235:b3aa]:7390, untrusted_services: NODE_NETWORK }, NewAlternate { addr: [f31f:b4ae:bdd0:4a6e:84f0:517a:35c:984c]:37173, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 67.207.60.74:45045, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 100.101.245.233:58059, untrusted_services: NODE_NETWORK }], initial_candidates = 3, extra_candidates = 3
	successes: 15
	local rejects: 5
		5 times at failed MetaAddr::is_valid_for_outbound
	global rejects: 0

https://github.com/ZcashFoundation/zebra/runs/3003578821#step:10:1835

Environment

Microsoft Windows Server 2019
10.0.17763
Datacenter

Windows is known to be non-monotonic:
https://doc.rust-lang.org/src/std/time.rs.html#223
https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/library/std/src/sys/windows/time.rs#L44

And there is a margin of error for cross-thread time comparisons:
https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/time.rs#L182

Zebra Version

Commit fdfa3cb

Linux 2021-07-13

Zebra Logs

Message:  Test failed: all candidates should obey the minimum rate-limit: now: Instant { tv_sec: 1122, tv_nsec: 23374628 } min: Instant { tv_sec: 1122, tv_nsec: 23772571 }; minimal failing input: peers = [NewAlternate { addr: 12.177.220.155:15904, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 208.121.138.118:16436, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 127.0.0.1:51426, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 213.182.135.51:10430, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 47.3.10.18:14565, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 127.0.0.1:11087, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 5.229.188.78:56800, untrusted_services: NODE_NETWORK }, NewAlternate { addr: 248.72.91.148:10038, untrusted_services: NODE_NETWORK }], initial_candidates = 3, extra_candidates = 3
	successes: 7
	local rejects: 5
		5 times at failed MetaAddr::is_valid_for_outbound
	global rejects: 0
...
 failures:
    peer_set::candidate_set::tests::prop::new_outbound_peer_connections_are_rate_limited

https://github.com/ZcashFoundation/zebra/pull/2487/checks?check_run_id=3052141305#step:10:968

Environment

Ubuntu
20.04.2
LTS

Rust assumes that most Linux distributions are monotonic.

Zebra Version

Commit 8dddda6 in PR #2487

@teor2345 teor2345 added C-bug Category: This is a bug S-needs-triage Status: A bug report needs triage P-Low I-integration-fail Continuous integration fails, including build and test failures labels Jul 6, 2021
@teor2345
Copy link
Contributor Author

We haven't seen these for a while, so some code changes or dependency update probably fixed them.

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

No branches or pull requests

2 participants