fix(tests): fix state sync test errors #12150
Conversation
These tests create node home dirs in tempfiles, but these are dropped too soon, and the directories are removed while the nodes are still running. This is visible in the logs as snapshot creation failure messages. After near#12147, these no longer cause test failures, but the underlying failures to create snapshots are still there. Fix it by keeping them around in an Arc in the enclosing scopes Note that every now and then these tests still fail with many messages showing `Received an invalid block during state sync` followed by a test timeout, but it seems to be for an unrelated reason.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12150 +/- ##
=======================================
Coverage 71.60% 71.61%
=======================================
Files 824 824
Lines 165507 165507
Branches 165507 165507
=======================================
+ Hits 118511 118527 +16
+ Misses 41873 41858 -15
+ Partials 5123 5122 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
hmm I know it's prob frowned upon but maybe we should disable this clippy redundant_clone failure on these tests. We need to clone the Arc so that we still have one around after one of them gets moved to I guess I could also just add redundant @nagisa have you ever seen this kind of clippy problem? Or am I doing something silly? |
|
For the most part it is also a great way to tell Rust that you want to move the guard/resource into a future, thus tying the lifetime of the two together, when there are no other "proper" uses of the variable. Last option for tempfile specifically is to use |
There was a problem hiding this comment.
Thanks for looking into this.
The Received an invalid block during state sync messages are normal while a node is performing state sync; they occur because the node is too far behind to validate any blocks at the head of the chain. I think we should consider something along the lines of silently ignoring blocks past a certain height while the node is in state sync.
| @@ -45,8 +45,13 @@ fn sync_state_nodes() { | |||
| let mut near1 = load_test_config("test1", port1, genesis.clone()); | |||
| near1.network_config.peer_store.boot_nodes = convert_boot_nodes(vec![]); | |||
| near1.client_config.min_num_peers = 0; | |||
|
|
|||
| let _dir1 = Arc::new(tempfile::Builder::new().prefix("sync_nodes_1").tempdir().unwrap()); | |||
There was a problem hiding this comment.
It would be helpful to leave a comment here explaining the observed issues with directories being cleaned up too soon.
These tests create node home dirs in tempfiles, but these are dropped too soon, and the directories are removed while the nodes are still running. This is visible in the logs as snapshot creation failure messages. After #12147, these no longer cause test failures, but the underlying failures to create snapshots are still there.
Fix it by keeping them around in an Arc in the enclosing scopes
Note that every now and then these tests still fail with many messages showing
Received an invalid block during state syncfollowed by a test timeout, but it seems to be for an unrelated reason.