Add one more test: unreliable and one key.

kvraft/src/client.rs

@@ -22,27 +22,40 @@ impl Clerk {
         }
     }
 
-    pub fn get(&mut self, key: String) -> Option<String> {
+    pub fn get<K: AsRef<str>>(&mut self, key: K) -> Option<String> {
         let (init, inner) = (&self.init, &mut self.inner);
         init.call_once(|| inner.commit_sentinel());
+        let key = key.as_ref();
         loop {
-            match inner.get(key.clone(), Default::default()) {
+            match inner.get(key.to_owned(), Default::default()) {
                 Some(val) => return val,
                 None => {}
             }
         }
     }
 
-    pub fn put(&mut self, key: String, value: String) -> Option<()> {
+    pub fn put<K: AsRef<str>, V: AsRef<str>>(
+        &mut self,
+        key: K,
+        value: V,
+    ) -> Option<()> {
         let (init, inner) = (&self.init, &mut self.inner);
         init.call_once(|| inner.commit_sentinel());
-        inner.put(key, value, Default::default())
+        let key = key.as_ref();
+        let value = value.as_ref();
+        inner.put(key.to_owned(), value.to_owned(), Default::default())
     }
 
-    pub fn append(&mut self, key: String, value: String) -> Option<()> {
+    pub fn append<K: AsRef<str>, V: AsRef<str>>(
+        &mut self,
+        key: K,
+        value: V,
+    ) -> Option<()> {
         let (init, inner) = (&self.init, &mut self.inner);
         init.call_once(|| inner.commit_sentinel());
-        inner.append(key, value, Default::default())
+        let key = key.as_ref();
+        let value = value.as_ref();
+        inner.append(key.to_owned(), value.to_owned(), Default::default())
     }
 }
 

kvraft/tests/service_test.rs

@@ -1,3 +1,5 @@
+#[macro_use]
+extern crate anyhow;
 extern crate kvraft;
 extern crate rand;
 #[macro_use]
@@ -10,6 +12,7 @@ use std::time::Duration;
 
 use rand::{thread_rng, Rng};
 
+use anyhow::Context;
 use kvraft::testing_utils::config::{make_config, Config};
 use kvraft::Clerk;
 
@@ -43,7 +46,7 @@ fn appending_client(
     let mut last = String::new();
     let mut rng = thread_rng();
 
-    clerk.put(key.clone(), last.clone());
+    clerk.put(&key, &last);
 
     while !stop.load(Ordering::Acquire) {
         eprintln!("client {} starting {}.", index, op_count);
@@ -51,12 +54,12 @@ fn appending_client(
             let value = format!("({}, {}), ", index, op_count);
 
             last.push_str(&value);
-            clerk.append(key.clone(), value);
+            clerk.append(&key, &value);
 
             op_count += 1;
         } else {
             let value = clerk
-                .get(key.clone())
+                .get(&key)
                 .expect(&format!("Key {} should exist.", index));
             assert_eq!(value, last);
         }
@@ -118,6 +121,52 @@ fn generic_test(clients: usize, unreliable: bool, maxraftstate: usize) {
     cfg.end();
 }
 
+fn check_concurrent_results(
+    value: String,
+    clients: usize,
+    expected: Vec<usize>,
+) -> anyhow::Result<()> {
+    if !value.starts_with('(') || !value.ends_with(')') {
+        bail!("Malformed value string {}", value)
+    }
+    let inner_value = &value[1..value.len() - 1];
+    let mut progress = vec![0; clients];
+    for pair_str in inner_value.split(")(") {
+        let mut nums = vec![];
+        for num_str in pair_str.split(", ") {
+            let num: usize = num_str.parse().context(format!(
+                "Parsing '{:?}' failed within '{:?}'",
+                num_str, value,
+            ))?;
+            nums.push(num);
+        }
+        if nums.len() != 2 {
+            bail!(
+                concat!(
+                    "More than two numbers in the same group when",
+                    " parsing '{:?}' failed within '{:?}'",
+                ),
+                pair_str,
+                value,
+            );
+        }
+        let (client, curr) = (nums[0], nums[1]);
+        if progress[client] != curr {
+            bail!(
+                "Client {} failed, expecting {}, got {}, others are {:?} in {}",
+                client,
+                progress[client],
+                curr,
+                progress,
+                value,
+            )
+        }
+        progress[client] = curr + 1;
+    }
+    assert_eq!(progress, expected, "Expecting progress in {}", value);
+    Ok(())
+}
+
 #[test]
 fn basic_service() {
     generic_test(1, false, 0);
@@ -132,3 +181,29 @@ fn concurrent_client() {
 fn unreliable() {
     generic_test(5, true, 0);
 }
+
+#[test]
+fn unreliable_one_key() -> anyhow::Result<()> {
+    const SERVERS: usize = 5;
+    let cfg = Arc::new(make_config(SERVERS, true, 0));
+    let mut clerk = cfg.make_clerk();
+
+    cfg.begin("Test: concurrent append to same key, unreliable (3A)");
+
+    clerk.put("k", "");
+
+    const CLIENTS: usize = 5;
+    const ATTEMPTS: usize = 10;
+    let client_results = spawn_clients(cfg, CLIENTS, |index, mut clerk| {
+        for i in 0..ATTEMPTS {
+            clerk.append("k", format!("({}, {})", index, i));
+        }
+    });
+    for client_result in client_results {
+        client_result.join().expect("Client should never fail");
+    }
+
+    let value = clerk.get("k").expect("Key should exist");
+
+    check_concurrent_results(value, CLIENTS, vec![ATTEMPTS; CLIENTS])
+}