make most requested changes

src/api/error.rs

@@ -29,7 +29,7 @@ pub enum Error {
 	NotFound,
 
 	// Category: bad request
-	/// The request used an invalid path
+	/// The request contained an invalid UTF-8 sequence in its path or in other parameters
 	#[error(display = "Invalid UTF-8: {}", _0)]
 	InvalidUTF8Str(#[error(source)] std::str::Utf8Error),
 
@@ -65,7 +65,7 @@ impl From<roxmltree::Error> for Error {
 }
 
 impl Error {
-	/// Convert an error into an Http status code
+	/// Get the HTTP status code that best represents the meaning of the error for the client
 	pub fn http_status_code(&self) -> StatusCode {
 		match self {
 			Error::NotFound => StatusCode::NOT_FOUND,

src/api/lib.rs

@@ -1,4 +1,3 @@
-#![deny(missing_crate_level_docs, missing_docs)]
 //! Crate for serving a S3 compatible API
 #[macro_use]
 extern crate log;

src/garage/main.rs

@@ -1,6 +1,5 @@
-#![deny(missing_crate_level_docs, missing_docs)]
 #![recursion_limit = "1024"]
-//! Garage CLI, used to interact with a running Garage instance, and to launch a Garage
+//! Garage CLI, used to interact with a running Garage instance, and to launch a Garage instance
 
 #[macro_use]
 extern crate log;

src/model/block.rs

@@ -157,7 +157,7 @@ impl BlockManager {
 	}
 
 	/// Write a block to disk
-	pub async fn write_block(&self, hash: &Hash, data: &[u8]) -> Result<Message, Error> {
+	async fn write_block(&self, hash: &Hash, data: &[u8]) -> Result<Message, Error> {
 		let _lock = self.data_dir_lock.lock().await;
 
 		let mut path = self.block_dir(hash);
@@ -176,7 +176,7 @@ impl BlockManager {
 	}
 
 	/// Read block from disk, verifying it's integrity
-	pub async fn read_block(&self, hash: &Hash) -> Result<Message, Error> {
+	async fn read_block(&self, hash: &Hash) -> Result<Message, Error> {
 		let path = self.block_path(hash);
 
 		let mut f = match fs::File::open(&path).await {
@@ -208,7 +208,7 @@ impl BlockManager {
 	}
 
 	/// Check if this node should have a block, but don't actually have it
-	pub async fn need_block(&self, hash: &Hash) -> Result<bool, Error> {
+	async fn need_block(&self, hash: &Hash) -> Result<bool, Error> {
 		let needed = self
 			.rc
 			.get(hash.as_ref())?

src/model/block_ref_table.rs

@@ -10,16 +10,14 @@ use crate::block::*;
 
 #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
 pub struct BlockRef {
-	// Primary key
-	/// Hash of the block
+	/// Hash of the block, used as partition key
 	pub block: Hash,
 
-	// Sort key
-	// why a version on a hashed (probably immutable) piece of data?
+	/// Id of the Version for the object containing this block, used as sorting key
 	pub version: UUID,
 
 	// Keep track of deleted status
-	/// Is that block deleted
+	/// Is the Version that contains this block deleted
 	pub deleted: crdt::Bool,
 }
 

src/model/key_table.rs

@@ -6,7 +6,7 @@ use garage_table::*;
 /// An api key
 #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
 pub struct Key {
-	/// The id of the key (immutable)
+	/// The id of the key (immutable), used as partition key
 	pub key_id: String,
 
 	/// The secret_key associated
@@ -19,6 +19,7 @@ pub struct Key {
 	pub deleted: crdt::Bool,
 
 	/// Buckets in which the key is authorized. Empty if `Key` is deleted
+	// CRDT interaction: deleted implies authorized_buckets is empty
 	pub authorized_buckets: crdt::LWWMap<String, PermissionSet>,
 }
 

src/model/lib.rs

@@ -1,4 +1,3 @@
-#![warn(missing_docs)]
 #[macro_use]
 extern crate log;
 

src/model/object_table.rs

@@ -14,13 +14,13 @@ use crate::version_table::*;
 /// An object
 #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
 pub struct Object {
-	/// The bucket in which the object is stored
+	/// The bucket in which the object is stored, used as partition key
 	pub bucket: String,
 
-	/// The key at which the object is stored in its bucket
+	/// The key at which the object is stored in its bucket, used as sorting key
 	pub key: String,
 
-	/// The list of known versions of the object
+	/// The list of currenty stored versions of the object
 	versions: Vec<ObjectVersion>,
 }
 
@@ -53,7 +53,7 @@ impl Object {
 		}
 	}
 
-	/// Get a list of all versions known for `Object`
+	/// Get a list of currently stored versions of `Object`
 	pub fn versions(&self) -> &[ObjectVersion] {
 		&self.versions[..]
 	}
@@ -77,7 +77,7 @@ pub enum ObjectVersionState {
 	Uploading(ObjectVersionHeaders),
 	/// The version is fully received
 	Complete(ObjectVersionData),
-	/// The version was never fully received
+	/// The version uploaded containded errors or the upload was explicitly aborted
 	Aborted,
 }
 
@@ -105,7 +105,7 @@ impl CRDT for ObjectVersionState {
 /// Data about an object version
 #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)]
 pub enum ObjectVersionData {
-	/// The version is deleted
+	/// The object was deleted, this Version is a tombstone to mark it as such
 	DeleteMarker,
 	/// The object is short, it's stored inlined
 	Inline(ObjectVersionMeta, #[serde(with = "serde_bytes")] Vec<u8>),
@@ -159,7 +159,7 @@ impl ObjectVersion {
 		}
 	}
 
-	/// Is the object version available (received and not deleted)
+	/// Is the object version available (received and not a tombstone)
 	pub fn is_data(&self) -> bool {
 		match self.state {
 			ObjectVersionState::Complete(ObjectVersionData::DeleteMarker) => false,

src/model/version_table.rs

@@ -13,7 +13,7 @@ use crate::block_ref_table::*;
 /// A version of an object
 #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
 pub struct Version {
-	/// UUID of the version
+	/// UUID of the version, used as partition key
 	pub uuid: UUID,
 
 	// Actual data: the blocks for this version
@@ -49,9 +49,9 @@ impl Version {
 
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Serialize, Deserialize)]
 pub struct VersionBlockKey {
-	/// Number of the part, starting at 1
+	/// Number of the part
 	pub part_number: u64,
-	/// offset of the block in the file, starting at 0
+	/// Offset of this sub-segment in its part
 	pub offset: u64,
 }
 

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]

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);

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 {

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>,
 

src/table/lib.rs

@@ -1,4 +1,3 @@
-#![warn(missing_docs)]
 #![recursion_limit = "1024"]
 
 #[macro_use]

src/table/replication/parameters.rs

@@ -16,7 +16,6 @@ pub trait TableReplication: Send + Sync {
 	fn write_nodes(&self, hash: &Hash) -> Vec<UUID>;
 	/// Responses needed to consider a write succesfull
 	fn write_quorum(&self) -> usize;
-	// this feels like its write_nodes().len() - write_quorum()
 	fn max_write_errors(&self) -> usize;
 
 	// Accessing partitions, for Merkle tree & sync

src/table/schema.rs

@@ -4,7 +4,7 @@ use garage_util::data::*;
 
 use crate::crdt::CRDT;
 
-/// Trait for partitionnable data
+/// Trait for field used to partition data
 pub trait PartitionKey {
 	/// Get the key used to partition
 	fn hash(&self) -> Hash;
@@ -22,7 +22,7 @@ impl PartitionKey for Hash {
 	}
 }
 
-/// Trait for sortable data
+/// Trait for field used to sort data
 pub trait SortKey {
 	/// Get the key used to sort
 	fn sort_key(&self) -> &[u8];

src/util/error.rs

@@ -1,5 +1,4 @@
 //! Module containing error types used in Garage
-#![allow(missing_docs)]
 use err_derive::Error;
 use hyper::StatusCode;
 use std::io;

src/util/lib.rs

@@ -1,4 +1,3 @@
-#![warn(missing_crate_level_docs, missing_docs)]
 //! Crate containing common functions and types used in Garage
 
 #[macro_use]

src/web/error.rs

@@ -19,7 +19,7 @@ pub enum Error {
 	#[error(display = "Not found")]
 	NotFound,
 
-	/// The client requested a malformed path
+	/// The request contained an invalid UTF-8 sequence in its path or in other parameters
 	#[error(display = "Invalid UTF-8: {}", _0)]
 	InvalidUTF8(#[error(source)] std::str::Utf8Error),
 

src/web/lib.rs

@@ -1,4 +1,3 @@
-#![deny(missing_crate_level_docs, missing_docs)]
 //! Crate for handling web serving of s3 bucket
 #[macro_use]
 extern crate log;