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

Commit timeout #2057

Merged
merged 4 commits into from
Aug 4, 2022
Merged

Commit timeout #2057

merged 4 commits into from
Aug 4, 2022

Conversation

faddat
Copy link
Member

@faddat faddat commented Jul 14, 2022

What is the purpose of the change

Reduce osmosis block times to about three seconds.

  • feat: fast tm-db #2009 compliments this and has been used by notional and others since evmos upgraded. Those using it and not are pretty strikingly visible here:

https://ping.pub/evmos/uptime

The trouble with the tm db branch is it is enormous and it is now very different from the mainline. I don't know if it will ever become the mainline. Here is the branch:

tendermint/tm-db#237

This is still very much draft work, and doesn't help with wall clock time at all yet.

Brief Changelog

  • make init.go set the commit timeout to two seconds
  • fix the commit timeout in root.go to two seconds

Please note that these are the only changes that have been made. I am treating additional changes that may be needed because of shorter block times is out of scope for this PR. I also think that it's possible that there are not additional changes needed.

Testing and Verifying

The best test of this code will be to run it on the osmosis test net. It has not yet been tested like that.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? not applicable

@faddat faddat requested a review from a team July 14, 2022 04:32
@faddat faddat marked this pull request as draft July 14, 2022 04:32
@faddat faddat marked this pull request as ready for review July 15, 2022 07:25
@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Jul 15, 2022
@faddat faddat changed the title precommit timeout Commit timeout Jul 21, 2022
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

This LGTM! Have you been trying it out on mainnet, whats the precommit inclusion like on blocks you produce?

@ValarDragon ValarDragon merged commit 701b391 into main Aug 4, 2022
@ValarDragon ValarDragon deleted the faddat/precommit-timeout branch August 4, 2022 14:29
@czarcas7ic czarcas7ic mentioned this pull request Aug 19, 2022
@faddat
Copy link
Member Author

faddat commented Sep 5, 2022

Actually yes, and in fact it turned out to work so well that we didn't notice the difference

czarcas7ic added a commit that referenced this pull request Sep 15, 2022
p0mvn pushed a commit that referenced this pull request Sep 17, 2022
* Revert "Commit timeout (#2057)"

This reverts commit 701b391.

* remove changelog entry

* keep timeout in init
mergify bot pushed a commit that referenced this pull request Sep 17, 2022
* Revert "Commit timeout (#2057)"

This reverts commit 701b391.

* remove changelog entry

* keep timeout in init

(cherry picked from commit 0df6256)
ValarDragon pushed a commit that referenced this pull request Sep 17, 2022
* Revert "Commit timeout (#2057)"

This reverts commit 701b391.

* remove changelog entry

* keep timeout in init

(cherry picked from commit 0df6256)

Co-authored-by: Adam Tucker <adam@osmosis.team>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:docs Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants