Refine comments in a few places.

durio/src/main.rs

@@ -157,11 +157,6 @@ mod tests {
     use super::*;
     use std::time::Duration;
 
-    // This test is ignored by default, because it cannot be run with other
-    // Ruaft tests at the same time. All other ruaft tests are compiled with
-    // feature 'integration-test', which is in conflict with this test. This
-    // test intends to verify that durio can be run under normal production
-    // setup, i.e. without 'integration-test'.
     #[tokio::test]
     async fn smoke_test() {
         let kv_addrs: Vec<SocketAddr> = vec![

src/peer_progress.rs

@@ -15,6 +15,10 @@ struct SharedProgress {
     indexes: Mutex<SharedIndexes>,
 }
 
+/// Progress a peer during log sync.
+///
+/// The number of unclaimed sync requests is stored here. This struct also
+/// contains the last known of the length of the logs held by a peer.
 #[derive(Clone)]
 #[repr(align(64))]
 pub(crate) struct PeerProgress {
@@ -36,20 +40,28 @@ impl PeerProgress {
         }
     }
 
-    pub fn should_schedule(&self) -> bool {
-        self.internal.opening.fetch_add(1, Ordering::AcqRel) == 0
-    }
-
+    /// Claim all pending sync requests and clear the request counter.
     pub fn take_task(&self) -> bool {
         self.internal.opening.swap(0, Ordering::AcqRel) != 0
     }
 
+    /// Increase the number of sync requests by one. A new round of log syncing
+    /// should be started if this is the first of a group of requests. No new
+    /// round is needed if the previous requests have not been claimed.
+    pub fn should_schedule(&self) -> bool {
+        self.internal.opening.fetch_add(1, Ordering::AcqRel) == 0
+    }
+
+    /// Reset progress data stored for a peer when a new term is started.
     pub fn reset_progress(&self, next_index: Index) {
         let mut internal = self.internal.indexes.lock();
         internal.next_index = next_index;
         internal.current_step = 0;
     }
 
+    /// Record failure of serving a sync request. The failure is usually caused
+    /// by peers disagreeing on the base commit. Here we chose to step back
+    /// exponentially to limit the rounds of syncing required.
     pub fn record_failure(&self, committed_index: Index) {
         let mut internal = self.internal.indexes.lock();
         let step = &mut internal.current_step;
@@ -70,12 +82,15 @@ impl PeerProgress {
         }
     }
 
+    /// Record a success in serving a sync request. Move the pointer to the next
+    /// log item to sync.
     pub fn record_success(&self, match_index: Index) {
         let mut internal = self.internal.indexes.lock();
         internal.next_index = match_index + 1;
         internal.current_step = 0;
     }
 
+    /// Returns the next log index to sync.
     pub fn next_index(&self) -> Index {
         self.internal.indexes.lock().next_index
     }

src/remote_context.rs

@@ -6,6 +6,29 @@ use crate::term_marker::TermMarker;
 use crate::verify_authority::DaemonBeatTicker;
 use crate::{Peer, RemoteRaft};
 
+/// A struct that contains useful utilities when contacting peers.
+///
+/// The motivation of RemoteContext is to avoid cloning the `Arc<>` to utilities
+/// multiple times for each request. Instead, we embed shared utilities in the
+/// thread local space of each thread in a thread pool. Those references will be
+/// released when the thread pool is destroyed.
+///
+/// This class provides a static reference to those utilities, which also helps
+/// simplifying lifetimes in futures.
+///
+/// A big hack in this class is that we need to store generic structs in a
+/// thread local environment. This is because that the RPC interface is generic
+/// over the data to store in Raft. Thus `RemoteContext` must be generic, too.
+///
+/// Generic thread local variables is not supported in Rust, due to the fact
+/// that one could potentially store multiple instances of the same class with
+/// different generic parameters. The instances must be somehow identified by
+/// their concrete types, which greatly increases implementation complexity.
+///
+/// Luckily in our case, the "multiple instance" issue does not apply. A thread
+/// pool belongs to a known Raft instance, which has one set of known generic
+/// parameters. We would only ever need to store one instance of RemoteContext
+/// in one thread. Thus the ambiguity is casted away in `fetch_context()`.
 #[derive(Clone)]
 pub(crate) struct RemoteContext<Command> {
     term_marker: TermMarker<Command>,