Skip to content

Commit

Permalink
Revert "feat!: account for time already elapsed when waiting after th…
Browse files Browse the repository at this point in the history
…e commit (#965)" (#1033)

This reverts commit cc1bc3f.
  • Loading branch information
evan-forbes authored and cmwaters committed Jul 24, 2023
1 parent 166875e commit 3042aa0
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 43 deletions.
2 changes: 1 addition & 1 deletion DOCKER/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ if [ ! -d "$CMTHOME/config" ]; then
-e "s/^proxy_app\s*=.*/proxy_app = \"$PROXY_APP\"/" \
-e "s/^moniker\s*=.*/moniker = \"$MONIKER\"/" \
-e 's/^addr_book_strict\s*=.*/addr_book_strict = false/' \
-e 's/^target_height_duration\s*=.*/target_height_duration = "1000ms"/' \
-e 's/^timeout_commit\s*=.*/timeout_commit = "500ms"/' \
-e 's/^index_all_tags\s*=.*/index_all_tags = true/' \
-e 's,^laddr = "tcp://127.0.0.1:26657",laddr = "tcp://0.0.0.0:26657",' \
-e 's/^prometheus\s*=.*/prometheus = true/' \
Expand Down
33 changes: 14 additions & 19 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"os"
"path/filepath"
"time"

tmtime "github.com/tendermint/tendermint/types/time"
)

const (
Expand Down Expand Up @@ -946,10 +944,11 @@ type ConsensusConfig struct {
TimeoutPrecommit time.Duration `mapstructure:"timeout_precommit"`
// How much the timeout_precommit increases with each round
TimeoutPrecommitDelta time.Duration `mapstructure:"timeout_precommit_delta"`
// TargetHeigtDuration is used to determine how long we wait after a
// block is committed. If this time is shorter than the actual time to reach
// consensus for that height, then we do not wait at all.
TargetHeightDuration time.Duration `mapstructure:"target_height_duration"`
// How long we wait after committing a block, before starting on the new
// height (this gives us a chance to receive some more precommits, even
// though we already have +2/3).
// NOTE: when modifying, make sure to update time_iota_ms genesis parameter
TimeoutCommit time.Duration `mapstructure:"timeout_commit"`

// Make progress as soon as we have all the precommits (as if TimeoutCommit = 0)
SkipTimeoutCommit bool `mapstructure:"skip_timeout_commit"`
Expand All @@ -969,13 +968,13 @@ type ConsensusConfig struct {
func DefaultConsensusConfig() *ConsensusConfig {
return &ConsensusConfig{
WalPath: filepath.Join(defaultDataDir, "cs.wal", "wal"),
TimeoutPropose: 2000 * time.Millisecond,
TimeoutPropose: 3000 * time.Millisecond,
TimeoutProposeDelta: 500 * time.Millisecond,
TimeoutPrevote: 1000 * time.Millisecond,
TimeoutPrevoteDelta: 500 * time.Millisecond,
TimeoutPrecommit: 1000 * time.Millisecond,
TimeoutPrecommitDelta: 500 * time.Millisecond,
TargetHeightDuration: 3000 * time.Millisecond,
TimeoutCommit: 1000 * time.Millisecond,
SkipTimeoutCommit: false,
CreateEmptyBlocks: true,
CreateEmptyBlocksInterval: 0 * time.Second,
Expand All @@ -995,7 +994,7 @@ func TestConsensusConfig() *ConsensusConfig {
cfg.TimeoutPrecommit = 10 * time.Millisecond
cfg.TimeoutPrecommitDelta = 1 * time.Millisecond
// NOTE: when modifying, make sure to update time_iota_ms (testGenesisFmt) in toml.go
cfg.TargetHeightDuration = 70 * time.Millisecond
cfg.TimeoutCommit = 10 * time.Millisecond
cfg.SkipTimeoutCommit = true
cfg.PeerGossipSleepDuration = 5 * time.Millisecond
cfg.PeerQueryMaj23SleepDuration = 250 * time.Millisecond
Expand Down Expand Up @@ -1029,14 +1028,10 @@ func (cfg *ConsensusConfig) Precommit(round int32) time.Duration {
) * time.Nanosecond
}

// NextStartTime adds the TargetHeightDuration to the provided starting time.
func (cfg *ConsensusConfig) NextStartTime(t time.Time) time.Time {
newStartTime := t.Add(cfg.TargetHeightDuration)
now := tmtime.Now()
if newStartTime.Before(now) {
return now
}
return newStartTime
// Commit returns the amount of time to wait for straggler votes after receiving +2/3 precommits
// for a single block (ie. a commit).
func (cfg *ConsensusConfig) Commit(t time.Time) time.Time {
return t.Add(cfg.TimeoutCommit)
}

// WalFile returns the full path to the write-ahead log file
Expand Down Expand Up @@ -1073,8 +1068,8 @@ func (cfg *ConsensusConfig) ValidateBasic() error {
if cfg.TimeoutPrecommitDelta < 0 {
return errors.New("timeout_precommit_delta can't be negative")
}
if cfg.TargetHeightDuration < 0 {
return errors.New("target_height_duration can't be negative")
if cfg.TimeoutCommit < 0 {
return errors.New("timeout_commit can't be negative")
}
if cfg.CreateEmptyBlocksInterval < 0 {
return errors.New("create_empty_blocks_interval can't be negative")
Expand Down
4 changes: 2 additions & 2 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ func TestConsensusConfig_ValidateBasic(t *testing.T) {
"TimeoutPrecommit negative": {func(c *ConsensusConfig) { c.TimeoutPrecommit = -1 }, true},
"TimeoutPrecommitDelta": {func(c *ConsensusConfig) { c.TimeoutPrecommitDelta = time.Second }, false},
"TimeoutPrecommitDelta negative": {func(c *ConsensusConfig) { c.TimeoutPrecommitDelta = -1 }, true},
"TargetHeightDuration": {func(c *ConsensusConfig) { c.TargetHeightDuration = time.Second }, false},
"TargetHeightDuration negative": {func(c *ConsensusConfig) { c.TargetHeightDuration = -1 }, true},
"TimeoutCommit": {func(c *ConsensusConfig) { c.TimeoutCommit = time.Second }, false},
"TimeoutCommit negative": {func(c *ConsensusConfig) { c.TimeoutCommit = -1 }, true},
"PeerGossipSleepDuration": {func(c *ConsensusConfig) { c.PeerGossipSleepDuration = time.Second }, false},
"PeerGossipSleepDuration negative": {func(c *ConsensusConfig) { c.PeerGossipSleepDuration = -1 }, true},
"PeerQueryMaj23SleepDuration": {func(c *ConsensusConfig) { c.PeerQueryMaj23SleepDuration = time.Second }, false},
Expand Down
8 changes: 4 additions & 4 deletions config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,10 +464,10 @@ timeout_prevote_delta = "{{ .Consensus.TimeoutPrevoteDelta }}"
timeout_precommit = "{{ .Consensus.TimeoutPrecommit }}"
# How much the timeout_precommit increases with each round
timeout_precommit_delta = "{{ .Consensus.TimeoutPrecommitDelta }}"
# TargetHeigtDuration is used to determine how long we wait after a
# block is committed. If this time is shorter than the actual time to reach
# consensus for that height, then we do not wait at all.
target_height_duration = "{{ .Consensus.TargetHeightDuration }}"
# How long we wait after committing a block, before starting on the new
# height (this gives us a chance to receive some more precommits, even
# though we already have +2/3).
timeout_commit = "{{ .Consensus.TimeoutCommit }}"
# How many blocks to look back to check existence of the node's consensus votes before joining consensus
# When non-zero, the node will panic upon restart
Expand Down
11 changes: 7 additions & 4 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,11 +664,14 @@ func (cs *State) updateToState(state sm.State) {
cs.updateRoundStep(0, cstypes.RoundStepNewHeight)

if cs.CommitTime.IsZero() {
// If it is the first block, start time is equal to
// states last block time which is the genesis time.
cs.StartTime = state.LastBlockTime
// "Now" makes it easier to sync up dev nodes.
// We add timeoutCommit to allow transactions
// to be gathered for the first block.
// And alternative solution that relies on clocks:
// cs.StartTime = state.LastBlockTime.Add(timeoutCommit)
cs.StartTime = cs.config.Commit(cmttime.Now())
} else {
cs.StartTime = cs.config.NextStartTime(cs.StartTime)
cs.StartTime = cs.config.Commit(cs.CommitTime)
}

cs.Validators = validators
Expand Down
22 changes: 12 additions & 10 deletions docs/core/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,10 @@ timeout_prevote_delta = "500ms"
timeout_precommit = "1s"
# How much the timeout_precommit increases with each round
timeout_precommit_delta = "500ms"
# TargetHeigtDuration is used to determine how long we wait after a
# block is committed. If this time is shorter than the actual time to reach
# consensus for that height, then we do not wait at all.
target_height_duration = "15s"
# How long we wait after committing a block, before starting on the new
# height (this gives us a chance to receive some more precommits, even
# though we already have +2/3).
timeout_commit = "1s"

# How many blocks to look back to check existence of the node's consensus votes before joining consensus
# When non-zero, the node will panic upon restart
Expand Down Expand Up @@ -537,9 +537,12 @@ timeout_prevote = "1s"
timeout_prevote_delta = "500ms"
timeout_precommit = "1s"
timeout_precommit_delta = "500ms"
target_height_duration = "1s"
timeout_commit = "1s"
```

Note that in a successful round, the only timeout that we absolutely wait no
matter what is `timeout_commit`.

Here's a brief summary of the timeouts:

- `timeout_propose` = how long we wait for a proposal block before prevoting nil
Expand All @@ -549,8 +552,7 @@ Here's a brief summary of the timeouts:
- `timeout_prevote_delta` = how much the `timeout_prevote` increases with each round
- `timeout_precommit` = how long we wait after receiving +2/3 precommits for
anything (ie. not a single block or nil)
- `timeout_precommit_delta` = how much the timeout_precommit increases with
each round
- `target_height_duration` = used to determine how long we wait after a
block is committed. If this time is shorter than the actual time to reach
consensus for that height, then we do not wait at all.
- `timeout_precommit_delta` = how much the `timeout_precommit` increases with each round
- `timeout_commit` = how long we wait after committing a block, before starting
on the new height (this gives us a chance to receive some more precommits,
even though we already have +2/3)
2 changes: 1 addition & 1 deletion state/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ func execBlockOnProxyApp(
return nil, err
}

logger.Info("executed block", "height", block.Height, "num_valid_txs", validTxs, "num_invalid_txs", invalidTxs, "time", block.Time)
logger.Info("executed block", "height", block.Height, "num_valid_txs", validTxs, "num_invalid_txs", invalidTxs)
return abciResponses, nil
}

Expand Down
4 changes: 2 additions & 2 deletions test/maverick/consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -900,9 +900,9 @@ func (cs *State) updateToState(state sm.State) {
// to be gathered for the first block.
// And alternative solution that relies on clocks:
// cs.StartTime = state.LastBlockTime.Add(timeoutCommit)
cs.StartTime = state.LastBlockTime
cs.StartTime = cs.config.Commit(cmttime.Now())
} else {
cs.StartTime = cs.config.NextStartTime(cs.StartTime)
cs.StartTime = cs.config.Commit(cs.CommitTime)
}

cs.Validators = validators
Expand Down

0 comments on commit 3042aa0

Please sign in to comment.