From e9fd265ce6d326425994ccfea9d5afc7165460db Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 20 Nov 2020 20:11:04 +0100 Subject: [PATCH] Slight refactoring to make things clearer with DeletedFilter --- src/api/s3_list.rs | 4 ++- src/garage/admin_rpc.rs | 6 ++-- src/model/block.rs | 4 +-- src/model/block_ref_table.rs | 6 ++-- src/model/bucket_table.rs | 7 +++-- src/model/key_table.rs | 6 ++-- src/model/object_table.rs | 7 +++-- src/model/version_table.rs | 6 ++-- src/table/lib.rs | 3 ++ src/table/schema.rs | 61 ++++++++++++++++-------------------- src/table/util.rs | 35 +++++++++++++++++++++ 11 files changed, 90 insertions(+), 55 deletions(-) create mode 100644 src/table/util.rs diff --git a/src/api/s3_list.rs b/src/api/s3_list.rs index f2b49a1..3b739a8 100644 --- a/src/api/s3_list.rs +++ b/src/api/s3_list.rs @@ -10,6 +10,8 @@ use garage_util::error::Error; use garage_model::garage::Garage; use garage_model::object_table::*; +use garage_table::DeletedFilter; + use crate::encoding::*; #[derive(Debug)] @@ -41,7 +43,7 @@ pub async fn handle_list( .get_range( &bucket.to_string(), Some(next_chunk_start.clone()), - Some(()), + Some(DeletedFilter::NotDeleted), max_keys + 1, ) .await?; diff --git a/src/garage/admin_rpc.rs b/src/garage/admin_rpc.rs index b29f2f7..778e4a1 100644 --- a/src/garage/admin_rpc.rs +++ b/src/garage/admin_rpc.rs @@ -67,7 +67,7 @@ impl AdminRpcHandler { let bucket_names = self .garage .bucket_table - .get_range(&EmptyKey, None, Some(()), 10000) + .get_range(&EmptyKey, None, Some(DeletedFilter::NotDeleted), 10000) .await? .iter() .map(|b| b.name.to_string()) @@ -101,7 +101,7 @@ impl AdminRpcHandler { let objects = self .garage .object_table - .get_range(&query.name, None, Some(()), 10) + .get_range(&query.name, None, Some(DeletedFilter::NotDeleted), 10) .await?; if !objects.is_empty() { return Err(Error::BadRPC(format!("Bucket {} is not empty", query.name))); @@ -170,7 +170,7 @@ impl AdminRpcHandler { let key_ids = self .garage .key_table - .get_range(&EmptyKey, None, Some(()), 10000) + .get_range(&EmptyKey, None, Some(DeletedFilter::NotDeleted), 10000) .await? .iter() .map(|k| (k.key_id.to_string(), k.name.to_string())) diff --git a/src/model/block.rs b/src/model/block.rs index 4e8bb7d..6a5d9c5 100644 --- a/src/model/block.rs +++ b/src/model/block.rs @@ -20,7 +20,7 @@ use garage_rpc::rpc_client::*; use garage_rpc::rpc_server::*; use garage_table::table_sharded::TableShardedReplication; -use garage_table::TableReplication; +use garage_table::{TableReplication, DeletedFilter}; use crate::block_ref_table::*; @@ -306,7 +306,7 @@ impl BlockManager { let garage = self.garage.load_full().unwrap(); let active_refs = garage .block_ref_table - .get_range(&hash, None, Some(()), 1) + .get_range(&hash, None, Some(DeletedFilter::NotDeleted), 1) .await?; let needed_by_others = !active_refs.is_empty(); if needed_by_others { diff --git a/src/model/block_ref_table.rs b/src/model/block_ref_table.rs index a00438c..5a7d9aa 100644 --- a/src/model/block_ref_table.rs +++ b/src/model/block_ref_table.rs @@ -47,7 +47,7 @@ impl TableSchema for BlockRefTable { type P = Hash; type S = UUID; type E = BlockRef; - type Filter = (); + type Filter = DeletedFilter; async fn updated(&self, old: Option, new: Option) -> Result<(), Error> { let block = &old.as_ref().or(new.as_ref()).unwrap().block; @@ -62,7 +62,7 @@ impl TableSchema for BlockRefTable { Ok(()) } - fn matches_filter(entry: &Self::E, _filter: &Self::Filter) -> bool { - !entry.deleted + fn matches_filter(entry: &Self::E, filter: &Self::Filter) -> bool { + filter.apply(entry.deleted) } } diff --git a/src/model/bucket_table.rs b/src/model/bucket_table.rs index 28234d8..adfb337 100644 --- a/src/model/bucket_table.rs +++ b/src/model/bucket_table.rs @@ -104,18 +104,19 @@ impl Entry for Bucket { pub struct BucketTable; + #[async_trait] impl TableSchema for BucketTable { type P = EmptyKey; type S = String; type E = Bucket; - type Filter = (); + type Filter = DeletedFilter; async fn updated(&self, _old: Option, _new: Option) -> Result<(), Error> { Ok(()) } - fn matches_filter(entry: &Self::E, _filter: &Self::Filter) -> bool { - !entry.deleted + fn matches_filter(entry: &Self::E, filter: &Self::Filter) -> bool { + filter.apply(entry.deleted) } } diff --git a/src/model/key_table.rs b/src/model/key_table.rs index 76d163b..f912b11 100644 --- a/src/model/key_table.rs +++ b/src/model/key_table.rs @@ -142,13 +142,13 @@ impl TableSchema for KeyTable { type P = EmptyKey; type S = String; type E = Key; - type Filter = (); + type Filter = DeletedFilter; async fn updated(&self, _old: Option, _new: Option) -> Result<(), Error> { Ok(()) } - fn matches_filter(entry: &Self::E, _filter: &Self::Filter) -> bool { - !entry.deleted + fn matches_filter(entry: &Self::E, filter: &Self::Filter) -> bool { + filter.apply(entry.deleted) } } diff --git a/src/model/object_table.rs b/src/model/object_table.rs index 719a222..929b63f 100644 --- a/src/model/object_table.rs +++ b/src/model/object_table.rs @@ -196,7 +196,7 @@ impl TableSchema for ObjectTable { type P = String; type S = String; type E = Object; - type Filter = (); + type Filter = DeletedFilter; async fn updated(&self, old: Option, new: Option) -> Result<(), Error> { let version_table = self.version_table.clone(); @@ -228,8 +228,9 @@ impl TableSchema for ObjectTable { Ok(()) } - fn matches_filter(entry: &Self::E, _filter: &Self::Filter) -> bool { - entry.versions.iter().any(|v| v.is_data()) + fn matches_filter(entry: &Self::E, filter: &Self::Filter) -> bool { + let deleted = !entry.versions.iter().any(|v| v.is_data()); + filter.apply(deleted) } fn try_migrate(bytes: &[u8]) -> Option { diff --git a/src/model/version_table.rs b/src/model/version_table.rs index 6054e38..0d83199 100644 --- a/src/model/version_table.rs +++ b/src/model/version_table.rs @@ -117,7 +117,7 @@ impl TableSchema for VersionTable { type P = Hash; type S = EmptyKey; type E = Version; - type Filter = (); + type Filter = DeletedFilter; async fn updated(&self, old: Option, new: Option) -> Result<(), Error> { let block_ref_table = self.block_ref_table.clone(); @@ -139,7 +139,7 @@ impl TableSchema for VersionTable { Ok(()) } - fn matches_filter(entry: &Self::E, _filter: &Self::Filter) -> bool { - !entry.deleted + fn matches_filter(entry: &Self::E, filter: &Self::Filter) -> bool { + filter.apply(entry.deleted) } } diff --git a/src/table/lib.rs b/src/table/lib.rs index ac12914..7684fe9 100644 --- a/src/table/lib.rs +++ b/src/table/lib.rs @@ -4,10 +4,13 @@ extern crate log; pub mod schema; +pub mod util; + pub mod table; pub mod table_fullcopy; pub mod table_sharded; pub mod table_sync; pub use schema::*; +pub use util::*; pub use table::*; diff --git a/src/table/schema.rs b/src/table/schema.rs index 1914320..49cede0 100644 --- a/src/table/schema.rs +++ b/src/table/schema.rs @@ -8,10 +8,36 @@ pub trait PartitionKey { fn hash(&self) -> Hash; } +impl PartitionKey for String { + fn hash(&self) -> Hash { + hash(self.as_bytes()) + } +} + +impl PartitionKey for Hash { + fn hash(&self) -> Hash { + self.clone() + } +} + + pub trait SortKey { fn sort_key(&self) -> &[u8]; } +impl SortKey for String { + fn sort_key(&self) -> &[u8] { + self.as_bytes() + } +} + +impl SortKey for Hash { + fn sort_key(&self) -> &[u8] { + self.as_slice() + } +} + + pub trait Entry: PartialEq + Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync { @@ -21,40 +47,6 @@ pub trait Entry: fn merge(&mut self, other: &Self); } -#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct EmptyKey; -impl SortKey for EmptyKey { - fn sort_key(&self) -> &[u8] { - &[] - } -} -impl PartitionKey for EmptyKey { - fn hash(&self) -> Hash { - [0u8; 32].into() - } -} - -impl PartitionKey for String { - fn hash(&self) -> Hash { - hash(self.as_bytes()) - } -} -impl SortKey for String { - fn sort_key(&self) -> &[u8] { - self.as_bytes() - } -} - -impl PartitionKey for Hash { - fn hash(&self) -> Hash { - self.clone() - } -} -impl SortKey for Hash { - fn sort_key(&self) -> &[u8] { - self.as_slice() - } -} #[async_trait] pub trait TableSchema: Send + Sync { @@ -74,3 +66,4 @@ pub trait TableSchema: Send + Sync { true } } + diff --git a/src/table/util.rs b/src/table/util.rs new file mode 100644 index 0000000..043a457 --- /dev/null +++ b/src/table/util.rs @@ -0,0 +1,35 @@ +use serde::{Deserialize, Serialize}; + +use garage_util::data::*; + +use crate::schema::*; + +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EmptyKey; +impl SortKey for EmptyKey { + fn sort_key(&self) -> &[u8] { + &[] + } +} +impl PartitionKey for EmptyKey { + fn hash(&self) -> Hash { + [0u8; 32].into() + } +} + +#[derive(Clone, Copy, Debug, Serialize, Deserialize)] +pub enum DeletedFilter { + All, + Deleted, + NotDeleted, +} + +impl DeletedFilter { + pub fn apply(&self, deleted: bool) -> bool { + match self { + DeletedFilter::All => true, + DeletedFilter::Deleted => deleted, + DeletedFilter::NotDeleted => !deleted, + } + } +}