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

Add support for multiple streams (channels) per Node #131

Closed
wants to merge 7 commits into from

Conversation

johningve
Copy link
Member

This PR attempts to make it possible to have multiple Channels (formerly orderedNodeStream) per Node. Currently, the channel for a call can be changed from the default by passing either a *gorums.Channel or a []*gorums.Channel as a ConfigOption.

johningve added a commit to relab/hotstuff that referenced this pull request Mar 25, 2021
This makes use of the work-in-progress channels in Gorums
(see relab/gorums#131).
We can use a dedicated channel for all messages that are core to the
protocol, such that they are all handled synchronously, and then another
channel for other messages, such as fetch requests/responses.
This allows us to do RPC messages on the event loop goroutine.
We can thus make the fetch operation part of the BlockChain.Get()
method, which simplifies things a lot.

However, logs show that allowing each call to Get() to attempt to fetch
blocks from other replicas will generate a lot of unnecessary fetch
attempts. To combat this, we add a LocalGet() method which should be
used instead of Get() when it is undesirable to fetch a missing block.

Special attention is needed in OnVote(), because it can very often fail
to find its blocks. Thus, we add the AwaitEvent() method to the event
loop. This method allows us to reschedule a vote event to run after
the next proposal event occurs. This allows us to avoid around 90% of
OnVote-initiated fetch attempts, according to my measurements.
OnVote will attempt to reschedule the event the first time it fails to
find its block. The second time, it will try to fetch the block.
This adds a Close() function to channels, and makes Node.NewChannel()
call Channel.Connect()
This allows us to differentiate between a canceled call and a call
that received errors.
@meling
Copy link
Member

meling commented Apr 13, 2021

@Raytar What do you want to do with this PR? There are some nice things to keep in this, even if we don't need the channels. It would be nice to bring these things into master before giving another go at #109.

@johningve
Copy link
Member Author

Closing in favor of #140. Might revisit the multiple channels idea in the future.

@johningve johningve closed this May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants