Remove PID file just before exit

Also upgrade GHA caching dep and fix imports that rustfmt
messed up.

.github/workflows/release.yml

@@ -41,14 +41,14 @@ jobs:
       ARTIFACT: ${{ matrix.artifact }}
     steps:
       - uses: actions/checkout@v2
-      - uses: actions/cache@v2
-        name: Restore cache
+      - name: Restore cache
+        uses: actions/cache@v3
         with:
           path: |
             ~/.cargo/registry
             ~/.cargo/git
-            target
-          key: ${{ matrix.rust }}-${{ hashFiles('Cargo.lock') }}
+            ./target
+          key: ${{ matrix.rust }}-rscache-${{ hashFiles('Cargo.lock') }}
 
       - name: Install Rust
         run: |
@@ -94,14 +94,14 @@ jobs:
       ARTIFACT: ${{ matrix.artifact }}
     steps:
       - uses: actions/checkout@v2
-      - uses: actions/cache@v2
-        name: Restore cache
+      - name: Restore cache
+        uses: actions/cache@v3
         with:
           path: |
             ~/.cargo/registry
             ~/.cargo/git
-            target
-          key: ${{ matrix.rust }}-${{ hashFiles('**/lockfiles') }}
+            ./target
+          key: ${{ matrix.rust }}-rscache-${{ hashFiles('Cargo.lock') }}
 
       - name: Install 32-bit tools (Linux)
         run: sudo apt update && sudo apt install gcc-multilib -y

.github/workflows/test-push.yml

@@ -52,27 +52,14 @@ jobs:
           chmod +x ci/setvars.sh
           ci/setvars.sh
 
-      - name: Cache Cargo registry
-        uses: actions/cache@v2
+      - name: Restore cache
+        uses: actions/cache@v3
         with:
-          path: ~/.cargo/registry
-          key: ${{ matrix.rust }}-cargo-registry-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-registry-
-      - name: Cache Cargo index
-        uses: actions/cache@v2
-        with:
-          path: ~/.cargo/git
-          key: ${{ matrix.rust }}-cargo-index-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-index-
-      - name: Cache Cargo build
-        uses: actions/cache@v2
-        with:
-          path: target
-          key: ${{ matrix.rust }}-target-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-target-
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ./target
+          key: ${{ matrix.rust }}-rscache-${{ hashFiles('Cargo.lock') }}
 
       - name: Install Rust
         run: |
@@ -134,29 +121,14 @@ jobs:
         with:
           install: "HTML::Entities"
 
-      - name: Cache Cargo registry
-        uses: actions/cache@v2
-        with:
-          path: ~/.cargo/registry
-          key: ${{ matrix.rust }}-cargo-registry-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-registry-
-
-      - name: Cache Cargo index
-        uses: actions/cache@v2
-        with:
-          path: ~/.cargo/git
-          key: ${{ matrix.rust }}-cargo-index-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-index-
-
-      - name: Cache Cargo build
-        uses: actions/cache@v2
+      - name: Restore cache
+        uses: actions/cache@v3
         with:
-          path: target
-          key: ${{ matrix.rust }}-target-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-target-
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ./target
+          key: ${{ matrix.rust }}-rscache-${{ hashFiles('Cargo.lock') }}
 
       - name: Install Rust
         run: |
@@ -216,29 +188,14 @@ jobs:
           chmod +x ci/setvars.sh
           ci/setvars.sh
 
-      - name: Cache Cargo registry
-        uses: actions/cache@v2
-        with:
-          path: ~/.cargo/registry
-          key: ${{ matrix.rust }}-cargo-registry-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-registry-
-
-      - name: Cache Cargo index
-        uses: actions/cache@v2
-        with:
-          path: ~/.cargo/git
-          key: ${{ matrix.rust }}-cargo-index-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-cargo-index-
-
-      - name: Cache Cargo build
-        uses: actions/cache@v2
+      - name: Restore cache
+        uses: actions/cache@v3
         with:
-          path: target
-          key: ${{ matrix.rust }}-target-${{ hashFiles('Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.rust }}-target-
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ./target
+          key: ${{ matrix.rust }}-rscache-${{ hashFiles('Cargo.lock') }}
 
       - name: Install Rust
         run: |

harness/src/main.rs

@@ -44,6 +44,8 @@ use crate::{
 use env_logger::Builder;
 use std::{env, process};
 
+const ROOT_DIR: &str = env!("ROOT_DIR");
+
 fn main() {
     Builder::new()
         .parse_filters(&env::var("SKYHARNESS_LOG").unwrap_or_else(|_| "info".to_owned()))

harness/src/test/mod.rs

@@ -57,10 +57,12 @@ pub fn run_test() -> HarnessResult<()> {
 
     let ret = run_test_inner();
     let kill_check = svc::kill_servers();
-    svc::wait_for_server_exit()?;
     if let Err(e) = kill_check {
         error!("Failed to kill servers with error: {e}");
     }
+    if let Err(e) = svc::wait_for_server_exit() {
+        error!("Servers did not terminate successfully: {e}");
+    }
 
     // clean up
     info!("Cleaning up test directories ...");

harness/src/test/svc.rs

@@ -26,7 +26,7 @@
 
 use crate::{
     util::{self},
-    HarnessError, HarnessResult,
+    HarnessError, HarnessResult, ROOT_DIR,
 };
 use skytable::Connection;
 #[cfg(windows)]
@@ -37,7 +37,6 @@ use std::{
     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");
@@ -76,10 +75,11 @@ 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() {
+        let filepath = Path::new(&path);
+        while filepath.exists() {
             if backoff > 64 {
                 return Err(HarnessError::Other(format!(
-                    "Backoff elapsed. {server_id} process did not exit. PID file still present"
+                    "Backoff elapsed. {server_id} process did not exit. PID file at {path} still present"
                 )));
             }
             info!("{server_id} process still live. Sleeping for {backoff} second(s)");

server/src/diskstore/flock.rs

@@ -31,12 +31,11 @@
 
 #![allow(dead_code)] // TODO(@ohsayan): Remove lint or remove offending methods
 
-use std::fs::File;
-use std::fs::OpenOptions;
-use std::io::Result;
-use std::io::Write;
-use std::io::{Seek, SeekFrom};
-use std::path::PathBuf;
+use std::{
+    fs::{File, OpenOptions},
+    io::{Result, Seek, SeekFrom, Write},
+    path::Path,
+};
 
 #[derive(Debug)]
 /// # File Lock
@@ -63,14 +62,14 @@ impl FileLock {
     /// Initialize a new `FileLock` by locking a file
     ///
     /// This function will create and lock a file if it doesn't exist or it
-    /// will create and lock a new file
+    /// will lock the existing file
     /// **This will immediately fail if locking fails, i.e it is non-blocking**
-    pub fn lock(filename: impl Into<PathBuf>) -> Result<Self> {
+    pub fn lock(filename: impl AsRef<Path>) -> Result<Self> {
         let file = OpenOptions::new()
             .create(true)
             .read(true)
             .write(true)
-            .open(filename.into())?;
+            .open(filename.as_ref())?;
         Self::_lock(&file)?;
         Ok(Self {
             file,

server/src/main.rs

@@ -116,13 +116,13 @@ fn main() {
             process::exit(1);
         }
     };
+    log::info!("Stopped accepting incoming connections");
+    arbiter::finalize_shutdown(db, pid_file);
     {
         // 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);
 }
 
 use self::config::ConfigurationSet;

server/src/util/os.rs

@@ -25,11 +25,12 @@
 */
 
 #[cfg(unix)]
-use std::ffi::OsStr;
 pub use unix::*;
 #[cfg(windows)]
 pub use windows::*;
 
+use std::ffi::OsStr;
+
 #[cfg(unix)]
 mod unix {
     use libc::{rlimit, RLIMIT_NOFILE};