Refine cluster initialization procedure

[drmingdrmer]

The cluster init procedure is specialized and introduced duplicated codes.
This procedure could be safely implemented with existing routines.
This way to reduce code and make it more easier to test.

Currently:

• Init effective-membership.
• Enter candidate state.
• Become leader and commit a membership log that contains the initial config or a blank log(cluster already initialized).

Expected:

• Append a membership log at index 0, with term 0
• Enter candidate state.
• Become leader and commit a blank log entry.

This way:

• commit_initial_leader_entry does not need to check last_log_index == 0.
• No more membership operations when initializing a cluster.

[github-actions]

👋 Thanks for opening this issue!

Get help or engage by:

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

[eliasyaoyc]

what i understand is pre-appened a membership log at index 0,with term 0 that helpful for the cluster first startup, as your said we don't need to judge last_log_index.
But not first startup, even if add this index 0 term 0 log , it will be skipped without any impact
Right?

[drmingdrmer]

But not first startup, even if add this index 0 term 0 log , it will be skipped without any impact
Right?

If a node has received any log from some Leader, when starting up it has not to add a log at (0,0);

If a node has no logs at all, when initialize(members) is called, it should add a membership log at (0,0);

[eliasyaoyc]

Okk，i seem to understand.

[eliasyaoyc]

/assignme

[drmingdrmer]

@eliasyaoyc
I am not fully sure this issue can be done before #49 and #61 are done. 🤔

[eliasyaoyc]

today or tomorrow.

[drmingdrmer]

today or tomorrow.

I mean, this issue may be affected by the changes introduced by #49 and #61

[eliasyaoyc]

OKk, when #49 and #61 is over, l will start.

[eliasyaoyc]

general plan, add a param on RaftInner.
when RaftCore::spanw called, then get_log_state will used that determine this node whether received any log.
no log mark this param is true, then identify this param to determine whether to send term 0,index 0 log.

[drmingdrmer]

What is the name of param that will be added? Since IMO, it could always read the state from storage without the help of other additional fields.

Currently, the implementation adds a blank log entry at term:0, index:0, so that the initial value of such as last_log_id is valid.

I've been trying to remove this initial log term:0,index:0. After these get merged, it'd be simpler to get this issue done.

[eliasyaoyc]

What is the name of param that will be added? Since IMO, it could always read the state from storage without the help of other additional fields.

I don't know how to notify initialize(members) that needs to add a term:0, index:0 log without additional field.

[drmingdrmer]

self.core.last_log_id.is_none() would be enough to ensure it is a uninitialized node I think 🤔

[eliasyaoyc]

yes, last_log.is_none() can complete this work.
but initialize(members) haven't core reference.

    pub fn new(id: NodeId, config: Arc<Config>, network: Arc<N>, storage: Arc<S>) -> Self {
       ...
       let raft_handle = RaftCore::spawn(id, config, network, storage, rx_api, tx_metrics, rx_shutdown);
       ...
    }

    pub async fn initialize(&self, members: BTreeSet<NodeId>) -> Result<(), InitializeError> {
        let (tx, rx) = oneshot::channel();
        self.call_core(RaftMsg::Initialize { members, tx }, rx).await
    }

Is there something wrong with my understanding? 😭

[drmingdrmer]

These should be done in handle_init_with_config().

I've get this done in: https://github.com/datafuselabs/openraft/pull/107/files#diff-2b96982d340b2d4f989026d55516c495678f2320fe05d2841c65748f52c42ff5

[drmingdrmer]

• fixed in Refactor: use Option<LogId> every where #107