[design] Command/Query Asymmetry

[kevburnsjr]

Proposal results include a uint64 Value and some Data in the form of a []byte.

func (nh *NodeHost) SyncPropose(ctx context.Context, session *client.Session, cmd []byte) (sm.Result, error)

type Result struct {
	Value uint64
	Data []byte
}

Query results include an error and some Data in the form of an interface{} or []byte

func (nh *NodeHost) StaleRead(shardID uint64, query interface{}) (interface{}, error)
func (nh *NodeHost) ReadLocalNode(rs *RequestState, query interface{}) (interface{}, error)
func (nh *NodeHost) NAReadLocalNode(rs *RequestState, query []byte) ([]byte, error)

What is the purpose of Value uint64 and why is it not included in query method responses?

[lni]

What is the purpose of Value uint64 and why is it not included in query method responses?

Thanks for the great question. The current approach is all about avoiding memory allocation.

When a proposal is applied, the proposal entry is already in memory. When an integer is all what you want to return, e.g. a status code indicating the outcome of applied proposal, such an uint64 value helps to avoid any extra memory allocation.

Some users might want to return an arbitrary struct for his/her updates implemented using proposals, it not recommended (which is documented). I thus didn't pay much attention to that []byte slice returned in Result.

For reads, according memory, dragonboat initially had a []byte slice as input to indicate what to be queried and it returns a []byte slice as the result. It was probably thought to be not that useful to return just an integer as the result. That worked for myself pretty well for quite long time until someone complained that his/her code is being forced to keep serializing/deserializing query inputs and results. At that time, returning an interface{} would guarantee an allocation, so when query input and result are converted to interface{}, NAReadLocalNode (NA means No Allocation) was added to use the existing []byte input and output to avoid allocations caused by having that interface{}.

I think you are 100% right, when you put those APIs together, they look strange & confusing when exhibiting such mentioned asymmetry.

Any recommendation on how to make it better & simple while still minimizing allocations? Will be great if I can get some feedbacks.

[kevburnsjr]

Thanks for the response.

Right now I'm developing an internal API which will allow any host in the cluster to receive a request for any shard and forward it internally to a host that has the necessary replica.

syntax = "proto3";

option go_package = "github.com/logbn/zongzi/internal";

// MessageService provides a gRPC api for internal host communication
service MessageService {
	// Propose provides unary request/response command forwarding
	rpc Propose(Request) returns (Response) {}

	// Stream provides streaming proposal forwarding
	rpc Stream(stream Request) returns (stream Response) {}

	// Query provides unary request/response query forwarding
	rpc Query(Request) returns (Response) {}

	// Watch provides streaming query response forwarding
	rpc Watch(Request) returns (stream Response) {}
}

message Request {
    uint64 shard_id = 1;
    uint64 replica_id = 2;
    bool linear = 3;
    bytes data = 15;
}

message Response {
    uint64 value = 1;
    string error = 2;
    bytes data = 15;
}

Serialization is going to need to happen either way for queries forwarded across the internal API.

The asymmetry is not that big a deal. The empty interface queries work fine for me because I can just define my own interface for what a statemachine should be and use a wrapper to hide the difference.

I ask because having spent a long time using HTTP, I'm accustomed to receiving a status code response for both writes and reads. I imagine that a lot of developers might feel the same.

I feel like a uniform Command/Query interface, while possibly less efficient, might make the interface more relatable and the query serialization overhead might not be a performance bottleneck if the query already has to be serialized across the network.

I think I will include uint64 Value in query responses for symmetry for what I'm building. I also recognize that mine is just one use case for Dragonboat and keeping Dragonboat's interfaces performant and flexible where possible for those who want to take advantage of those features is necessary in order to retain the title of most performant multi-raft implementation in Go.

[kevburnsjr]

Closing as asked and answered.

Maybe consider enabling Discussions for this repo to make conversations like this more discoverable.
https://github.com/lni/dragonboat/settings#features

[lni]

Sorry that I missed your reply above as the issue was closed and I was just looking at open issues.

I ask because having spent a long time using HTTP, I'm accustomed to receiving a status code response for both writes and reads. I imagine that a lot of developers might feel the same.

agreed.

I feel like a uniform Command/Query interface, while possibly less efficient, might make the interface more relatable and the query serialization overhead might not be a performance bottleneck if the query already has to be serialized across the network.

I am not worried about the cost of serialization, I am more concerned with allocation. depends on how serialization is done, sometimes more serializations lead to more allocations. if you look at the raftpb folder, there are some changes applied to protobuf generated code to avoid some unwanted allocations.

I think I will include uint64 Value in query responses for symmetry for what I'm building.

totally agreed. there is no cost of returning such an uint64, it is useful to quite a few users, makes the interface more consistent.

please allow a few weeks (as I am busy with another consensus related project I am building), this change will definitely be in for v4.0. thanks again for raising and addressing this issue.