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

feat: use a dynamic timeout commit modelled on the size of the block #1342

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,8 +1042,21 @@

// 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)
func (cfg *ConsensusConfig) Commit(t time.Time, blockSizeBytes int64) time.Time {
// adjust the timeout based on how large the block size is for a total of
// 8MB which would decrease the timeout commit by roughly 4 seconds
perByteDelay := time.Duration(time.Microsecond / 2)

Check failure on line 1048 in config/config.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unnecessary conversion (unconvert)
maxBlockSize := int64(8 * 1024 * 1024)
if blockSizeBytes > maxBlockSize {
blockSizeBytes = maxBlockSize
}

timeoutCommit := cfg.TimeoutCommit - (time.Duration(blockSizeBytes) * perByteDelay)
Copy link
Member

Choose a reason for hiding this comment

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

what would be the best of determining this value? I guess adjusting on mocha?

would you describe the upside being simplicity and the downside being that changes to the network require re-calibrating this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this function should be determined empirically by running 100 node networks with varying block sizes from 0MB to 8MB and modelling that into a function.

would you describe the upside being simplicity and the downside being that changes to the network require re-calibrating this value?

Yes it's a good simple temporary solution. If we go down the road of either compact blocks or proposer based timestamps. Those solutions can replace this current one

// make sure timeout is not negative
if timeoutCommit < 0 {
timeoutCommit = time.Millisecond
}
return t.Add(timeoutCommit)
}

// WalFile returns the full path to the write-ahead log file
Expand Down
4 changes: 2 additions & 2 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,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 = cs.config.Commit(cmttime.Now())
cs.StartTime = cs.config.Commit(cmttime.Now(), 0)
} else {
cs.StartTime = cs.config.Commit(cs.CommitTime)
cs.StartTime = cs.config.Commit(cs.CommitTime, cs.ProposalBlockParts.ByteSize())
}

cs.Validators = validators
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 = cs.config.Commit(cmttime.Now())
cs.StartTime = cs.config.Commit(cmttime.Now(), 0)
} else {
cs.StartTime = cs.config.Commit(cs.CommitTime)
cs.StartTime = cs.config.Commit(cs.CommitTime, cs.ProposalBlockParts.ByteSize())
}

cs.Validators = validators
Expand Down
Loading