Refactor: Introduce RefLogId as a reference to a log ID

Existing `LogIdOf<C>` provides a minimal storage implementation for a
log ID with essential properties. In contrast, `RefLogId` offers the
same information as `LogIdOf<C>` while adding additional system-defined
properties.

For example, in the future, `LogIdOf<C>` defined by the application will
not need to implement `Ord`. However, `RefLogId`, used internally, will
provide a system-defined `Ord` implementation.

This change updates internal components to return `RefLogId` or accept
it as an argument where possible, enabling more flexibility and
consistency in handling log IDs.

openraft/src/engine/handler/log_handler/mod.rs

@@ -2,6 +2,7 @@ use crate::display_ext::DisplayOptionExt;
 use crate::engine::Command;
 use crate::engine::EngineConfig;
 use crate::engine::EngineOutput;
+use crate::log_id::option_ref_log_id_ext::OptionRefLogIdExt;
 use crate::raft_state::LogStateReader;
 use crate::type_config::alias::LogIdOf;
 use crate::LogIdOptionExt;
@@ -108,6 +109,6 @@ where C: RaftTypeConfig
             st
         );
 
-        log_id
+        log_id.to_log_id()
     }
 }

openraft/src/engine/log_id_list.rs

@@ -1,6 +1,7 @@
 use std::ops::RangeInclusive;
 
 use crate::engine::leader_log_ids::LeaderLogIds;
+use crate::log_id::ref_log_id::RefLogId;
 use crate::log_id::RaftLogId;
 use crate::storage::RaftLogReaderExt;
 use crate::type_config::alias::LogIdOf;
@@ -275,24 +276,30 @@ where C: RaftTypeConfig
         }
     }
 
-    /// Get the log id at the specified index.
+    /// Get the log id at the specified index in a [`RefLogId`].
     ///
     /// It will return `last_purged_log_id` if index is at the last purged index.
-    // leader_id: Copy is feature gated
     #[allow(clippy::clone_on_copy)]
-    pub(crate) fn get(&self, index: u64) -> Option<LogIdOf<C>> {
+    pub(crate) fn get(&self, index: u64) -> Option<RefLogId<'_, C>> {
         let res = self.key_log_ids.binary_search_by(|log_id| log_id.index().cmp(&index));
 
-        match res {
-            Ok(i) => Some(LogIdOf::<C>::new(self.key_log_ids[i].leader_id().clone(), index)),
+        // Index of the leadership change point that covers the target index.
+        // It points to either:
+        // - The exact matching log entry if found, or
+        // - The most recent change point before the target index
+        let change_point = match res {
+            Ok(i) => i,
             Err(i) => {
+                // i - 1 is the last one that is smaller than the input.
                 if i == 0 || i == self.key_log_ids.len() {
-                    None
+                    return None;
                 } else {
-                    Some(LogIdOf::<C>::new(self.key_log_ids[i - 1].leader_id().clone(), index))
+                    i - 1
                 }
             }
-        }
+        };
+
+        Some(RefLogId::new(self.key_log_ids[change_point].leader_id(), index))
     }
 
     pub(crate) fn first(&self) -> Option<&LogIdOf<C>> {

openraft/src/engine/tests/log_id_list_test.rs

@@ -1,6 +1,7 @@
 use crate::engine::leader_log_ids::LeaderLogIds;
 use crate::engine::testing::UTConfig;
 use crate::engine::LogIdList;
+use crate::log_id::option_ref_log_id_ext::OptionRefLogIdExt;
 use crate::testing::log_id;
 
 #[test]
@@ -338,17 +339,17 @@ fn test_log_id_list_get_log_id() -> anyhow::Result<()> {
         log_id(7, 1, 10),
     ]);
 
-    assert_eq!(None, ids.get(0));
-    assert_eq!(Some(log_id(1, 1, 1)), ids.get(1));
-    assert_eq!(Some(log_id(1, 1, 2)), ids.get(2));
-    assert_eq!(Some(log_id(3, 1, 3)), ids.get(3));
-    assert_eq!(Some(log_id(3, 1, 4)), ids.get(4));
-    assert_eq!(Some(log_id(3, 1, 5)), ids.get(5));
-    assert_eq!(Some(log_id(5, 1, 6)), ids.get(6));
-    assert_eq!(Some(log_id(5, 1, 7)), ids.get(7));
-    assert_eq!(Some(log_id(7, 1, 8)), ids.get(8));
-    assert_eq!(Some(log_id(7, 1, 9)), ids.get(9));
-    assert_eq!(Some(log_id(7, 1, 10)), ids.get(10));
+    assert_eq!(None, ids.get(0).to_log_id());
+    assert_eq!(Some(log_id(1, 1, 1)), ids.get(1).to_log_id());
+    assert_eq!(Some(log_id(1, 1, 2)), ids.get(2).to_log_id());
+    assert_eq!(Some(log_id(3, 1, 3)), ids.get(3).to_log_id());
+    assert_eq!(Some(log_id(3, 1, 4)), ids.get(4).to_log_id());
+    assert_eq!(Some(log_id(3, 1, 5)), ids.get(5).to_log_id());
+    assert_eq!(Some(log_id(5, 1, 6)), ids.get(6).to_log_id());
+    assert_eq!(Some(log_id(5, 1, 7)), ids.get(7).to_log_id());
+    assert_eq!(Some(log_id(7, 1, 8)), ids.get(8).to_log_id());
+    assert_eq!(Some(log_id(7, 1, 9)), ids.get(9).to_log_id());
+    assert_eq!(Some(log_id(7, 1, 10)), ids.get(10).to_log_id());
     assert_eq!(None, ids.get(11));
 
     Ok(())

openraft/src/log_id/mod.rs

@@ -5,6 +5,9 @@ mod log_id_option_ext;
 mod log_index_option_ext;
 mod raft_log_id;
 
+pub(crate) mod option_ref_log_id_ext;
+pub(crate) mod ref_log_id;
+
 use std::fmt::Display;
 use std::fmt::Formatter;
 

openraft/src/log_id/option_ref_log_id_ext.rs

@@ -0,0 +1,17 @@
+use crate::log_id::ref_log_id::RefLogId;
+use crate::type_config::alias::LogIdOf;
+use crate::RaftTypeConfig;
+
+pub(crate) trait OptionRefLogIdExt<C>
+where C: RaftTypeConfig
+{
+    fn to_log_id(&self) -> Option<LogIdOf<C>>;
+}
+
+impl<C> OptionRefLogIdExt<C> for Option<RefLogId<'_, C>>
+where C: RaftTypeConfig
+{
+    fn to_log_id(&self) -> Option<LogIdOf<C>> {
+        self.as_ref().map(|r| r.to_log_id())
+    }
+}

openraft/src/log_id/ref_log_id.rs

@@ -0,0 +1,46 @@
+use std::fmt::Display;
+use std::fmt::Formatter;
+
+use crate::type_config::alias::CommittedLeaderIdOf;
+use crate::type_config::alias::LogIdOf;
+use crate::RaftTypeConfig;
+
+/// A reference to a log id, combining a reference to a committed leader ID and an index.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub(crate) struct RefLogId<'k, C>
+where C: RaftTypeConfig
+{
+    pub(crate) committed_leader_id: &'k CommittedLeaderIdOf<C>,
+    pub(crate) index: u64,
+}
+
+impl<'l, C> RefLogId<'l, C>
+where C: RaftTypeConfig
+{
+    pub(crate) fn new(committed_leader_id: &'l CommittedLeaderIdOf<C>, index: u64) -> Self {
+        RefLogId {
+            committed_leader_id,
+            index,
+        }
+    }
+
+    pub(crate) fn committed_leader_id(&self) -> &CommittedLeaderIdOf<C> {
+        self.committed_leader_id
+    }
+
+    pub(crate) fn index(&self) -> u64 {
+        self.index
+    }
+
+    pub(crate) fn to_log_id(&self) -> LogIdOf<C> {
+        LogIdOf::<C>::new(self.committed_leader_id.clone(), self.index)
+    }
+}
+
+impl<C> Display for RefLogId<'_, C>
+where C: RaftTypeConfig
+{
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}.{}", self.committed_leader_id(), self.index())
+    }
+}

openraft/src/progress/entry/tests.rs

@@ -2,6 +2,7 @@ use std::borrow::Borrow;
 
 use crate::engine::testing::UTConfig;
 use crate::engine::EngineConfig;
+use crate::log_id::ref_log_id::RefLogId;
 use crate::progress::entry::ProgressEntry;
 use crate::progress::inflight::Inflight;
 use crate::raft_state::LogStateReader;
@@ -118,6 +119,10 @@ impl LogStateReader<UTConfig> for LogState {
         }
     }
 
+    fn ref_log_id(&self, _index: u64) -> Option<RefLogId<'_, UTConfig>> {
+        todo!()
+    }
+
     fn last_log_id(&self) -> Option<&LogIdOf<UTConfig>> {
         self.last.as_ref()
     }

openraft/src/raft_state/log_state_reader.rs

@@ -1,3 +1,5 @@
+use crate::log_id::option_ref_log_id_ext::OptionRefLogIdExt;
+use crate::log_id::ref_log_id::RefLogId;
 use crate::type_config::alias::LogIdOf;
 use crate::LogIdOptionExt;
 use crate::RaftTypeConfig;
@@ -34,12 +36,16 @@ where C: RaftTypeConfig
         }
     }
 
+    fn get_log_id(&self, index: u64) -> Option<LogIdOf<C>> {
+        self.ref_log_id(index).to_log_id()
+    }
+
     /// Get the log id at the specified index.
     ///
     /// It will return `last_purged_log_id` if index is at the last purged index.
     /// If the log at the specified index is smaller than `last_purged_log_id`, or greater than
     /// `last_log_id`, it returns None.
-    fn get_log_id(&self, index: u64) -> Option<LogIdOf<C>>;
+    fn ref_log_id(&self, index: u64) -> Option<RefLogId<'_, C>>;
 
     /// The last known log id in the store.
     ///

openraft/src/raft_state/mod.rs

@@ -36,6 +36,7 @@ pub(crate) use vote_state_reader::VoteStateReader;
 
 use crate::base::ord_by::OrdBy;
 use crate::display_ext::DisplayOptionExt;
+use crate::log_id::ref_log_id::RefLogId;
 use crate::proposer::Leader;
 use crate::proposer::LeaderQuorumSet;
 use crate::type_config::alias::InstantOf;
@@ -109,7 +110,7 @@ where C: RaftTypeConfig
 impl<C> LogStateReader<C> for RaftState<C>
 where C: RaftTypeConfig
 {
-    fn get_log_id(&self, index: u64) -> Option<LogIdOf<C>> {
+    fn ref_log_id(&self, index: u64) -> Option<RefLogId<'_, C>> {
         self.log_ids.get(index)
     }