api: support for pre-call translation to custom type

[meling]

Background: We now have support for custom return types that can be returned from the different quorum call variants to encode additional details that does need to be sent on the wire or exposed to the server. We also have support for perNode arguments that translate the request parameter to a value to be sent to the different servers. This latter feature is a pre-call translation while the return types are handled by the quorum function and so there is a form of symmetry here.

What is missing is a general pre-call translation or adapter that takes one type as input and creates another type that can be sent to the servers. See attached picture.

When is this useful? We can make use of this when we need to create signatures or operate on the input message. We already do something like this for the byzq, but without support from Gorums. We implement a method for signing in the quorum spec.

How should it be implemented? One obvious solution would be to have the user provide a string option such as pre_call_adapter to specify a custom type expected, which would then produce a function MethodNamePreCallAdapter() in the QuorumSpec interface that the user needs to implement.

There are some issues regarding the implementation that remains undecided on my end wrt. naming, but I think the principle is very clear to me.

An alternative solution is to define a per_call_adapter string in the proto file. Instead of defining the PreCallAdapter in the QuorumSpec interface, we can create a separate interface called MethodNamePreCallAdapter that Gorums can check for with a type assertion and call it if it exists. That is, if the QuorumSpec object also implements the MethodNamePreCallAdapter method, it can invoke this method to do the adaptation. Is this too much magic?

Can this somehow be integrated with the per_node_arg option, making it so that this named function MethodNamePrecallAdapter or something is called, and that can generate either a single message to be sent to all servers or a unique message for each server. This needs some more thinking.

[meling]

I hacked together a quick prototype for this yesterday in branch call-adapter. Haven't tested it with byzq and recstore yet, but that is on my agenda. However, I wanted some more input on this before I continue, as there are some design tradeoffs to be made.

The primary reason for moving ahead with this pre-call adaptation is as for quorum functions: separation of concerns and encapsulation. We would like to hide technical details related to such adaptation from the implementation of the main protocol flow.

I have identified two separate types of adaptation that is relevant. One is actually already implemented in the form of per_node_arg, although this exposes the perNode function to the main protocol flow. The two types are:

1. PerNodeAdaptation: This is similar to per_node_arg, but moves the adaptation logic to a separate method that must be implemented (see below for an example).
2. PerCallAdaptation: This will take as input one message type and and return another to be sent to the servers.

Both types can be declared in a CallAdapter interface that must be implemented; it would function much like the QuorumSpec interface. In fact, the methods below could also be defined in the QuorumSpec interface. Currently the are kept separate.

The per-node version would look like this, where the MyState is what the servers expect, but clients wants to work with State objects. It is possible to define that they both be the same type. This function would be called exactly the same way as is currently done for per_node_arg. The main difference is that the code would be hidden from the main protocol flow. I can imagine that we could add methods to the Node type that would allow a protocol to access a per-node map, that could be used to keep certain protocol state. This would be similar to the context.Context.

WriteAdapterAdapter(req *State, node *Node) *MyState

(PS: the double AdapterAdapter is because the method is called WriteAdapter; this wouldn't normally be the case.)

The implementation looks just like per_node_arg:

	expected := c.n
	replyChan := make(chan internalWriteResponse, expected)
	for _, n := range c.nodes {
		nodeArg := c.adapt.WriteAdapterAdapter(a, n)
		if nodeArg == nil {
			expected--
			continue
		}
		go callGRPCWriteAdapter(ctx, n, nodeArg, replyChan)
	}

The per-call version would look like this, and it would be called only once for each quorum call.

WriteAdapterAdapter(req *State) *MyState

The implementation would look like:

	arg := c.adapt.WriteAdapterAdapter(a)
	expected := c.n
	replyChan := make(chan internalWriteResponse, expected)
	for _, n := range c.nodes {
		go callGRPCWriteAdapter(ctx, n, arg, replyChan)
	}

I think both use cases are reasonable and matches the byzq and recstore needs. I have looked at the use of per_node_arg in the Raft implementation, and there the situation is not as clear cut, because the perNode function is accessing a lot of protocol state, such as log entries etc. However, if the Node type is extended with a map, and corresponding accessor functions, that could keep this state, perhaps it could still work out.

So my question to @s111 and @tormoder is: does it make sense to keep both per_node_arg and this per_node_adapter options, given their similarity in function?? Their usage interfaces are different obviously.

Any other thoughts on this design? Naming suggestions?

PS: The current prototype implementation only implements one of the two adaptation types.

[meling]

An alternative to adding two new gorums-level options would be to add ManagerOptions (or maybe ConfigurationOptions?) for which a user could specify which adapter function to call. It would allow us to do something like this:

	var arg *MyState
	if c.opts.perCallAdapter != nil {
		arg = c.opts.perCallAdapter(a)
	}

The user would specify these adapters using two functions like so:

1. WithPerCallAdapter(WriteCallAdapter) and
2. WithPerNodeAdapter(WriteNodeAdapter).

The only drawback is that there would be two extra nil checks, including one inside the loop, even for quorum calls that don't use these adapters. Not sure what would be the impact of this, but if it were a problem, the nil checks could actually both be done outside the loop and make two different loops, one that calls the adapter and another that doesn't.

Some issues:

1. One issue is that the adapter functions typically operate on the QuorumSpec. Will this cause any problems?? One possibility is to do c.opts.perCallAdapter(c.qspec, a)
2. Another issue is how would we distinguish between different service methods, since a configuration is often associated with more than one service method.

(this was just a quick write up... no prototyping... or careful thought...)

[meling]

I've pushed an implementation to the call-adapter branch that supports both PerCallAdapter and PerNodeAdapter, but not at the same time. It also supports the per_node_arg function approach, but it is not clear if we should support both or do something else. That is, I'm starting to think about a possible layered approach instead, which would encapsulate, or hide message fields and methods, so that both client-side and server-side application code cannot access fields that they shouldn't.

[meling]

I have reread this. Here is a link to the call-adapter branch, for the record.

I've come to a few conclusions about this. First, I think there is some merit to providing a feature to separate the logic/concern of pre-call transformations from the logic of the method call, e.g. the signing of individual messages to the group/configuration being called. The per_node_arg option does provide one approach for this, but it will typically be implemented inline with the rest of the method call logic, giving it access to configuration object state, such as nodes. The main benefit of the adapter options over the current per_node_arg option is that

However, I've come to dislike the idea of generating code, relying on the options per_call_adapter and per_node_adapter, which are strings naming the type to be adapted to (the server-side expected type). This is complicated and inflexible. Actually, I have a similar feeling about the per_node_arg option.

I like better the idea of using ManagerOptions to provide the relevant adapters. However, it is not clear how this can be done on a per method basis, unless the option itself provides a mapping between method (name) and the transformation/adapter function.

However, perhaps a better idea is to design a simple way for users to provide their own interceptors for Gorums, as outlined in #37. Maybe this could even replace the specialized per_node_arg option. @Raytar maybe you have some thoughts on this?