Feature: Transfer Leader

[drmingdrmer]

Changelog

Feature: Transfer Leader

Call Raft.trigger().transfer_leader(to) to inform the raft node to
transfer its leadership to Node to.

This feature is enabled only when: the application uses RaftNetworkV2
and implements the RaftNetworkV2::transfer_leader() methods. This method
provides a default implementation that returns an Unreachable, and
such an error will be just ignored.

Application upgrading Openraft from older version does not need to modify any
codes, unless TransferLeader is required.

Upgrade tip:

Implement RaftNetworkV2::transfer_leader() to send the
TransferLeaderRequest to the target node.
The target node that receives this request should then pass it to
Raft::transfer_leader().

---

This change is 

[schreter]

Nice!

Reviewed 27 of 27 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @drmingdrmer)

---

openraft/src/engine/handler/leader_handler/mod.rs line 112 at r1 (raw file):

    /// Disable proposing new logs for this Leader, and transfer Leader to another node
    // TODO: test

?

---

openraft/src/raft/mod.rs line 650 at r1 (raw file):

    ///
    /// [`RaftNetworkV2::transfer_leader`]: crate::network::v2::RaftNetworkV2::transfer_leader
    #[since(version = "0.10.0")]

?

In general, it is a bit convoluted. Wouldn't it make more sense to create a tiny wrapper here for the trigger named transfer_leader() and rename this method to handle_transfer_leader_request() or so, so it's clear what is what? Or at least rename this method to indicate it's not the request, but the handler?

Suggestion:

    /// Otherwise, it just resets the Leader lease to allow the `to` node to become the Leader.
    ///
    /// The application calls
    /// [`Raft::trigger().transfer_leader()`](crate::raft::trigger::Trigger::transfer_leader) to
    /// submit Transfer Leader command. Then, the current Leader will broadcast it to every node in
    /// the cluster via [`RaftNetworkV2::transfer_leader`] and the implementation on the remote node
    /// responds to transfer leader request by calling this method.
    ///
    /// [`RaftNetworkV2::transfer_leader`]: crate::network::v2::RaftNetworkV2::transfer_leader
    #[since(version = "0.10.0")]

[drmingdrmer]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @schreter)

---

openraft/src/engine/handler/leader_handler/mod.rs line 112 at r1 (raw file):

Previously, schreter wrote…

?

˙–˙

---

openraft/src/raft/mod.rs line 650 at r1 (raw file):

Previously, schreter wrote…

?

In general, it is a bit convoluted. Wouldn't it make more sense to create a tiny wrapper here for the trigger named transfer_leader() and rename this method to handle_transfer_leader_request() or so, so it's clear what is what? Or at least rename this method to indicate it's not the request, but the handler?

handle_transfer_leader_request() would be good.

But I did not get what you mean about "a tiny wrapper here for the trigger". 🤔

[schreter]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @drmingdrmer)

---

openraft/src/raft/mod.rs line 650 at r1 (raw file):

"a tiny wrapper here for the trigger". 🤔

Simply something like this:

async fn transfer_leader(to: ID) -> Result<(), ...> {
    self.trigger().transfer_leader(to).await
}

[drmingdrmer]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @schreter)

---

openraft/src/raft/mod.rs line 650 at r1 (raw file):

Previously, schreter wrote…

"a tiny wrapper here for the trigger". 🤔

Simply something like this:

async fn transfer_leader(to: ID) -> Result<(), ...> {
    self.trigger().transfer_leader(to).await
}

Oh. I'd like to put all user triggered command to trigger().

[schreter]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @drmingdrmer)

---

openraft/src/raft/mod.rs line 650 at r1 (raw file):

Oh. I'd like to put all user triggered command to trigger().

No problem, it was just an idea.