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

Revert "consensus: restructure peer catchup sleep" #783

Closed
wants to merge 1 commit into from

Conversation

evan-forbes
Copy link
Member

@evan-forbes evan-forbes commented Jun 7, 2022

This reverts commit a889f17.

When proposing large blocks, this refactor to timeouts was contributing to nodes prematurely starting a new round before the block could be gossiped.

@adlerjohn
Copy link
Member

adlerjohn commented Jun 8, 2022

Why are we reverting tendermint/tendermint#8651? Isn't that a fix? The rationale for this PR isn't documented. Is 8651 being reverting upstream as well?

@evan-forbes
Copy link
Member Author

Why are we reverting tendermint/tendermint#8651? Isn't that a fix? The rationale for this PR isn't documented. Is 8651 being reverting upstream as well?

As mentioned briefly in the description and in celestiaorg/celestia-app#475, using v0.35.6 with large blocks results in consensus taking many rounds, as block producers will prematurely start a new round as block gossiping is too slow. This PR fixes this by reverting the change, however this PR was created before talking with the tendermint team to find a better solution.

it became clear that the default settings also need to be changed in order for the fix to work, specifically peer-gossip-sleep-duration needs to be set a few ms, not the default 100ms. Instead of this PR, we should change the default settings.

@evan-forbes evan-forbes closed this Jun 8, 2022
@evan-forbes evan-forbes deleted the evan/revert-timeout-refactor branch September 2, 2022 04:39
cmwaters pushed a commit that referenced this pull request Sep 20, 2023
* Unsafe int cast in `kill` command (#783)

* Unsafe int cast in `kill` command

* Revert "Unsafe int cast in `kill` command"

This reverts commit bbd649bd372ca90f83dea7b424d67dafbd9eb541.

* Changed strategy

(cherry picked from commit 03c5e7727a03983b54623e731d5d3d8dd4ac75ec)

# Conflicts:
#	cmd/cometbft/commands/debug/kill.go

* Revert "Unsafe int cast in `kill` command (#783)"

This reverts commit b7ab279a6df1f062bec60bcf95947d2a87f4ccec.

* Unsafe int cast in `kill` command (#783)

* Unsafe int cast in `kill` command

* Revert "Unsafe int cast in `kill` command"

This reverts commit bbd649bd372ca90f83dea7b424d67dafbd9eb541.

* Changed strategy

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants