Killing the leader of a cluster generates excessive network attempts for replicating log entries

[undecidedapollo]

Describe the bug
When the leader of a cluster with multiple members is killed, a new follower is promoted to leader. The new leader then tries to replicate log entries repeatedly with no delay to the old leader which is no longer running.

All logs, screenshots, etc. are with log level set to warn to highlight the problem areas, frequency of issues.

To Reproduce
Steps to reproduce the behavior:

1. Pull latest main
2. cd examples/raft-kv-memstore
3. Run modified test-cluster.sh script attached: test-cluster.sh.zip
4. Check the logs on either n2.log or n3.log for repeated excessive network attempts, even with logs set to WARN it fills up the file quickly

To mitigate this I can apply the following diff to wait until the next tick vs. immediately trying again. I'm not certain the impact this has on the overall code though:

diff --git a/openraft/src/engine/handler/replication_handler/mod.rs b/openraft/src/engine/handler/replication_handler/mod.rs
index a98492dd..09bb39fb 100644
--- a/openraft/src/engine/handler/replication_handler/mod.rs
+++ b/openraft/src/engine/handler/replication_handler/mod.rs
@@ -324,22 +324,6 @@ where C: RaftTypeConfig
         // The purge job may be postponed because a replication task is using them.
         // Thus we just try again to purge when progress is updated.
         self.try_purge_log();
-
-        // initialize next replication to this target
-
-        {
-            let p = self.leader.progress.get_mut(&target).unwrap();
-
ries);
-            tracing::debug!(next_send_res = debug(&r), "next_send");
-
-            if let Ok(inflight) = r {
-                Self::send_to_target(self.output, &target, inflight);
-            } else {
-                // TODO:
-                tracing::debug!("can not send: TODO");
-            }
-        }
     }

Expected behavior
I'd expect it to retry once every tick or with an exponential backoff if this occurred, not trying immediately.

Actual behavior
Generates requests continuously:

Env (please complete the following information):

• Openraft version: main -> 9e4ba84
• Does the bug still there in the latest version? yes
• Rust-toolchain: cargo 1.78.0-nightly (f772ec022 2024-03-01)
• OS: mac os 14.3.1 (23D60)
• CPU: Apple M2 Max

Additional files:

• Logs + Cargolock of raft-kv-memstore:
  LogsAndLock.zip

Additional context
I'm new to working with raft so may just be missing something here. Noticed this when working with my own project and had similar issues.

[github-actions]

👋 Thanks for opening this issue!

Get help or engage by:

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

[drmingdrmer]

I can not download the log file you attached, which gave me a 404 error. :(

But I guess what happened with your application is this problem:
https://docs.rs/openraft/0.9.1/openraft/docs/faq/index.html#how-to-minimize-error-logging-when-a-follower-is-offline

How to minimize error logging when a follower is offline?

Excessive error logging, like ERROR openraft::replication: 248: RPCError err=NetworkError: ..., occurs when a follower node becomes unresponsive. To alleviate this, implement a mechanism within RaftNetwork that returns a Unreachable error instead of a NetworkError when immediate replication retries to the affected node are not advised.

[undecidedapollo]

Hey @drmingdrmer, thanks for looking into this and sending over that link. That is what seems to be happening. After updating the raft_network_impl.rs file to check if it was a connection related error and return Unreachable instead it behaves as expected.

        let resp = client
            .post(url)
            .json(&req)
            .send()
            .await
            .map_err(|e| {
                if e.is_connect() {
                    return openraft::error::RPCError::Unreachable(Unreachable::new(&e));
                }
                openraft::error::RPCError::Network(NetworkError::new(&e))
            })?;

Any concerns if I add this change to the example in the repo along w/ a comment on why? I feel it may add benefit to others if they encounter the same issue.

[drmingdrmer]

Any concerns if I add this change to the example in the repo along w/ a comment on why? I feel it may add benefit to others if they encounter the same issue.

Absolutely, let's polish it up!

It gonna be an excellent job sharpening the example to fit perfectly into this case!