From 74373aebcfdfcf5c03e4fb6510d8dd664a0b9b88 Mon Sep 17 00:00:00 2001
From: Trinity Pointard <trinity.pointard@gmail.com>
Date: Tue, 6 Apr 2021 05:25:28 +0200
Subject: make most requested changes

---
 src/rpc/lib.rs        |  1 -
 src/rpc/membership.rs | 11 ++++++-----
 src/rpc/ring.rs       |  4 +++-
 src/rpc/rpc_client.rs |  9 +++++----
 4 files changed, 14 insertions(+), 11 deletions(-)

(limited to 'src/rpc')

diff --git a/src/rpc/lib.rs b/src/rpc/lib.rs
index 308baa83..96561d0e 100644
--- a/src/rpc/lib.rs
+++ b/src/rpc/lib.rs
@@ -1,4 +1,3 @@
-#![warn(missing_crate_level_docs, missing_docs)]
 //! Crate containing rpc related functions and types used in Garage
 
 #[macro_use]
diff --git a/src/rpc/membership.rs b/src/rpc/membership.rs
index c465ce68..4fce1a7b 100644
--- a/src/rpc/membership.rs
+++ b/src/rpc/membership.rs
@@ -1,4 +1,4 @@
-/// Module containing structs related to membership management
+//! Module containing structs related to membership management
 use std::collections::HashMap;
 use std::fmt::Write as FmtWrite;
 use std::io::{Read, Write};
@@ -96,7 +96,7 @@ pub struct System {
 	rpc_client: Arc<RpcClient<Message>>,
 
 	pub(crate) status: watch::Receiver<Arc<Status>>,
-	/// The ring, viewed by this node
+	/// The ring
 	pub ring: watch::Receiver<Arc<Ring>>,
 
 	update_lock: Mutex<Updaters>,
@@ -114,9 +114,8 @@ struct Updaters {
 #[derive(Debug, Clone)]
 pub struct Status {
 	/// Mapping of each node id to its known status
-	// considering its sorted regularly, maybe it should be a btreeset?
 	pub nodes: HashMap<UUID, Arc<StatusEntry>>,
-	/// Hash of this entry
+	/// Hash of `nodes`, used to detect when nodes have different views of the cluster
 	pub hash: Hash,
 }
 
@@ -380,7 +379,7 @@ impl System {
 					id_option,
 					addr,
 					sys.rpc_client
-						.get_addr()
+						.by_addr()
 						.call(&addr, ping_msg_ref, PING_TIMEOUT)
 						.await,
 				)
@@ -418,6 +417,8 @@ impl System {
 				}
 			} else if let Some(id) = id_option {
 				if let Some(st) = status.nodes.get_mut(id) {
+					// TODO this might double-increment the value as the counter is already
+					// incremented for any kind of failure in rpc_client
 					st.num_failures.fetch_add(1, Ordering::SeqCst);
 					if !st.is_up() {
 						warn!("Node {:?} seems to be down.", id);
diff --git a/src/rpc/ring.rs b/src/rpc/ring.rs
index 6f341fa8..04f8b590 100644
--- a/src/rpc/ring.rs
+++ b/src/rpc/ring.rs
@@ -1,4 +1,5 @@
 //! Module containing types related to computing nodes which should receive a copy of data blocks
+//! and metadata
 use std::collections::{HashMap, HashSet};
 use std::convert::TryInto;
 
@@ -197,7 +198,7 @@ impl Ring {
 		top >> (16 - PARTITION_BITS)
 	}
 
-	/// Get the list of partitions and
+	/// Get the list of partitions and the first hash of a partition key that would fall in it
 	pub fn partitions(&self) -> Vec<(Partition, Hash)> {
 		let mut ret = vec![];
 
@@ -211,6 +212,7 @@ impl Ring {
 		ret
 	}
 
+	// TODO rename this function as it no longer walk the ring
 	/// Walk the ring to find the n servers in which data should be replicated
 	pub fn walk_ring(&self, from: &Hash, n: usize) -> Vec<UUID> {
 		if self.ring.len() != 1 << PARTITION_BITS {
diff --git a/src/rpc/rpc_client.rs b/src/rpc/rpc_client.rs
index 53051cee..8a6cc721 100644
--- a/src/rpc/rpc_client.rs
+++ b/src/rpc/rpc_client.rs
@@ -53,6 +53,7 @@ impl RequestStrategy {
 		self
 	}
 	/// Set if requests can be dropped after quorum has been reached
+	/// In general true for read requests, and false for write
 	pub fn interrupt_after_quorum(mut self, interrupt: bool) -> Self {
 		self.rs_interrupt_after_quorum = interrupt;
 		self
@@ -103,8 +104,8 @@ impl<M: RpcMessage + 'static> RpcClient<M> {
 		self.local_handler.swap(Some(Arc::new((my_id, handler))));
 	}
 
-	/// Get the server address this client connect to
-	pub fn get_addr(&self) -> &RpcAddrClient<M> {
+	/// Get a RPC client to make calls using node's SocketAddr instead of its ID
+	pub fn by_addr(&self) -> &RpcAddrClient<M> {
 		&self.rpc_addr_client
 	}
 
@@ -166,7 +167,7 @@ impl<M: RpcMessage + 'static> RpcClient<M> {
 	}
 
 	/// Make a RPC call to multiple servers, returning either a Vec of responses, or an error if
-	/// strategy could not be respected due to to many errors
+	/// strategy could not be respected due to too many errors
 	pub async fn try_call_many(
 		self: &Arc<Self>,
 		to: &[UUID],
@@ -226,7 +227,7 @@ impl<M: RpcMessage + 'static> RpcClient<M> {
 	}
 }
 
-/// Endpoint to which send RPC
+/// Thin wrapper arround an `RpcHttpClient` specifying the path of the request
 pub struct RpcAddrClient<M: RpcMessage> {
 	phantom: PhantomData<M>,
 
-- 
cgit v1.2.3