chore: slightly increase blocktime #9021
Conversation
PaddyMc
added
V:state/compatible/backport
WalkthroughThis pull request introduces a new entry in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/osmosisd/cmd/root.go (1)
141-162:❓ Verification inconclusive
Verify the impact on network performance
The adjustment to consensus timeouts will slightly increase block times as intended, potentially improving network stability by allowing more time for consensus operations.
These timeout adjustments should be monitored after deployment to ensure they achieve the desired balance between block time and network stability. Consider measuring:
- Actual block times before and after the change
- Rate of consensus timeouts
- Network participation metrics
🏁 Script executed:
#!/bin/bash # Check for references to consensus parameters in the codebase echo "Searching for consensus parameter references..." rg -A 2 -B 2 "timeout_commit|timeout_propose" --type goLength of output: 816
Monitor Consensus Timeout Impact & Performance Metrics
The changes in consensus timeout values (timeout_commit at 550ms and timeout_propose at 1.9s) in
cmd/osmosisd/cmd/root.goare correctly applied as confirmed by the reference search. These adjustments are expected to slightly increase block times, potentially enhancing network stability by allowing more time for consensus operations. Please ensure you monitor the following after deployment:
- Actual block times before and after the change
- Rates of consensus timeouts
- Network participation metrics
This will help confirm that the new timeout settings provide the desired balance between block time and stability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)cmd/osmosisd/cmd/root.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: go (03)
- GitHub Check: go (02)
- GitHub Check: go (01)
- GitHub Check: go (00)
- GitHub Check: e2e
- GitHub Check: Summary
🔇 Additional comments (3)
cmd/osmosisd/cmd/root.go (2)
149-151: Timeout commit increased by 10%This change increases the
timeout_commitvalue from 500ms to 550ms, which is a 10% increase. This aligns with the PR objective of slightly increasing block time.
154-156: Timeout propose increased by 5.6%This change increases the
timeout_proposevalue from 1.8s to 1.9s, which is approximately a 5.6% increase. This complements the timeout_commit increase to achieve the desired block time adjustment.CHANGELOG.md (1)
56-56: Ensure Changelog Entry Clarity and ConsistencyThe newly added line
* [#9021](https://github.com/osmosis-labs/osmosis/pull/9021) chore: slightly increase blocktime
is well-formatted and follows the established pattern for changelog entries. It correctly includes the issue/PR reference and a concise description. Please double-check that this documentation accurately reflects the corresponding consensus configuration update (i.e., the timeout changes) and that its placement under the "State Compatible" section is appropriate for a minor blocktime adjustment.
cmd/osmosisd/cmd/root.go
Outdated
| }, | ||
| { | ||
| Section: "consensus", | ||
| Key: "timeout_propose", | ||
| Value: "1.8s", | ||
| Value: "1.9s", |
There was a problem hiding this comment.
Lets keep timeout propose where it is, otw LGTM!
There was a problem hiding this comment.
For context, this is is saying "if the proposer is offline, how long do we wait for round 2".
Our blocktime is 1.3s (maybe 1.5s), 1.8s is already way longer! Seeing that its 1.8s, I'd actually love to lower it. If your in favor, can we do 1.6s in this PR? My mental model is 1s would be a good conservative parameterization for where are rn. However, don't want need to bundle in now.
This timeout propose actually can/should be lower than average blocktime.
There was a problem hiding this comment.
updated to 1.6s, thanks for your input 🙇
* chore: slightly increase blocktime * chore: change CHANGELOG.md * chore: reduce timeout commit to 1.6s (cherry picked from commit 3830890)
* chore: slightly increase blocktime * chore: change CHANGELOG.md * chore: reduce timeout commit to 1.6s (cherry picked from commit 3830890)
* chore: slightly increase blocktime * chore: change CHANGELOG.md * chore: reduce timeout commit to 1.6s (cherry picked from commit 3830890) Co-authored-by: PaddyMc <[email protected]>
* chore: slightly increase blocktime * chore: change CHANGELOG.md * chore: reduce timeout commit to 1.6s (cherry picked from commit 3830890) Co-authored-by: PaddyMc <[email protected]>
What is the purpose of the change
Slightly increase blocktime