Skip to content

Consensus refactor#167

Merged
jaekwon merged 12 commits intomasterfrom
consensus_refactor
Dec 17, 2015
Merged

Consensus refactor#167
jaekwon merged 12 commits intomasterfrom
consensus_refactor

Conversation

@jaekwon
Copy link
Contributor

@jaekwon jaekwon commented Dec 12, 2015

Creating pull for comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of select { internal <-; <-timeout }, how about select { internal <-; default CList.PushBack(msg) } ? We can shuffle more from CList to internalMsgQueue on the bottom of receiveMessage routine's for-loop.

We don't want to wait 10 milliseconds on things, and launching a new goroutine would make the order of sends to internalMsgQueue nondeterministic.

Copy link
Contributor

Choose a reason for hiding this comment

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

thats interesting. my thinking was two fold:

  1. we should almost never fail to send on internal, because it has a large buffer and the receiveRoutine select should be balanced
  2. we don't get determinism anyways until after something is read in the receiveRoutine

agree 10 ms might be too much in the event we are backlogged. so we could just use "default: go internal<-"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be confusing to see internalMsgQueue items out of order, e.g. a precommit before a prevote. At least we can make the order of messages in internalMsgQueue deterministic.
But I think that's fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/1/RoundStepNewHeight/g

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe SetEventSwitch where possible? Probably everywhere now since we're not using the cache, yeah?

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