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

Time between heartbeats at bootstrap #4897

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

sstanculeanu
Copy link
Contributor

@sstanculeanu sstanculeanu commented Jan 23, 2023

Reasoning behind the pull request

  • during bootstrap, heartbeats will be used to keep track of trie sync progress. Currently, the time between sends is the same as regular heartbeats, 5 min, which may be too much for a proper progress bar

Proposed changes

  • added new config value

Testing procedure

  • start a node on testnet. Either check logs for heartbeat message sent to be ~1 min, either check https://testnet-gateway.multiversx.com/node/heartbeatstatus and make sure the numTrieNodesReceived for your node increases each ~minute during trie sync
  • after bootstrap, the same log should be after ~5min, and numTrieNodesReceived 0

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@@ -917,6 +917,7 @@
PeerAuthenticationTimeBetweenSendsWhenErrorInSec = 60 # 1min
PeerAuthenticationThresholdBetweenSends = 0.1 # 10%
HeartbeatTimeBetweenSendsInSec = 60 # 1min
HeartbeatTimeBetweenSendsDuringBootstrapInSec = 60 # 1min
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to have this set to 1 minute on the public testnets & mainnet? The original code used HeartbeatTimeBetweenSendsInSec which was set to 5 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, this was the reason behind the PR. While testing on public testnet, I realized that 5 min with no feedback would be too much

@gabi-vuls gabi-vuls merged commit d0d05e7 into rc/v1.4.0 Jan 26, 2023
@gabi-vuls gabi-vuls deleted the time_between_heartbeats_bootstrap branch January 26, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants