diff --git a/CHANGELOG.md b/CHANGELOG.md index bceaf233..539994d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ All changes in this project will be noted in this file. ```sql DROP KEYSPACE ``` + - If a keyspace still has tables, it cannot be dropped by using `DROP KEYSPACE `, instead + one needs to use `DROP KEYSPACE force` to force drop the keyspace - **Multiple tables**: - Tables hold the actual data. When you connect to the server, you are connected to the `default` table in the `default` keyspace. This table is non-removable diff --git a/Cargo.lock b/Cargo.lock index bcbf6187..4667521c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,9 +60,9 @@ checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040" [[package]] name = "cc" -version = "1.0.68" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a72c244c1ff497a746a7e1fb3d14bd08420ecda70c8f25c7112f2781652d787" +checksum = "e70cc2f62c6ce1868963827bd677764c62d07c3d9a3e1fb1177ee1a9ab199eb2" [[package]] name = "cfg-if" @@ -243,9 +243,9 @@ checksum = "c34f04666d835ff5d62e058c3995147c06f42fe86ff053337632bca83e42702d" [[package]] name = "env_logger" -version = "0.8.4" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" +checksum = "0b2cf0344971ee6c64c31be0d530793fba457d322dfec2810c453d0ef228f9c3" dependencies = [ "atty", "humantime", @@ -459,9 +459,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.97" +version = "0.2.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12b8adadd720df158f4d70dfe7ccc6adb0472d7c55ca83445f6a5ab3e36f8fb6" +checksum = "320cfe77175da3a483efed4bc0adc1968ca050b098ce4f2f1c13a56626128790" [[package]] name = "libsky" @@ -704,9 +704,9 @@ checksum = "bc881b2c22681370c6a780e47af9840ef841837bc98118431d4e1868bd0c1086" [[package]] name = "proc-macro2" -version = "1.0.27" +version = "1.0.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0d8caf72986c1a598726adc988bb5984792ef84f5ee5aa50209145ee8077038" +checksum = "5c7ed8b8c7b886ea3ed7dde405212185f423ab44682667c8c6dd14aa1d9f6612" dependencies = [ "unicode-xid", ] @@ -889,9 +889,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.64" +version = "1.0.66" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "799e97dc9fdae36a5c8b8f2cae9ce2ee9fdce2058c57a93e6099d919fd982f79" +checksum = "336b10da19a12ad094b59d870ebde26a45402e5b470add4b5fd03c5048a32127" dependencies = [ "itoa", "ryu", @@ -1044,9 +1044,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" [[package]] name = "syn" -version = "1.0.73" +version = "1.0.74" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f71489ff30030d2ae598524f61326b902466f72a0fb1a8564c001cc63425bcc7" +checksum = "1873d832550d4588c3dbc20f01361ab00bfe741048f71e3fecf145a7cc18b29c" dependencies = [ "proc-macro2", "quote", @@ -1055,9 +1055,9 @@ dependencies = [ [[package]] name = "sysinfo" -version = "0.18.2" +version = "0.19.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d404aefa651a24a7f2a1190fec9fb6380ba84ac511a6fefad79eb0e63d39a97d" +checksum = "9e7de153d0438a648bb71e06e300e54fc641685e96af96d49b843f43172d341c" dependencies = [ "cfg-if", "core-foundation-sys", @@ -1100,9 +1100,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.7.1" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fb2ed024293bb19f7a5dc54fe83bf86532a44c12a2bb8ba40d64a4509395ca2" +checksum = "4b7b349f11a7047e6d1276853e612d152f5e8a352c61917887cc2169e2366b4c" dependencies = [ "autocfg", "bytes", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index de44783f..75feb108 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -11,7 +11,7 @@ edition = "2018" libsky = { path="../libsky" } skytable = { git="https://github.com/skytable/client-rust", branch="next", features=["async", "aio-sslv"], default-features=false } # external deps -tokio = { version="1.7.0", features=["full"] } +tokio = { version="1.9.0", features=["full"] } clap = { version="2.33.3", features=["yaml"] } rustyline = "8.2.0" crossterm = "0.20.0" diff --git a/server/Cargo.toml b/server/Cargo.toml index 1e4fad09..a02aa1ef 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -10,7 +10,7 @@ build = "build.rs" skytable = { git = "https://github.com/skytable/client-rust", branch = "next", default-features = false } sky_macros = { path = "../sky-macros" } # external deps -tokio = { version = "1.7.0", features = ["full"] } +tokio = { version = "1.9.0", features = ["full"] } bytes = "1.0.1" libsky = { path = "../libsky" } bincode = "1.3.3" @@ -18,12 +18,12 @@ dashmap = { version = "4.0.2", features = ["serde", "raw-api"] } serde = { version = "1.0.126", features = ["derive"] } toml = "0.5.8" clap = { version = "2.33.3", features = ["yaml"] } -env_logger = "0.8.4" +env_logger = "0.9.0" log = "0.4.14" chrono = "0.4.19" regex = "1.5.4" -tokio-openssl = "0.6" -openssl = { version = "0.10.34", features = ["vendored"] } +tokio-openssl = "0.6.2" +openssl = { version = "0.10.35", features = ["vendored"] } [target.'cfg(not(target_env = "msvc"))'.dependencies] # external deps @@ -34,7 +34,7 @@ winapi = { version = "0.3.9", features = ["fileapi"] } [target.'cfg(unix)'.build-dependencies] # external deps -cc = "1.0.68" +cc = "1.0.69" [dev-dependencies] # internal deps @@ -44,8 +44,8 @@ skytable = { git = "https://github.com/skytable/client-rust", features = [ "aio-ssl", ], default-features = false, branch = "next" } # external deps -tokio = { version = "1.6.1", features = ["test-util"] } +tokio = { version = "1.9.0", features = ["test-util"] } rand = "0.8.4" [target.'cfg(unix)'.dependencies] # external deps -libc = "0.2.96" +libc = "0.2.98" diff --git a/server/src/corestore/htable.rs b/server/src/corestore/htable.rs index 7fcf44c4..fd4a04b1 100644 --- a/server/src/corestore/htable.rs +++ b/server/src/corestore/htable.rs @@ -29,6 +29,7 @@ use bytes::Bytes; use libsky::TResult; use serde::{Deserialize, Serialize}; use std::borrow::Borrow; +use std::collections::hash_map::RandomState; use std::fmt; use std::hash::Hash; use std::iter::FromIterator; @@ -38,6 +39,7 @@ use dashmap::iter::Iter; pub use dashmap::lock::RwLock as MapRWL; pub use dashmap::lock::RwLockReadGuard as MapRWLGuard; pub use dashmap::mapref::entry::Entry as MapEntry; +pub use dashmap::mapref::entry::OccupiedEntry; pub use dashmap::mapref::one::Ref as MapSingleReference; use dashmap::mapref::one::Ref; use dashmap::DashMap; @@ -136,7 +138,14 @@ where K: Borrow, Q: Hash + Eq + ?Sized, { - self.inner.remove_if(key, exec).is_some() + self.remove_if(key, exec).is_some() + } + pub fn remove_if(&self, key: &Q, exec: impl FnOnce(&K, &V) -> bool) -> Option<(K, V)> + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { + self.inner.remove_if(key, exec) } /// Update or insert pub fn upsert(&self, k: K, v: V) { @@ -151,6 +160,13 @@ where false } } + pub fn mut_entry(&self, key: K) -> Option> { + if let MapEntry::Occupied(oe) = self.inner.entry(key) { + Some(oe) + } else { + None + } + } } impl Coremap diff --git a/server/src/corestore/memstore.rs b/server/src/corestore/memstore.rs index 644a71a9..cbfd9408 100644 --- a/server/src/corestore/memstore.rs +++ b/server/src/corestore/memstore.rs @@ -137,6 +137,8 @@ pub enum DdlError { AlreadyExists, /// The target object is not ready NotReady, + /// The target object is not empty + NotEmpty, /// The DDL transaction failed DdlTransactionFailure, } @@ -224,23 +226,77 @@ impl Memstore { self.keyspaces .true_if_insert(keyspace_identifier, Arc::new(Keyspace::empty())) } - pub fn drop_keyspace(&self, ksid: &Q) -> KeyspaceResult<()> - where - ObjectID: Borrow, - Q: Hash + Eq + PartialEq + ?Sized, - { + /// Drop a keyspace only if it is empty and has no clients connected to it + /// + /// The invariants maintained here are: + /// 1. The keyspace is not referenced to + /// 2. There are no tables in the keyspace + pub fn drop_keyspace(&self, ksid: ObjectID) -> KeyspaceResult<()> { if ksid.eq(&SYSTEM) || ksid.eq(&DEFAULT) { Err(DdlError::ProtectedObject) } else if !self.keyspaces.contains_key(&ksid) { Err(DdlError::ObjectNotFound) } else { - let good_to_remove = self.keyspaces.true_remove_if(&ksid, |_, arc| { - Arc::strong_count(arc) == 1 && arc.table_count() == 0 - }); - if good_to_remove { - Ok(()) - } else { - Err(DdlError::StillInUse) + let removed_keyspace = self.keyspaces.mut_entry(ksid); + match removed_keyspace { + Some(ks) => { + let no_one_is_using_keyspace = Arc::strong_count(ks.get()) == 1; + let no_tables_are_in_keyspace = ks.get().table_count() == 0; + if no_one_is_using_keyspace && no_tables_are_in_keyspace { + // we are free to drop this + ks.remove_entry(); + Ok(()) + } else if !no_tables_are_in_keyspace { + // not empty; may be referenced to or not referenced to + Err(DdlError::NotEmpty) + } else { + // still referenced to; but is empty + Err(DdlError::StillInUse) + } + } + None => Err(DdlError::ObjectNotFound), + } + } + } + /// Force remove a keyspace along with all its tables. This force however only + /// removes tables if they aren't in use and iff the keyspace is not currently + /// in use to avoid the problem of having "ghost tables" + /// + /// The invariants maintained here are: + /// 1. The keyspace is not referenced to + /// 2. The tables in the keyspace are not referenced to + pub fn force_drop_keyspace(&self, ksid: ObjectID) -> KeyspaceResult<()> { + if ksid.eq(&SYSTEM) || ksid.eq(&DEFAULT) { + Err(DdlError::ProtectedObject) + } else if !self.keyspaces.contains_key(&ksid) { + Err(DdlError::ObjectNotFound) + } else { + let removed_keyspace = self.keyspaces.mut_entry(ksid); + match removed_keyspace { + Some(keyspace) => { + // force remove drops tables, but only if they aren't in use + // we can potentially have "ghost" tables if we don't do this + // check. Similarly, if we don't check the strong count of the + // keyspace, we might end up with "ghost" keyspaces. A good + // question would be: why not just check the number of tables -- + // well, the force drop is specifically built to address the problem + // of having not empty keyspaces. The invariant that `drop keyspace force` + // maintains is that the keyspace or any of its objects are never + // referenced to -- only then it is safe to delete the keyspace + let no_tables_in_use = Arc::strong_count(keyspace.get()) == 1 + && keyspace + .get() + .tables + .iter() + .all(|table| Arc::strong_count(table.value()) == 1); + if no_tables_in_use { + keyspace.remove_entry(); + Ok(()) + } else { + Err(DdlError::StillInUse) + } + } + None => Err(DdlError::ObjectNotFound), } } } diff --git a/server/src/corestore/mod.rs b/server/src/corestore/mod.rs index 8541b1c8..01027112 100644 --- a/server/src/corestore/mod.rs +++ b/server/src/corestore/mod.rs @@ -54,6 +54,8 @@ pub mod lazy; pub mod lock; pub mod memstore; pub mod table; +#[cfg(test)] +mod tests; pub(super) type KeyspaceResult = Result; type OptionTuple = (Option, Option); @@ -354,14 +356,15 @@ impl Corestore { } /// Drop a keyspace - pub fn drop_keyspace(&self, ksid: &Q) -> KeyspaceResult<()> - where - ObjectID: Borrow, - Q: Hash + Eq + PartialEq + ?Sized, - { + pub fn drop_keyspace(&self, ksid: ObjectID) -> KeyspaceResult<()> { self.store.drop_keyspace(ksid) } + /// Force drop a keyspace + pub fn force_drop_keyspace(&self, ksid: ObjectID) -> KeyspaceResult<()> { + self.store.force_drop_keyspace(ksid) + } + /// Execute a query that has already been validated by `Connection::read_query` pub async fn execute_query(&mut self, query: Query, con: &mut T) -> TResult<()> where diff --git a/server/src/corestore/tests.rs b/server/src/corestore/tests.rs new file mode 100644 index 00000000..0ee670c2 --- /dev/null +++ b/server/src/corestore/tests.rs @@ -0,0 +1,121 @@ +/* + * Created on Fri Jul 30 2021 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2021, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +mod memstore_keyspace_tests { + use super::super::memstore::*; + use super::super::table::Table; + + #[test] + fn test_drop_keyspace_empty() { + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + ms.create_keyspace(obj.clone()); + assert!(ms.drop_keyspace(obj).is_ok()); + } + + #[test] + fn test_drop_keyspace_still_accessed() { + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + ms.create_keyspace(obj.clone()); + let _ks_ref = ms.get_keyspace_atomic_ref(&obj); + assert_eq!(ms.drop_keyspace(obj).unwrap_err(), DdlError::StillInUse); + } + + #[test] + fn test_drop_keyspace_not_empty() { + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + ms.create_keyspace(obj.clone()); + let ks_ref = ms.get_keyspace_atomic_ref(&obj).unwrap(); + ks_ref.create_table( + unsafe { ObjectID::from_slice("mytbl") }, + Table::new_default_kve(), + ); + assert_eq!(ms.drop_keyspace(obj).unwrap_err(), DdlError::NotEmpty); + } + + #[test] + fn test_force_drop_keyspace_empty() { + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + ms.create_keyspace(obj.clone()); + assert!(ms.force_drop_keyspace(obj).is_ok()); + } + + #[test] + fn test_force_drop_keyspace_still_accessed() { + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + ms.create_keyspace(obj.clone()); + let _ks_ref = ms.get_keyspace_atomic_ref(&obj); + assert_eq!( + ms.force_drop_keyspace(obj).unwrap_err(), + DdlError::StillInUse + ); + } + + #[test] + fn test_force_drop_keyspace_table_referenced() { + // the check here is to see if all the tables are not in active use + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + let tblid = unsafe { ObjectID::from_slice("mytbl") }; + // create the ks + ms.create_keyspace(obj.clone()); + // get an atomic ref to the keyspace + let ks_ref = ms.get_keyspace_atomic_ref(&obj).unwrap(); + // create a table + ks_ref.create_table(tblid.clone(), Table::new_default_kve()); + // ref to the table + let _tbl_ref = ks_ref.get_table_atomic_ref(&tblid).unwrap(); + // drop ks ref + drop(ks_ref); + assert_eq!( + ms.force_drop_keyspace(obj).unwrap_err(), + DdlError::StillInUse + ); + } + + #[test] + fn test_force_drop_keyspace_nonempty_okay() { + // the check here is to see if drop succeeds, provided that no + // tables are in active use + let ms = Memstore::new_empty(); + let obj = unsafe { ObjectID::from_slice("myks") }; + let tblid = unsafe { ObjectID::from_slice("mytbl") }; + // create the ks + ms.create_keyspace(obj.clone()); + // get an atomic ref to the keyspace + let ks_ref = ms.get_keyspace_atomic_ref(&obj).unwrap(); + // create a table + ks_ref.create_table(tblid, Table::new_default_kve()); + // drop ks ref + drop(ks_ref); + // should succeed because the keyspace is non-empty, but no table is referenced to + assert!(ms.force_drop_keyspace(obj).is_ok()); + } +} diff --git a/server/src/protocol/responses.rs b/server/src/protocol/responses.rs index 29e55ca2..30ad2833 100644 --- a/server/src/protocol/responses.rs +++ b/server/src/protocol/responses.rs @@ -81,6 +81,7 @@ pub mod groups { pub const BAD_CONTAINER_NAME: &[u8] = "!18\nbad-container-name\n".as_bytes(); pub const UNKNOWN_INSPECT_QUERY: &[u8] = "!21\nunknown-inspect-query\n".as_bytes(); pub const UNKNOWN_PROPERTY: &[u8] = "!16\nunknown-property\n".as_bytes(); + pub const KEYSPACE_NOT_EMPTY: &[u8] = "!18\nkeyspace-not-empty\n".as_bytes(); } pub mod full_responses { diff --git a/server/src/queryengine/ddl.rs b/server/src/queryengine/ddl.rs index bfc72f17..5325b725 100644 --- a/server/src/queryengine/ddl.rs +++ b/server/src/queryengine/ddl.rs @@ -36,6 +36,7 @@ use core::str; pub const TABLE: &[u8] = "TABLE".as_bytes(); pub const KEYSPACE: &[u8] = "KEYSPACE".as_bytes(); const VOLATILE: &[u8] = "volatile".as_bytes(); +const FORCE_REMOVE: &[u8] = "force".as_bytes(); action!( /// Handle `create table (args)` and `create keyspace ` @@ -200,13 +201,24 @@ action! { if ksid.len() > 64 { return con.write_response(responses::groups::CONTAINER_NAME_TOO_LONG).await; } + let force_remove = match act.next() { + Some(bts) if bts.eq(FORCE_REMOVE) => true, + None => false, + _ => return conwrite!(con, responses::groups::UNKNOWN_ACTION) + }; if registry::state_okay() { - let ks_slice = &ksid[..]; - let ret = match handle.drop_keyspace(ks_slice) { + let objid = unsafe {ObjectID::from_slice(ksid)}; + let result = if force_remove { + handle.force_drop_keyspace(objid) + } else { + handle.drop_keyspace(objid) + }; + let ret = match result { Ok(()) => responses::groups::OKAY, Err(DdlError::ProtectedObject) => responses::groups::PROTECTED_OBJECT, Err(DdlError::ObjectNotFound) => responses::groups::CONTAINER_NOT_FOUND, Err(DdlError::StillInUse) => responses::groups::STILL_IN_USE, + Err(DdlError::NotEmpty) => responses::groups::KEYSPACE_NOT_EMPTY, Err(_) => unsafe { // we know that Memstore::drop_table won't ever return anything else impossible!() diff --git a/sky-bench/Cargo.toml b/sky-bench/Cargo.toml index 21ac6a73..12066456 100644 --- a/sky-bench/Cargo.toml +++ b/sky-bench/Cargo.toml @@ -16,4 +16,4 @@ rand = "0.8.4" devtimer = "4.0.1" clap = { version="2.33.3", features=["yaml"] } serde = { version="1.0.126", features=["derive"] } -serde_json = "1.0.64" +serde_json = "1.0.66" diff --git a/sky-macros/Cargo.toml b/sky-macros/Cargo.toml index 9d9126ba..b43cc842 100644 --- a/sky-macros/Cargo.toml +++ b/sky-macros/Cargo.toml @@ -11,7 +11,7 @@ proc-macro = true [dependencies] # external deps -syn = {version = "1.0.73", features = ["full"]} +syn = { version = "1.0.74", features = ["full"] } quote = "1.0.9" -proc-macro2 = "1.0.27" -rand = "0.8.4" \ No newline at end of file +proc-macro2 = "1.0.28" +rand = "0.8.4" diff --git a/stress-test/Cargo.toml b/stress-test/Cargo.toml index c0e5f654..69ba4aae 100644 --- a/stress-test/Cargo.toml +++ b/stress-test/Cargo.toml @@ -8,12 +8,14 @@ edition = "2018" [dependencies] # internal deps -libstress = { path="../libstress" } -skytable = { git="https://github.com/skytable/client-rust.git", branch="next", features=["dbg"] } -sysinfo = "0.18.2" +libstress = { path = "../libstress" } +skytable = { git = "https://github.com/skytable/client-rust.git", branch = "next", features = [ + "dbg", +] } +sysinfo = "0.19.2" devtimer = "4.0.1" # external deps -env_logger = "0.8.4" +env_logger = "0.9.0" log = "0.4.14" rand = "0.8.4" crossbeam-channel = "0.5.1" diff --git a/stress-test/src/utils.rs b/stress-test/src/utils.rs index b54aa59f..5d001189 100644 --- a/stress-test/src/utils.rs +++ b/stress-test/src/utils.rs @@ -28,7 +28,7 @@ use skytable::Query; use sysinfo::{System, SystemExt}; pub fn calculate_max_keylen(expected_queries: usize, sys: &mut System) -> usize { - let total_mem_in_bytes = (sys.get_total_memory() * 1024) as usize; + let total_mem_in_bytes = (sys.total_memory() * 1024) as usize; trace!( "This host has a total memory of: {} Bytes", total_mem_in_bytes