Improve server shutdown heuristic and fix Windows path evaluation

harness/src/test/mod.rs

@@ -26,7 +26,7 @@
 
 use crate::{
     build::BuildMode,
-    util::{self, SLEEP_FOR_TERMINATION},
+    util::{self},
     HarnessError, HarnessResult,
 };
 use openssl::{
@@ -57,7 +57,7 @@ pub fn run_test() -> HarnessResult<()> {
 
     let ret = run_test_inner();
     let kill_check = svc::kill_servers();
-    util::sleep_sec(SLEEP_FOR_TERMINATION);
+    svc::wait_for_server_exit()?;
     if let Err(e) = kill_check {
         error!("Failed to kill servers with error: {e}");
     }

harness/src/test/svc.rs

@@ -25,7 +25,7 @@
 */
 
 use crate::{
-    util::{self, SLEEP_FOR_TERMINATION},
+    util::{self},
     HarnessError, HarnessResult,
 };
 use skytable::Connection;
@@ -36,6 +36,8 @@ use std::{
     path::Path,
     process::{Child, Command},
 };
+
+const ROOT_DIR: &str = env!("ROOT_DIR");
 #[cfg(windows)]
 /// The powershell script hack to send CTRL+C using kernel32
 const POWERSHELL_SCRIPT: &str = include_str!("../../../ci/windows/stop.ps1");
@@ -70,6 +72,26 @@ pub fn get_run_server_cmd(server_id: &'static str, target_folder: impl AsRef<Pat
     cmd
 }
 
+pub(super) fn wait_for_server_exit() -> HarnessResult<()> {
+    for (server_id, _) in SERVERS {
+        let mut backoff = 1;
+        let path = format!("{ROOT_DIR}{server_id}/.sky_pid");
+        while Path::new(&path).exists() {
+            if backoff > 64 {
+                return Err(HarnessError::Other(format!(
+                    "Backoff elapsed. {server_id} process did not exit. PID file still present"
+                )));
+            }
+            info!("{server_id} process still live. Sleeping for {backoff} second(s)");
+            util::sleep_sec(backoff);
+            backoff *= 2;
+        }
+        info!("{server_id} has exited completely");
+    }
+    info!("All servers have exited completely");
+    Ok(())
+}
+
 fn connection_refused<T>(input: Result<T, IoError>) -> HarnessResult<bool> {
     match input {
         Ok(_) => Ok(false),
@@ -143,9 +165,8 @@ fn wait_for_shutdown() -> HarnessResult<()> {
             info!("Server at {connection_string} has stopped accepting connections");
         }
     }
-    info!("All servers have stopped accepting connections. Allowing {SLEEP_FOR_TERMINATION} seconds for them to exit");
-    util::sleep_sec(SLEEP_FOR_TERMINATION);
-    info!("All servers have shutdown");
+    info!("All servers have stopped accepting connections. Waiting for complete process exit");
+    wait_for_server_exit()?;
     Ok(())
 }
 
@@ -182,7 +203,6 @@ pub(super) fn run_with_servers(
 pub(super) fn kill_servers() -> HarnessResult<()> {
     info!("Terminating server instances ...");
     kill_servers_inner()?;
-    info!("Sent termination signals");
     Ok(())
 }
 

harness/src/util.rs

@@ -41,7 +41,6 @@ pub const VAR_ARTIFACT: &str = "ARTIFACT";
 #[cfg(test)]
 pub const VAR_ARTIFACT: &str = "ARTIFACT_TESTSUITE";
 pub const WORKSPACE_ROOT: &str = env!("ROOT_DIR");
-pub const SLEEP_FOR_TERMINATION: u64 = 15;
 
 pub fn get_var(var: &str) -> Option<String> {
     env::var_os(var).map(|v| v.to_string_lossy().to_string())

server/src/main.rs

@@ -116,6 +116,11 @@ fn main() {
             process::exit(1);
         }
     };
+    {
+        // remove this file in debug builds for harness to pick it up
+        #[cfg(debug_assertions)]
+        std::fs::remove_file(PID_FILE_PATH).unwrap();
+    }
     log::info!("Stopped accepting incoming connections");
     arbiter::finalize_shutdown(db, pid_file);
 }

server/src/storage/v1/tests.rs

@@ -708,6 +708,8 @@ mod storage_target_directory_structure {
             Wrapper,
         },
     };
+    use std::collections::HashSet;
+    use std::path::PathBuf;
     enum FileKind {
         Dir(&'static str),
         File(&'static str),
@@ -755,6 +757,19 @@ mod storage_target_directory_structure {
         ));
         store
     }
+    fn ensure_paths_equality(a: Vec<EntryKind>, b: Vec<EntryKind>) {
+        assert_eq!(a.len(), b.len());
+        let aset: HashSet<PathBuf> = a
+            .iter()
+            .map(|apath| PathBuf::from(apath.to_string()))
+            .collect();
+        let bset: HashSet<PathBuf> = a
+            .iter()
+            .map(|bpath| PathBuf::from(bpath.to_string()))
+            .collect();
+        aset.into_iter()
+            .for_each(|apath| assert_eq!(&apath, bset.get(&apath).unwrap()));
+    }
     use std::fs;
     #[test]
     fn local_snapshot_dir() {
@@ -766,7 +781,7 @@ mod storage_target_directory_structure {
         let target = LocalSnapshot::new("localsnap".to_owned());
         flush::flush_full(target, &store).unwrap();
         let files = os::rlistdir("data/snaps/localsnap").unwrap();
-        assert_veceq!(files, paths);
+        ensure_paths_equality(files, paths);
         fs::remove_dir_all("data/snaps/localsnap").unwrap();
     }
     #[test]
@@ -779,7 +794,7 @@ mod storage_target_directory_structure {
         let target = RemoteSnapshot::new("wisnap");
         flush::flush_full(target, &store).unwrap();
         let files = os::rlistdir("data/rsnap/wisnap").unwrap();
-        assert_veceq!(files, paths);
+        ensure_paths_equality(files, paths);
         fs::remove_dir_all("data/rsnap/wisnap").unwrap();
     }
 }

server/src/util/os.rs

@@ -25,6 +25,7 @@
 */
 
 #[cfg(unix)]
+use std::ffi::OsStr;
 pub use unix::*;
 #[cfg(windows)]
 pub use windows::*;
@@ -217,6 +218,11 @@ pub enum EntryKind {
 }
 
 impl EntryKind {
+    pub fn into_inner(self) -> String {
+        match self {
+            Self::Directory(path) | Self::File(path) => path,
+        }
+    }
     pub fn get_inner(&self) -> &str {
         match self {
             Self::Directory(rf) | Self::File(rf) => rf,
@@ -224,6 +230,24 @@ impl EntryKind {
     }
 }
 
+impl ToString for EntryKind {
+    fn to_string(&self) -> String {
+        self.get_inner().to_owned()
+    }
+}
+
+impl AsRef<str> for EntryKind {
+    fn as_ref(&self) -> &str {
+        self.get_inner()
+    }
+}
+
+impl AsRef<OsStr> for EntryKind {
+    fn as_ref(&self) -> &OsStr {
+        OsStr::new(self.get_inner())
+    }
+}
+
 /// Returns a vector with a complete list of entries (both directories and files)
 /// in the given path (recursive extraction)
 pub fn rlistdir(path: impl AsRef<Path>) -> crate::IoResult<Vec<EntryKind>> {