Use separate flag for tree cleanup

CHANGELOG.md

@@ -24,15 +24,18 @@ All changes in this project will be noted in this file.
 
 ### Fixes
 
-- (skysh) Added error code output for new error codes which otherwise printed "Unknown error"
-- (skysh) Fixed handling of EOF keystroke
-- (skyd) Respect logging level for all terminal output
-- (skyd) Fixed LF check in protocol impl
-- (skyd) Fixed new instance detection (now checks if data directory is empty or not)
-- (skyd) Fixed panic resulting from corrupted metadata in `PARTMAP`
-- (skyd) Fixed invalid pipeline response when action error is propagated from a single stage
-- (skyd) Fixed bug where the preload wasn't flushed if a snapshot already flushed it before the
-  save on termination routine
+- skysh:
+  - Added error code output for new error codes which otherwise printed "Unknown error"
+  - Fixed handling of EOF keystroke
+- skyd:
+  - Respect logging level for all terminal output
+  - Fixed LF check in protocol impl
+  - Fixed new instance detection (now checks if data directory is empty or not)
+  - Fixed panic resulting from corrupted metadata in `PARTMAP`
+  - Fixed invalid pipeline response when action error is propagated from a single stage
+  - Fixed bug where the preload wasn't flushed if a snapshot already flushed it before the
+    save on termination routine
+  - Fixed bug that prevented tree cleanup from working
 
 ## Version 0.7.4
 

ci/windows/stop.ps1

@@ -12,6 +12,8 @@ function Terminate-Process() {
 $ProcessIDs=(get-process skyd).id
 $Proc1=($ProcessIDs -split '\n')[0]
 $Proc2=($ProcessIDs -split '\n')[1]
+$Proc3=($ProcessIDs -split '\n')[2]
 
 Terminate-Process -ProcessID $Proc1
 Terminate-Process -ProcessID $Proc2
+Terminate-Process -ProcessID $Proc3

server/src/corestore/memstore.rs

@@ -262,6 +262,8 @@ impl Memstore {
                         ks.remove();
                         // trip the preload switch
                         registry::get_preload_tripswitch().trip();
+                        // trip the cleanup switch
+                        registry::get_cleanup_tripswitch().trip();
                         Ok(())
                     } else if !no_tables_are_in_keyspace {
                         // not empty; may be referenced to or not referenced to
@@ -312,6 +314,8 @@ impl Memstore {
                         keyspace.remove();
                         // trip the preload switch
                         registry::get_preload_tripswitch().trip();
+                        // trip the cleanup switch
+                        registry::get_cleanup_tripswitch().trip();
                         Ok(())
                     } else {
                         Err(DdlError::StillInUse)
@@ -420,6 +424,8 @@ impl Keyspace {
             if did_remove {
                 // we need to re-init tree; so trip
                 registry::get_preload_tripswitch().trip();
+                // we need to cleanup tree; so trip
+                registry::get_cleanup_tripswitch().trip();
                 Ok(())
             } else {
                 Err(DdlError::StillInUse)

server/src/queryengine/parser.rs

@@ -204,7 +204,9 @@ impl<'a> Entity<'a> {
     fn verify_entity_name(input: &[u8]) -> Result<&[u8], &'static [u8]> {
         let valid_name = input.len() < 65
             && encoding::is_utf8(input)
-            && unsafe { VALID_CONTAINER_NAME.is_match(str::from_utf8_unchecked(input)) };
+            && unsafe { VALID_CONTAINER_NAME.is_match(str::from_utf8_unchecked(input)) }
+            && input != b"PARTMAP"
+            && input != b"PRELOAD";
         if compiler::likely(valid_name && !input.is_empty()) {
             // valid name
             Ok(input)

server/src/registry/mod.rs

@@ -79,6 +79,7 @@ static GLOBAL_STATE: AtomicBool = AtomicBool::new(true);
 static FLUSH_STATE: QuickLock<()> = QuickLock::new(());
 /// The preload trip switch
 static PRELOAD_TRIPSWITCH: Trip = Trip::new_untripped();
+static CLEANUP_TRIPSWITCH: Trip = Trip::new_untripped();
 
 /// Check the global system state
 pub fn state_okay() -> bool {
@@ -105,3 +106,8 @@ pub fn unpoison() {
 pub fn get_preload_tripswitch() -> &'static Trip {
     &PRELOAD_TRIPSWITCH
 }
+
+/// Get a static reference to the global cleanup trip switch
+pub fn get_cleanup_tripswitch() -> &'static Trip {
+    &CLEANUP_TRIPSWITCH
+}

server/src/storage/v1/interface.rs

@@ -81,10 +81,13 @@ pub fn create_tree_fresh<T: StorageTarget>(target: &T, memroot: &Memstore) -> Io
 /// **Warning**: Calling this is quite inefficient so consider calling it once or twice
 /// throughout the lifecycle of the server
 pub fn cleanup_tree(memroot: &Memstore) -> IoResult<()> {
-    if registry::get_preload_tripswitch().is_tripped() {
+    if registry::get_cleanup_tripswitch().is_tripped() {
         // only run a cleanup if someone tripped the switch
         // hashset because the fs itself will not allow duplicate entries
-        let dir_keyspaces: HashSet<String> = read_dir_to_col!(DIR_KSROOT);
+        // the keyspaces directory will contain the PRELOAD file, but we'll just
+        // remove it from the list
+        let mut dir_keyspaces: HashSet<String> = read_dir_to_col!(DIR_KSROOT);
+        dir_keyspaces.remove("PRELOAD");
         let our_keyspaces: HashSet<String> = memroot
             .keyspaces
             .iter()
@@ -92,9 +95,25 @@ pub fn cleanup_tree(memroot: &Memstore) -> IoResult<()> {
             .collect();
         // these are the folders that we need to remove; plonk the deleted keyspaces first
         for folder in dir_keyspaces.difference(&our_keyspaces) {
-            if folder != "PRELOAD" {
-                let ks_path = concat_str!(DIR_KSROOT, "/", folder);
-                fs::remove_dir_all(ks_path)?;
+            let ks_path = concat_str!(DIR_KSROOT, "/", folder);
+            fs::remove_dir_all(ks_path)?;
+        }
+        // now remove the tables
+        for keyspace in memroot.keyspaces.iter() {
+            let ks_path = unsafe { concat_str!(DIR_KSROOT, "/", keyspace.key().as_str()) };
+            let mut dir_tbls: HashSet<String> = read_dir_to_col!(&ks_path);
+            // in the list of directories we collected, remove PARTMAP because we should NOT
+            // delete it
+            dir_tbls.remove("PARTMAP");
+            let our_tbls: HashSet<String> = keyspace
+                .value()
+                .tables
+                .iter()
+                .map(|v| unsafe { v.key().as_str() }.to_owned())
+                .collect();
+            for old_file in dir_tbls.difference(&our_tbls) {
+                let fpath = concat_path!(&ks_path, old_file);
+                fs::remove_file(&fpath)?;
             }
         }
         // now plonk the data files