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

Clean up Gorums method options #56

Closed
5 tasks done
meling opened this issue Apr 14, 2020 · 6 comments
Closed
5 tasks done

Clean up Gorums method options #56

meling opened this issue Apr 14, 2020 · 6 comments
Assignees

Comments

@meling
Copy link
Member

meling commented Apr 14, 2020

Currently, we have several options, some of which may be removed and others should be reorganized and renamed to make things easier to maintain.

  • We should rename the gorums.qc option to gorums.quorumcall. It is longer, but much clearer.
  • gorums.qf_with_req is often used, so perhaps we should once again reconsider whether or not we should require users to specify this; just make it the default.
  • gorums.correctable_stream can be replaced by gorums.correctable and the stream annotation at the output type (we can test for both of these in the code, so it shouldn't be difficult to implement). If necessary, we can follow a similar structure as the new gorums.ordered option.
  • Consider whether or not gorums.qc_future can be replaced with gorums.correctable.
  • If we want to keep the future option, replace gorums.qc_future with two options: gorums.quorumcall and gorums.future. This can follow a similar structure as the new gorums.ordered option.

Before making hard decisions regarding the removal of certain options, e.g. future, we should implement test examples to use them to get more experience with it...

Opinions?

@meling
Copy link
Member Author

meling commented May 4, 2020

Some further thoughts. I've been thinking about the name gorums.qc_future. I think we should rename it to gorums.async, since that has become the de facto naming for such call semantics, even though it isn't exactly the async/await semantics of some languages. Thoughts? @tormoder @leandernikolaus @Raytar

@meling meling self-assigned this May 4, 2020
@johningve
Copy link
Member

Some further thoughts. I've been thinking about the name gorums.qc_future. I think we should rename it to gorums.async, since that has become the de facto naming for such call semantics, even though it isn't exactly the async/await semantics of some languages. Thoughts? @tormoder @leandernikolaus @Raytar

Is the idea still to make it work like the ordered option, such that you specify gorums.quorumcall and gorums.async? In that case, does it make sense to use it with any other call types? Can a correctable be async?

@meling
Copy link
Member Author

meling commented May 5, 2020

This is the question. I'm considering whether or not it makes sense to use async as a secondary option to quorumcall. Not sure it makes sense to use it along with the other call types, such as correctable, since correctable variants follow a similar design as that of async. To that end, maybe the correctable variants are really sub types of async. But there are essentially three variants:

  • async (currently qc_future)
  • correctable
  • correctable_stream (currently this only requires correctable along with the rpc method being server streaming.)

@tormoder
Copy link
Contributor

tormoder commented May 5, 2020

(It's been a long time and I don't have the bandwidth to look at the details).

future/async for quorumcall was, as I remember it, just a "programming feature" to make the client able to execute quorum call asynchronously in code. It was really more of a convenience feature, and clients could write the wrapper themselves. I.e. it is not a "property" of the underlying call.

A correctable is inherently async, since a client may get an initial result, but the result may be updated several times asynchronously and concurrently with the client using the initial reply.

@meling
Copy link
Member Author

meling commented May 17, 2020

Below is one idea for the option combinations that we can support. It feels a bit unnatural to require async together with correctable.

What do you think?
Anyone have better ideas?? @tormoder @leandernikolaus @Raytar

  // New ordered synchronous RPC call (not a quorum call, but one-to-one call)
  rpc OrderedRPC(Request) returns (Response) {
    option (gorums.ordered) = true;
  }

  // Regular synchronous quorum call
  rpc QuorumCall(Request) returns (Response) {
    option (gorums.quorumcall) = true;
  }

  // This is the Qc_Future
  rpc AsyncQuorumCall(Request) returns (Response) {
    option (gorums.quorumcall) = true;
    option (gorums.async) = true;
  }

  // Ordered quorum call (new)
  rpc OrderedQuorumCall(Request) returns (Response) {
    option (gorums.quorumcall) = true;
    option (gorums.ordered) = true;
  }

  // This is the new Qc_Future with ordering (not yet merged)
  rpc OrderedAsyncQuorumCall(Request) returns (Response) {
    option (gorums.quorumcall) = true;
    option (gorums.async) = true;
    option (gorums.ordered) = true;
  }

  rpc Correctable(Request) returns (Response) {
    option (gorums.correctable) = true;
  }

  rpc CorrectableStream(Request) returns (stream Response) {
    option (gorums.correctable) = true;
  }

@meling
Copy link
Member Author

meling commented May 29, 2020

Closing since this is now implemented in the reorg-generator branch.

@meling meling closed this as completed May 29, 2020
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

No branches or pull requests

3 participants