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

Reduce likelyhood of race in node.fork_publish #4804

Merged

Conversation

gr0vity-dev
Copy link
Contributor

This testcase fails a lot on windows github runners and in ~3% locally.

  1. Reorder block processing in direct succession to increase likelihood of fork handling before election confirmation. (With this change alone the testcase fails in ~0.2%)

  2. Reduce default poll interval from 50ms to 10ms to reduce internal node operations between 2 polls. (With this change alone the testcase fails in ~0.2%)

Both changes are needed - Reordering alone or poll interval changes alone were insufficient. With both changes applied, test passes reliably (10000/10000).

gr0vity added 2 commits December 5, 2024 21:23
Some testcases (e.g. node.fork_publish) are racy because too many things can happen between 2 polls. While this doesn't fix the racy testcase, it greatly reduces the likelyhood that testcases are failing due to timing issues
@qwahzi qwahzi added this to the V28 milestone Dec 5, 2024
@qwahzi qwahzi added the unit test Related to a new, changed or fixed unit test label Dec 5, 2024
@gr0vity-dev-bot
Copy link

gr0vity-dev-bot commented Dec 5, 2024

Test Results for Commit 1e08592

Pull Request 4804: Results
Overall Status:

Test Case Results

  • 5n4pr_conf_10k_bintree: PASS (Duration: 112s)
  • 5n4pr_conf_10k_change: PASS (Duration: 192s)
  • 5n4pr_conf_change_dependant: PASS (Duration: 141s)
  • 5n4pr_conf_change_independant: PASS (Duration: 136s)
  • 5n4pr_conf_send_dependant: PASS (Duration: 128s)
  • 5n4pr_conf_send_independant: PASS (Duration: 125s)
  • 5n4pr_rocks_10k_bintree: PASS (Duration: 110s)
  • 5n4pr_rocks_10k_change: PASS (Duration: 193s)

Last updated: 2024-12-05 23:29:49 UTC

@pwojcikdev pwojcikdev merged commit a865db5 into nanocurrency:develop Dec 6, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unit test Related to a new, changed or fixed unit test
Projects
Status: Merged / V28.0
Development

Successfully merging this pull request may close these issues.

4 participants