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

introduce a feature to disable/enable to contain a node-id in LogId #660

Closed
drmingdrmer opened this issue Feb 10, 2023 · 1 comment · Fixed by #673
Closed

introduce a feature to disable/enable to contain a node-id in LogId #660

drmingdrmer opened this issue Feb 10, 2023 · 1 comment · Fixed by #673

Comments

@drmingdrmer
Copy link
Member

          > Alternatively, looking at the code again, it seems like `Entry` could use a simpler `LogId` only containing `term` and `index`, without the `node_id`. The `node_id` seems to be only relevant for replication reply, where it can be easily computed based on the last `voted_for` for `Matching`. For `Conflict`, it can be ignored, since it's not used. Of course, the default implementation of `RaftLogReader` would be more complex/impossible and quite a few changes would be needed elsewhere...

LogId is also used on a follower to determine if a log entry is identical to the follower's local log entry at the same index, when a follower receives append-entries request.

I have a LogIdList structure that contains every log id from which a new LeaderId starts:
https://github.com/datafuselabs/openraft/blob/main/openraft/src/engine/log_id_list.rs

In fact, in my opinion, such a structure reflects more precisely the data structure in raft.
Logs should be a 2 dimension list entry = logs[leader_id][index].

May be I can introduce a feature to disable/enable to contain a node-id in LogId. 🤔

I'll probably go with the above mentioned storage optimization in the log ("switch" entries) or we'll store our 16B node IDs for each log entry anyway.

I did not know you have such a big node-id :(
Is it possible using a u32 node-id and store the 16B real id in a Node?
Membership stores every NodeId and Node for applications:

pub struct Membership<NID, N>
where
N: Node,
NID: NodeId,
{
/// Multi configs of members.
///
/// AKA a joint config in original raft paper.
configs: Vec<BTreeSet<NID>>,
/// Additional info of all nodes, e.g., the connecting host and port.
///
/// A node-id key that is in `nodes` but is not in `configs` is a **learner**.
nodes: BTreeMap<NID, N>,
}

Originally posted by @drmingdrmer in #652 (comment)

@github-actions
Copy link

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

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 a pull request may close this issue.

1 participant