From a2a2b914299edd464759b7cf3dc3dca65744f7c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= <drdr.xp@gmail.com>
Date: Tue, 19 Mar 2024 10:50:13 +0800
Subject: [PATCH] Fix: Ensure `RaftMetrics` are sent after `RaftDataMetrics`
 and `RaftServerMetrics`

This commit addresses an issue where `RaftMetrics` could be sent before
`RaftDataMetrics` and `RaftServerMetrics`. Since `Wait` relies solely on
`RaftMetrics` to reflect the latest state, it is crucial that
`RaftMetrics` are dispatched only after the other two metrics have been
updated. This change guarantees that once a change in `RaftMetrics` is
detected, it accurately represents the most recent changes from both
`RaftDataMetrics` and `RaftServerMetrics`.
---
 openraft/src/core/raft_core.rs | 35 ++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs
index 1dc9e481b..f2afc83aa 100644
--- a/openraft/src/core/raft_core.rs
+++ b/openraft/src/core/raft_core.rs
@@ -550,13 +550,6 @@ where
             replication: replication.clone(),
         };
 
-        tracing::debug!("report_metrics: {}", m.summary());
-        let res = self.tx_metrics.send(m);
-
-        if let Err(err) = res {
-            tracing::error!(error=%err, id=display(self.id), "error reporting metrics");
-        }
-
         let data_metrics = RaftDataMetrics {
             last_log: st.last_log_id().copied(),
             last_applied: st.io_applied().copied(),
@@ -565,13 +558,6 @@ where
             millis_since_quorum_ack,
             replication,
         };
-        self.tx_data_metrics.send_if_modified(|metrix| {
-            if data_metrics.ne(metrix) {
-                *metrix = data_metrics.clone();
-                return true;
-            }
-            false
-        });
 
         let server_metrics = RaftServerMetrics {
             id: self.id,
@@ -580,6 +566,20 @@ where
             current_leader,
             membership_config,
         };
+
+        // Start to send metrics
+        // `RaftMetrics` is sent last, because `Wait` only examines `RaftMetrics`
+        // but not `RaftDataMetrics` and `RaftServerMetrics`.
+        // Thus if `RaftMetrics` change is perceived, the other two should have been updated.
+
+        self.tx_data_metrics.send_if_modified(|metrix| {
+            if data_metrics.ne(metrix) {
+                *metrix = data_metrics.clone();
+                return true;
+            }
+            false
+        });
+
         self.tx_server_metrics.send_if_modified(|metrix| {
             if server_metrics.ne(metrix) {
                 *metrix = server_metrics.clone();
@@ -587,6 +587,13 @@ where
             }
             false
         });
+
+        tracing::debug!("report_metrics: {}", m.summary());
+        let res = self.tx_metrics.send(m);
+
+        if let Err(err) = res {
+            tracing::error!(error=%err, id=display(self.id), "error reporting metrics");
+        }
     }
 
     /// Handle the admin command `initialize`.