Add `drop keyspace <name> force` (#192)

* Add forceful dropping of keyspaces

This commit also improves the reliability of `drop keyspace` in general

* Add changelog

* Add tests for `force_drop_keyspace`

* Upgrade deps
next
Sayan 3 years ago committed by GitHub
parent e32b3e8ea1
commit b6fcb4c035
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -18,6 +18,8 @@ All changes in this project will be noted in this file.
```sql
DROP KEYSPACE <name>
```
- If a keyspace still has tables, it cannot be dropped by using `DROP KEYSPACE <name>`, instead
one needs to use `DROP KEYSPACE <name> 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

32
Cargo.lock generated

@ -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",

@ -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"

@ -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"

@ -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>,
Q: Hash + Eq + ?Sized,
{
self.inner.remove_if(key, exec).is_some()
self.remove_if(key, exec).is_some()
}
pub fn remove_if<Q>(&self, key: &Q, exec: impl FnOnce(&K, &V) -> bool) -> Option<(K, V)>
where
K: Borrow<Q>,
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<OccupiedEntry<K, V, RandomState>> {
if let MapEntry::Occupied(oe) = self.inner.entry(key) {
Some(oe)
} else {
None
}
}
}
impl<K, V> Coremap<K, V>

@ -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<Q>(&self, ksid: &Q) -> KeyspaceResult<()>
where
ObjectID: Borrow<Q>,
Q: Hash + Eq + PartialEq<ObjectID> + ?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),
}
}
}

@ -54,6 +54,8 @@ pub mod lazy;
pub mod lock;
pub mod memstore;
pub mod table;
#[cfg(test)]
mod tests;
pub(super) type KeyspaceResult<T> = Result<T, DdlError>;
type OptionTuple<T> = (Option<T>, Option<T>);
@ -354,14 +356,15 @@ impl Corestore {
}
/// Drop a keyspace
pub fn drop_keyspace<Q>(&self, ksid: &Q) -> KeyspaceResult<()>
where
ObjectID: Borrow<Q>,
Q: Hash + Eq + PartialEq<ObjectID> + ?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<T, Strm>(&mut self, query: Query, con: &mut T) -> TResult<()>
where

@ -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 <ohsayan@outlook.com>
*
* 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 <https://www.gnu.org/licenses/>.
*
*/
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());
}
}

@ -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 {

@ -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 <tableid> <model>(args)` and `create keyspace <ksid>`
@ -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!()

@ -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"

@ -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"
proc-macro2 = "1.0.28"
rand = "0.8.4"

@ -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"

@ -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

Loading…
Cancel
Save