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

Refactor and rename orderedNodeStream and receiveQueue #140

Merged
merged 7 commits into from
May 10, 2021

Conversation

johningve
Copy link
Member

This PR cleans up and renames some of the types that implement our "node streams". Before, when we included most of the gorums code in the generated code, it was important that the names of our methods and types did not conflict with other generated code from protobuf and grpc. Now, most of that code has been moved into the gorums package, which is imported by the generated code.

Some of the name changes:

  • orderedNodeStream => channel
  • gorumsStreamRequest => request
  • gorumsStreamResult => response

Also, I have removed the receiveQueue struct. The manager now provides a getMsgID() function instead, and the "routing" of response messages is handled by the channels. This seems to have improved performance for async quorum calls, but I'm not sure why. Possibly due to reducing lock contention?

image

They can have simpler names since they are no longer part of the generated code
.
This also includes some changes that will make supporting multiple
channels easier in the future.
Instead, channels will provide an enqueue function where calltypes can
pass a channel to receive responses on.
@johningve johningve requested a review from meling May 6, 2021 14:16
The race detector complained about the streamBroken boolean, so this
commit makes it atomic.
channel.go Outdated Show resolved Hide resolved
@meling meling merged commit a5ad159 into relab:master May 10, 2021
@johningve johningve deleted the channels-refactor branch May 28, 2021 15:25
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