gRPC network kv-memstore example

[sainad2222]

Related to #1271 and #161

---

This change is 

[drmingdrmer]

Reviewed 7 of 18 files at r1, all commit messages.
Reviewable status: 7 of 18 files reviewed, 8 unresolved discussions (waiting on @sainad2222)

---

examples/raft-kv-memstore-grpc/proto/internal_service.proto line 7 at r1 (raw file):

service InternalService {
  // Vote handles vote requests between Raft nodes during leader election
  rpc Vote(RaftRequestBytes) returns (RaftReplyBytes) {}

Although Vote RPC is not on the hot path, defining specific message types for its request and response better represents the voting protocol's semantics.

---

examples/raft-kv-memstore-grpc/proto/internal_service.proto line 10 at r1 (raw file):

  // Append handles call related to append entries RPC
  rpc Append(RaftRequestBytes) returns (RaftReplyBytes) {}

AppendEntries is on the hot path. To avoid double encoding/decoding, the request should be defined with a protobuf message instead of a general serialized bytes wrapper.

---

examples/raft-kv-memstore-grpc/proto/internal_service.proto line 13 at r1 (raw file):

  // Snapshot handles install snapshot RPC
  rpc Snapshot(RaftRequestBytes) returns (RaftReplyBytes) {}

Snapshot is recommended to be transfer via a stream, refer to:
https://github.com/datafuselabs/databend/blob/22a8a82bc3fd532988ed661084eeeb1ef3bf3066/src/meta/types/proto/meta.proto#L273

And using RaftNetworkV2 is recommended. See below.

---

examples/raft-kv-memstore-grpc/src/lib.rs line 21 at r1 (raw file):

        D = Request,
        R = Response,
        Node = Node,

Entry should be defined with protobuf too.

---

examples/raft-kv-memstore-grpc/src/lib.rs line 32 at r1 (raw file):

    pub node_id: u64,
    pub rpc_addr: String,
}

Use protobuf defined Node?

Code quote:

#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq, Default)]
pub struct Node {
    pub node_id: u64,
    pub rpc_addr: String,
}

---

examples/raft-kv-memstore-grpc/README.md line 1 at r1 (raw file):

# Example distributed key-value store built upon openraft.

This README should be updated to reflect the purpose of the gRPC example.

---

examples/raft-kv-memstore-grpc/src/network/mod.rs line 62 at r1 (raw file):

/// Implementation of RaftNetwork trait for handling Raft protocol communications.
#[allow(clippy::blocks_in_conditions)]
impl RaftNetwork<TypeConfig> for NetworkConnection {

Implement RaftNetworkV2 instead of RaftNetwork. V2 provide a more generic snapshot API that allows user to customize the snapshot transmit.

---

examples/raft-kv-memstore-grpc/proto/management_service.proto line 33 at r1 (raw file):

message AddLearnerRequest {
  Node node = 1;      // Node to be added as a learner
  bool blocking = 2;  // Whether to wait for the operation to complete

As an example it does not have to exactly reflect the API and should be simplified if possible. We can assume in this example add_learner is always invoked in blocking mode and this argument is unnecessary.

As a result, some of the request type may be removed. just use Node as the RPC request type.

[sainad2222]

Entry should be defined with protobuf too.

Not sure how to convert all of entry to protobuf and it looks complex to convert with all the traits that we need to implement for that protobuf Entry. I looked for the reference in both databend repo and robustmq repo but couldn't find anything in both. Any references you can share or any pointers to get started?

AppendEntries is on the hot path. To avoid double encoding/decoding, the request should be defined with a protobuf message instead of a general serialized bytes wrapper.

Same as above since request of this depends on Entry. Also wouldn't we writing a converter to convert from protobuf::AppendRequest to openraft::AppendEntriesRequest which also has some overhead? Will it be significantly better than serialization and deserialization?

[schreter]

it looks complex to convert with all the traits that we need to implement for that protobuf Entry

I don't have a solution for you, but maybe a direction: You can add traits yourself. And, for instance, implement AppendEntries gRPC which consumes arbitrary entries fulfilling where EntryType: ProvidesRpcEntry or so and define a trait ProvidesRpcEntry, which will give you the entry directly as gRPC object, which you can then serialize 1:1 into AppendEntries w/o extra steps. This would require the application to use entries which implement the specified trait, but that's OK.

[drmingdrmer]

Entry should be defined with protobuf too.

Not sure how to convert all of entry to protobuf and it looks complex to convert with all the traits that we need to implement for that protobuf Entry. I looked for the reference in both databend repo and robustmq repo but couldn't find anything in both. Any references you can share or any pointers to get started?

You are right.

Currently, Entry cannot be defined in protobuf because it depends on LogId and Membership, which are not part of the RaftTypeConfig trait.

I'll try to refactor the traits so that an Entry that does not depends on LogId and Membership can be introduced.

[sainad2222]

@drmingdrmer I made changes except for Entry and AppendEntries since it also depends on Entry. We can pick these two in a separate PR after your refactor. I used AI to make changes for snapshot function since I am new to streaming RPCs. Let me know if anything can be improved there

Edit: Network connection caching is also pending. You can review the rest in the meantime

[drmingdrmer]

Reviewed 3 of 18 files at r1, 3 of 11 files at r2, 16 of 17 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @sainad2222)

---

examples/raft-kv-memstore-grpc/src/grpc/api_service.rs line 60 at r4 (raw file):

        }
        Ok(())
    }

Empty string could be a valid key or value. It does not have to be such strict.

Code quote:

    /// Validates a key-value request
    fn validate_request(&self, key: &str, value: Option<&str>) -> Result<(), Status> {
        if key.is_empty() {
            return Err(Status::internal("Key cannot be empty"));
        }
        if let Some(val) = value {
            if val.is_empty() {
                return Err(Status::internal("Value cannot be empty"));
            }
        }
        Ok(())
    }

---

examples/raft-kv-memstore-grpc/src/grpc/api_service.rs line 83 at r4 (raw file):

                key: req.key.clone(),
                value: req.value.clone(),
            })

Just use req here.

Suggestion:

            .client_write(req)

---

examples/raft-kv-memstore-grpc/src/store/mod.rs line 29 at r4 (raw file):

pub enum Request {
    Set { key: String, value: String },
}

There should not be a Request again. Where this Request is used, it should be replaced with the protobuf defined request: SetRequest

Code quote:

#[derive(Serialize, Deserialize, Debug, Clone)]
pub enum Request {
    Set { key: String, value: String },
}

---

examples/raft-kv-memstore-grpc/src/store/mod.rs line 43 at r4 (raw file):

pub struct Response {
    pub value: Option<String>,
}

And this should be replace with the protobuf defined type too.

Code quote:

#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct Response {
    pub value: Option<String>,
}

[sainad2222]

@drmingdrmer Resolved new comments too. You can review again

[drmingdrmer]

Looks good enough

Reviewed 12 of 12 files at r5, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @sainad2222)

[drmingdrmer]

Oh one more thing: you need to add a CI entry for this new example, in the openraft/.github/workflows/ci.yaml.
Find the examples: and add this new gRPC example to make sure it passes the test.