From 1e7ba58fe87dacf3d457dec36a3e35aa78868394 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Sun, 26 Jan 2020 19:55:53 +1000 Subject: [PATCH] Add concurrent structures, and initial attempt at benchmarking (#173) --- kanidmd/Cargo.toml | 7 + kanidmd/benches/kanidm_benchmark.rs | 28 ++ kanidmd/src/lib/access.rs | 163 +++++---- kanidmd/src/lib/core.rs | 4 +- kanidmd/src/lib/idm/server.rs | 16 +- kanidmd/src/lib/plugins/mod.rs | 80 +++-- kanidmd/src/lib/schema.rs | 529 +++++++++++++++------------- kanidmd/src/lib/server.rs | 6 +- 8 files changed, 480 insertions(+), 353 deletions(-) create mode 100644 kanidmd/benches/kanidm_benchmark.rs diff --git a/kanidmd/Cargo.toml b/kanidmd/Cargo.toml index 26853332d..b602cc559 100644 --- a/kanidmd/Cargo.toml +++ b/kanidmd/Cargo.toml @@ -20,6 +20,10 @@ path = "src/lib/lib.rs" name = "kanidmd" path = "src/server/main.rs" +# [[bench]] +# name = "kanidm_benchmark" +# harness = false + [dependencies] kanidm_proto = { path = "../kanidm_proto", version = "0.1" } @@ -64,3 +68,6 @@ num_cpus = "1.10" idlset = "0.1" zxcvbn = "2.0" +[dev-dependencies] +criterion = "0.3" + diff --git a/kanidmd/benches/kanidm_benchmark.rs b/kanidmd/benches/kanidm_benchmark.rs new file mode 100644 index 000000000..4c9ae6429 --- /dev/null +++ b/kanidmd/benches/kanidm_benchmark.rs @@ -0,0 +1,28 @@ +use criterion::{criterion_group, criterion_main, Criterion}; + +#[macro_use] +use kanidm; +use kanidm::constants::UUID_ADMIN; + +pub fn criterion_benchmark_search_1(c: &mut Criterion) { + // Setup + // + run_test!(|server: &QueryServer, audit: &mut AuditScope| { + let filt = filter!(f_eq("name", PartialValue::new_iutf8s("testperson"))); + let admin = server_txn + .internal_search_uuid(audit, &UUID_ADMIN) + .expect("failed"); + + let se1 = unsafe { SearchEvent::new_impersonate_entry(admin.clone(), filt.clone()) }; + + c.bench_function("search 2000", |b| { + b.iter(|| { + let r1 = server_txn.search(audit, &se1).expect("search failure"); + assert!(r1.is_empty()); + }) + }); + }) +} + +criterion_group!(benches, criterion_benchmark_search_1); +criterion_main!(benches); diff --git a/kanidmd/src/lib/access.rs b/kanidmd/src/lib/access.rs index f147bc2b0..0d4660c40 100644 --- a/kanidmd/src/lib/access.rs +++ b/kanidmd/src/lib/access.rs @@ -15,10 +15,10 @@ // requirements (also search). // -use concread::cowcell::{CowCell, CowCellReadTxn, CowCellWriteTxn}; +use concread::collections::bptree::*; use kanidm_proto::v1::Filter as ProtoFilter; use kanidm_proto::v1::OperationError; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeSet; use uuid::Uuid; use crate::audit::AuditScope; @@ -338,32 +338,19 @@ impl AccessControlProfile { // ACP transactions and management for server bits. // ========================================================================= -#[derive(Debug, Clone)] -pub struct AccessControlsInner { - // What is the correct key here? - acps_search: BTreeMap, - acps_create: BTreeMap, - acps_modify: BTreeMap, - acps_delete: BTreeMap, -} - -impl AccessControlsInner { - fn new() -> Self { - AccessControlsInner { - acps_search: BTreeMap::new(), - acps_create: BTreeMap::new(), - acps_modify: BTreeMap::new(), - acps_delete: BTreeMap::new(), - } - } -} - pub struct AccessControls { - inner: CowCell, + // inner: CowCell, + acps_search: BptreeMap, + acps_create: BptreeMap, + acps_modify: BptreeMap, + acps_delete: BptreeMap, } pub trait AccessControlsTransaction { - fn get_inner(&self) -> &AccessControlsInner; + fn get_search(&self) -> BptreeMapReadSnapshot; + fn get_create(&self) -> BptreeMapReadSnapshot; + fn get_modify(&self) -> BptreeMapReadSnapshot; + fn get_delete(&self) -> BptreeMapReadSnapshot; // Contains all the way to eval acps to entries fn search_filter_entries( @@ -385,11 +372,10 @@ pub trait AccessControlsTransaction { }; // Some useful references we'll use for the remainder of the operation - let state = self.get_inner(); + let search_state = self.get_search(); // First get the set of acps that apply to this receiver - let related_acp: Vec<&AccessControlSearch> = state - .acps_search + let related_acp: Vec<&AccessControlSearch> = search_state .iter() .filter_map(|(_, acs)| { // Now resolve the receiver filter @@ -537,11 +523,10 @@ pub trait AccessControlsTransaction { }; // Some useful references we'll use for the remainder of the operation - let state = self.get_inner(); + let search_state = self.get_search(); // Get the relevant acps for this receiver. - let related_acp: Vec<&AccessControlSearch> = state - .acps_search + let related_acp: Vec<&AccessControlSearch> = search_state .iter() .filter_map(|(_, acs)| { let f_val = acs.acp.receiver.clone(); @@ -674,7 +659,7 @@ pub trait AccessControlsTransaction { }; // Some useful references we'll use for the remainder of the operation - let state = self.get_inner(); + let modify_state = self.get_modify(); // Pre-check if the no-no purge class is present let disallow = me.modlist.iter().fold(false, |acc, m| { @@ -693,8 +678,7 @@ pub trait AccessControlsTransaction { } // Find the acps that relate to the caller. - let related_acp: Vec<&AccessControlModify> = state - .acps_modify + let related_acp: Vec<&AccessControlModify> = modify_state .iter() .filter_map(|(_, acs)| { let f_val = acs.acp.receiver.clone(); @@ -868,11 +852,10 @@ pub trait AccessControlsTransaction { }; // Some useful references we'll use for the remainder of the operation - let state = self.get_inner(); + let create_state = self.get_create(); // Find the acps that relate to the caller. - let related_acp: Vec<&AccessControlCreate> = state - .acps_create + let related_acp: Vec<&AccessControlCreate> = create_state .iter() .filter_map(|(_, acs)| { let f_val = acs.acp.receiver.clone(); @@ -1028,11 +1011,10 @@ pub trait AccessControlsTransaction { }; // Some useful references we'll use for the remainder of the operation - let state = self.get_inner(); + let delete_state = self.get_delete(); // Find the acps that relate to the caller. - let related_acp: Vec<&AccessControlDelete> = state - .acps_delete + let related_acp: Vec<&AccessControlDelete> = delete_state .iter() .filter_map(|(_, acs)| { let f_val = acs.acp.receiver.clone(); @@ -1113,14 +1095,14 @@ pub trait AccessControlsTransaction { } pub struct AccessControlsWriteTransaction<'a> { - inner: CowCellWriteTxn<'a, AccessControlsInner>, + // inner: CowCellWriteTxn<'a, AccessControlsInner>, + acps_search: BptreeMapWriteTxn<'a, Uuid, AccessControlSearch>, + acps_create: BptreeMapWriteTxn<'a, Uuid, AccessControlCreate>, + acps_modify: BptreeMapWriteTxn<'a, Uuid, AccessControlModify>, + acps_delete: BptreeMapWriteTxn<'a, Uuid, AccessControlDelete>, } impl<'a> AccessControlsWriteTransaction<'a> { - fn get_inner_mut(&mut self) -> &mut AccessControlsInner { - &mut self.inner - } - // We have a method to update each set, so that if an error // occurs we KNOW it's an error, rather than using errors as // part of the logic (IE try-parse-fail method). @@ -1128,54 +1110,78 @@ impl<'a> AccessControlsWriteTransaction<'a> { // Clear the existing tree. We don't care that we are wiping it // because we have the transactions to protect us from errors // to allow rollbacks. - let inner = self.get_inner_mut(); - inner.acps_search.clear(); + self.acps_search.clear(); for acp in acps { let uuid = acp.acp.uuid; - inner.acps_search.insert(uuid, acp); + self.acps_search.insert(uuid, acp); } + self.acps_search.compact(); Ok(()) } pub fn update_create(&mut self, acps: Vec) -> Result<(), OperationError> { - let inner = self.get_inner_mut(); - inner.acps_create.clear(); + self.acps_create.clear(); for acp in acps { let uuid = acp.acp.uuid; - inner.acps_create.insert(uuid, acp); + self.acps_create.insert(uuid, acp); } + self.acps_create.compact(); Ok(()) } pub fn update_modify(&mut self, acps: Vec) -> Result<(), OperationError> { - let inner = self.get_inner_mut(); - inner.acps_modify.clear(); + self.acps_modify.clear(); for acp in acps { let uuid = acp.acp.uuid; - inner.acps_modify.insert(uuid, acp); + self.acps_modify.insert(uuid, acp); } + self.acps_modify.compact(); Ok(()) } pub fn update_delete(&mut self, acps: Vec) -> Result<(), OperationError> { - let inner = self.get_inner_mut(); - inner.acps_delete.clear(); + self.acps_delete.clear(); for acp in acps { let uuid = acp.acp.uuid; - inner.acps_delete.insert(uuid, acp); + self.acps_delete.insert(uuid, acp); } + // We could consider compact here ... + self.acps_delete.compact(); Ok(()) } pub fn commit(self) -> Result<(), OperationError> { - self.inner.commit(); + let AccessControlsWriteTransaction { + acps_search, + acps_create, + acps_modify, + acps_delete, + } = self; + + acps_search.commit(); + acps_create.commit(); + acps_modify.commit(); + acps_delete.commit(); + Ok(()) } } impl<'a> AccessControlsTransaction for AccessControlsWriteTransaction<'a> { - fn get_inner(&self) -> &AccessControlsInner { - &self.inner + fn get_search(&self) -> BptreeMapReadSnapshot { + self.acps_search.to_snapshot() + } + + fn get_create(&self) -> BptreeMapReadSnapshot { + self.acps_create.to_snapshot() + } + + fn get_modify(&self) -> BptreeMapReadSnapshot { + self.acps_modify.to_snapshot() + } + + fn get_delete(&self) -> BptreeMapReadSnapshot { + self.acps_delete.to_snapshot() } } @@ -1184,12 +1190,27 @@ impl<'a> AccessControlsTransaction for AccessControlsWriteTransaction<'a> { // ========================================================================= pub struct AccessControlsReadTransaction { - inner: CowCellReadTxn, + acps_search: BptreeMapReadTxn, + acps_create: BptreeMapReadTxn, + acps_modify: BptreeMapReadTxn, + acps_delete: BptreeMapReadTxn, } impl AccessControlsTransaction for AccessControlsReadTransaction { - fn get_inner(&self) -> &AccessControlsInner { - &self.inner + fn get_search(&self) -> BptreeMapReadSnapshot { + self.acps_search.to_snapshot() + } + + fn get_create(&self) -> BptreeMapReadSnapshot { + self.acps_create.to_snapshot() + } + + fn get_modify(&self) -> BptreeMapReadSnapshot { + self.acps_modify.to_snapshot() + } + + fn get_delete(&self) -> BptreeMapReadSnapshot { + self.acps_delete.to_snapshot() } } @@ -1200,19 +1221,31 @@ impl AccessControlsTransaction for AccessControlsReadTransaction { impl AccessControls { pub fn new() -> Self { AccessControls { - inner: CowCell::new(AccessControlsInner::new()), + // inner: CowCell::new(AccessControlsInner::new()), + acps_search: BptreeMap::new(), + acps_create: BptreeMap::new(), + acps_modify: BptreeMap::new(), + acps_delete: BptreeMap::new(), } } pub fn read(&self) -> AccessControlsReadTransaction { AccessControlsReadTransaction { - inner: self.inner.read(), + // inner: self.inner.read(), + acps_search: self.acps_search.read(), + acps_create: self.acps_create.read(), + acps_modify: self.acps_modify.read(), + acps_delete: self.acps_delete.read(), } } pub fn write(&self) -> AccessControlsWriteTransaction { AccessControlsWriteTransaction { - inner: self.inner.write(), + // inner: self.inner.write(), + acps_search: self.acps_search.write(), + acps_create: self.acps_create.write(), + acps_modify: self.acps_modify.write(), + acps_delete: self.acps_delete.write(), } } } diff --git a/kanidmd/src/lib/core.rs b/kanidmd/src/lib/core.rs index 262e9ec11..7b94c2700 100644 --- a/kanidmd/src/lib/core.rs +++ b/kanidmd/src/lib/core.rs @@ -1202,7 +1202,7 @@ pub fn restore_server_core(config: Configuration, dst_path: &str) { }; // Limit the scope of the schema txn. - let idxmeta = { schema.write().get_idxmeta() }; + let idxmeta = { schema.write().get_idxmeta_set() }; let mut be_wr_txn = be.write(idxmeta); let r = be_wr_txn @@ -1266,7 +1266,7 @@ pub fn reindex_server_core(config: Configuration) { info!("Start Index Phase 1 ..."); // Limit the scope of the schema txn. - let idxmeta = { schema.write().get_idxmeta() }; + let idxmeta = { schema.write().get_idxmeta_set() }; // Reindex only the core schema attributes to bootstrap the process. let be_wr_txn = be.write(idxmeta); diff --git a/kanidmd/src/lib/idm/server.rs b/kanidmd/src/lib/idm/server.rs index 8d99dd7bd..4a6c35ea2 100644 --- a/kanidmd/src/lib/idm/server.rs +++ b/kanidmd/src/lib/idm/server.rs @@ -17,8 +17,7 @@ use kanidm_proto::v1::AuthState; use kanidm_proto::v1::OperationError; use kanidm_proto::v1::RadiusAuthToken; -use concread::cowcell::{CowCell, CowCellWriteTxn}; -use std::collections::BTreeMap; +use concread::collections::bptree::*; use std::time::Duration; use uuid::Uuid; use zxcvbn; @@ -28,7 +27,7 @@ pub struct IdmServer { // means that limits to sessions can be easily applied and checked to // variaous accounts, and we have a good idea of how to structure the // in memory caches related to locking. - sessions: CowCell>, + sessions: BptreeMap, // Need a reference to the query server. qs: QueryServer, // thread/server id @@ -39,7 +38,7 @@ pub struct IdmServerWriteTransaction<'a> { // Contains methods that require writes, but in the context of writing to // the idm in memory structures (maybe the query server too). This is // things like authentication - sessions: CowCellWriteTxn<'a, BTreeMap>, + sessions: BptreeMapWriteTxn<'a, Uuid, AuthSession>, qs: &'a QueryServer, sid: &'a SID, } @@ -60,7 +59,7 @@ impl IdmServer { // TODO #59: Make number of authsessions configurable!!! pub fn new(qs: QueryServer, sid: SID) -> IdmServer { IdmServer { - sessions: CowCell::new(BTreeMap::new()), + sessions: BptreeMap::new(), qs, sid, } @@ -97,9 +96,8 @@ impl<'a> IdmServerWriteTransaction<'a> { // ct is current time - sub the timeout. and then split. let expire = ct - Duration::from_secs(AUTH_SESSION_TIMEOUT); let split_at = uuid_from_duration(expire, *self.sid); - let valid = self.sessions.split_off(&split_at); - // swap them? - *self.sessions = valid; + // Removes older sessions in place. + self.sessions.split_off_lt(&split_at); // expired will now be dropped, and can't be used by future sessions. } @@ -189,7 +187,7 @@ impl<'a> IdmServerWriteTransaction<'a> { // Do we have a session? let auth_session = try_audit!( au, - (*self.sessions) + self.sessions // Why is the session missing? .get_mut(&creds.sessionid) .ok_or(OperationError::InvalidSessionState) diff --git a/kanidmd/src/lib/plugins/mod.rs b/kanidmd/src/lib/plugins/mod.rs index 0fc9b458b..50c7b1661 100644 --- a/kanidmd/src/lib/plugins/mod.rs +++ b/kanidmd/src/lib/plugins/mod.rs @@ -300,9 +300,13 @@ impl Plugins { cand: &[Entry], ce: &CreateEvent, ) -> Result<(), OperationError> { - audit_segment!(au, || { - run_pre_create_plugin!(au, qs, cand, ce, protected::Protected) - }) + audit_segment!(au, || run_pre_create_plugin!( + au, + qs, + cand, + ce, + protected::Protected + )) } pub fn run_post_create( @@ -311,10 +315,20 @@ impl Plugins { cand: &[Entry], ce: &CreateEvent, ) -> Result<(), OperationError> { - audit_segment!(au, || { - run_post_create_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity) - .and_then(|_| run_post_create_plugin!(au, qs, cand, ce, memberof::MemberOf)) - }) + audit_segment!(au, || run_post_create_plugin!( + au, + qs, + cand, + ce, + refint::ReferentialIntegrity + ) + .and_then(|_| run_post_create_plugin!( + au, + qs, + cand, + ce, + memberof::MemberOf + ))) } pub fn run_pre_modify( @@ -340,13 +354,23 @@ impl Plugins { cand: &[Entry], me: &ModifyEvent, ) -> Result<(), OperationError> { - audit_segment!(au, || { - run_post_modify_plugin!(au, qs, pre_cand, cand, me, refint::ReferentialIntegrity) - .and_then(|_| { - run_post_modify_plugin!(au, qs, pre_cand, cand, me, memberof::MemberOf) - }) - .and_then(|_| run_post_modify_plugin!(au, qs, pre_cand, cand, me, spn::Spn)) - }) + audit_segment!(au, || run_post_modify_plugin!( + au, + qs, + pre_cand, + cand, + me, + refint::ReferentialIntegrity + ) + .and_then(|_| run_post_modify_plugin!(au, qs, pre_cand, cand, me, memberof::MemberOf)) + .and_then(|_| run_post_modify_plugin!( + au, + qs, + pre_cand, + cand, + me, + spn::Spn + ))) } pub fn run_pre_delete( @@ -355,9 +379,13 @@ impl Plugins { cand: &mut Vec>, de: &DeleteEvent, ) -> Result<(), OperationError> { - audit_segment!(au, || { - run_pre_delete_plugin!(au, qs, cand, de, protected::Protected) - }) + audit_segment!(au, || run_pre_delete_plugin!( + au, + qs, + cand, + de, + protected::Protected + )) } pub fn run_post_delete( @@ -366,10 +394,20 @@ impl Plugins { cand: &[Entry], de: &DeleteEvent, ) -> Result<(), OperationError> { - audit_segment!(au, || { - run_post_delete_plugin!(au, qs, cand, de, refint::ReferentialIntegrity) - .and_then(|_| run_post_delete_plugin!(au, qs, cand, de, memberof::MemberOf)) - }) + audit_segment!(au, || run_post_delete_plugin!( + au, + qs, + cand, + de, + refint::ReferentialIntegrity + ) + .and_then(|_| run_post_delete_plugin!( + au, + qs, + cand, + de, + memberof::MemberOf + ))) } pub fn run_verify( diff --git a/kanidmd/src/lib/schema.rs b/kanidmd/src/lib/schema.rs index 1075b7047..2bff958f9 100644 --- a/kanidmd/src/lib/schema.rs +++ b/kanidmd/src/lib/schema.rs @@ -5,11 +5,11 @@ use crate::value::{IndexType, PartialValue, SyntaxType, Value}; use kanidm_proto::v1::{ConsistencyError, OperationError, SchemaError}; use std::borrow::Borrow; -use std::collections::BTreeSet; use std::collections::HashMap; +use std::collections::{BTreeMap, BTreeSet}; use uuid::Uuid; -use concread::cowcell::{CowCell, CowCellReadTxn, CowCellWriteTxn}; +use concread::collections::bptree::*; // representations of schema that confines object types, classes // and attributes. This ties in deeply with "Entry". @@ -23,6 +23,24 @@ lazy_static! { static ref PVCLASS_CLASSTYPE: PartialValue = PartialValue::new_class("classtype"); } +pub struct Schema { + classes: BptreeMap, + attributes: BptreeMap, + idxmeta: BptreeMap, +} + +pub struct SchemaWriteTransaction<'a> { + classes: BptreeMapWriteTxn<'a, String, SchemaClass>, + attributes: BptreeMapWriteTxn<'a, String, SchemaAttribute>, + idxmeta: BptreeMapWriteTxn<'a, String, IndexType>, +} + +pub struct SchemaReadTransaction { + classes: BptreeMapReadTxn, + attributes: BptreeMapReadTxn, + idxmeta: BptreeMapReadTxn, +} + #[derive(Debug, Clone)] pub struct SchemaAttribute { // Is this ... used? @@ -466,14 +484,91 @@ pub struct SchemaInner { } pub trait SchemaTransaction { - fn get_inner(&self) -> &SchemaInner; + fn get_classes(&self) -> BptreeMapReadSnapshot; + fn get_attributes(&self) -> BptreeMapReadSnapshot; + fn get_idxmeta(&self) -> BptreeMapReadSnapshot; - fn validate(&self, audit: &mut AuditScope) -> Vec> { - self.get_inner().validate(audit) + fn validate(&self, _audit: &mut AuditScope) -> Vec> { + let mut res = Vec::new(); + + let class_snapshot = self.get_classes(); + let attribute_snapshot = self.get_attributes(); + // Does this need to validate anything further at all? The UUID + // will be checked as part of the schema migration on startup, so I think + // just that all the content is sane is fine. + for class in class_snapshot.values() { + // report the class we are checking + for a in &class.systemmay { + // report the attribute. + /* + audit_log!( + audit, + "validate systemmay class:attr -> {}:{}", + class.name, + a + ); + */ + if !attribute_snapshot.contains_key(a) { + res.push(Err(ConsistencyError::SchemaClassMissingAttribute( + class.name.clone(), + a.clone(), + ))) + } + } + for a in &class.may { + // report the attribute. + /* + audit_log!(audit, "validate may class:attr -> {}:{}", class.name, a); + */ + if !attribute_snapshot.contains_key(a) { + res.push(Err(ConsistencyError::SchemaClassMissingAttribute( + class.name.clone(), + a.clone(), + ))) + } + } + for a in &class.systemmust { + // report the attribute. + /* + audit_log!( + audit, + "validate systemmust class:attr -> {}:{}", + class.name, + a + ); + */ + if !attribute_snapshot.contains_key(a) { + res.push(Err(ConsistencyError::SchemaClassMissingAttribute( + class.name.clone(), + a.clone(), + ))) + } + } + for a in &class.must { + // report the attribute. + /* + audit_log!(audit, "validate must class:attr -> {}:{}", class.name, a); + */ + if !attribute_snapshot.contains_key(a) { + res.push(Err(ConsistencyError::SchemaClassMissingAttribute( + class.name.clone(), + a.clone(), + ))) + } + } + } + + res } fn is_multivalue(&self, attr: &str) -> Result { - self.get_inner().is_multivalue(attr) + match self.get_attributes().get(attr) { + Some(a_schema) => Ok(a_schema.multivalue), + None => { + debug!("Attribute does not exist?!"); + Err(SchemaError::InvalidAttribute) + } + } } fn normalise_attr_name(&self, an: &str) -> String { @@ -500,45 +595,113 @@ pub trait SchemaTransaction { // Probably need something like get_classes or similar // so that externals can call and use this data. - fn get_classes(&self) -> &HashMap { - &self.get_inner().classes - } - - fn get_attributes(&self) -> &HashMap { - &self.get_inner().attributes - } - - fn get_reference_types(&self) -> HashMap<&String, &SchemaAttribute> { - self.get_attributes() + fn get_reference_types(&self) -> BTreeMap { + let snapshot = self.get_attributes(); + snapshot .iter() .filter(|(_, sa)| match &sa.syntax { SyntaxType::REFERENCE_UUID => true, _ => false, }) + .map(|(k, v)| (k.clone(), v.clone())) .collect() } - fn get_idxmeta(&self) -> BTreeSet<(String, IndexType)> { - // TODO: We could cache this in the schema and recalc on reload instead to avoid - // so much cloning? - // for each attribute, if indexed, yield and flatten the attr + type. - self.get_inner().idxmeta.clone() + fn get_idxmeta_set(&self) -> BTreeSet<(String, IndexType)> { + self.get_idxmeta() + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect() } } -impl SchemaInner { - pub fn new(audit: &mut AuditScope) -> Result { - let mut au = AuditScope::new("schema_new"); +impl SchemaInner {} + +impl<'a> SchemaWriteTransaction<'a> { + // Schema probably needs to be part of the backend, so that commits are wholly atomic + // but in the current design, we need to open be first, then schema, but we have to commit be + // first, then schema to ensure that the be content matches our schema. Saying this, if your + // schema commit fails we need to roll back still .... How great are transactions. + // At the least, this is what validation is for! + pub fn commit(self) -> Result<(), OperationError> { + let SchemaWriteTransaction { + classes, + attributes, + idxmeta, + } = self; + + classes.commit(); + attributes.commit(); + idxmeta.commit(); + Ok(()) + } + + pub fn update_attributes( + &mut self, + attributetypes: Vec, + ) -> Result<(), OperationError> { + // purge all old attributes. + self.attributes.clear(); + // Update with new ones. + // Do we need to check for dups? + // No, they'll over-write each other ... but we do need name uniqueness. + attributetypes.into_iter().for_each(|a| { + self.attributes.insert(a.name.clone(), a); + }); + // Now update the idxmeta + self.reload_idxmeta(); + + Ok(()) + } + + pub fn update_classes( + &mut self, + attributetypes: Vec, + ) -> Result<(), OperationError> { + // purge all old attributes. + self.classes.clear(); + // Update with new ones. + // Do we need to check for dups? + // No, they'll over-write each other ... but we do need name uniqueness. + attributetypes.into_iter().for_each(|a| { + self.classes.insert(a.name.clone(), a); + }); + Ok(()) + } + + fn reload_idxmeta(&mut self) { + self.idxmeta.clear(); + self.idxmeta.extend(self.attributes.values().flat_map(|a| { + a.index + .iter() + .map(move |itype: &IndexType| (a.name.clone(), (*itype).clone())) + })); + } + + pub fn to_entries(&self) -> Vec> { + let r: Vec<_> = self + .attributes + .values() + .map(Entry::::from) + .chain( + self.classes + .values() + .map(Entry::::from), + ) + .collect(); + r + } + + pub fn generate_in_memory(&mut self, audit: &mut AuditScope) -> Result<(), OperationError> { + let mut au = AuditScope::new("generate_in_memory"); let r = audit_segment!(au, || { // - let mut s = SchemaInner { - classes: HashMap::new(), - attributes: HashMap::new(), - idxmeta: BTreeSet::new(), - }; + self.classes.clear(); + self.attributes.clear(); + self.idxmeta.clear(); // Bootstrap in definitions of our own schema types // First, add all the needed core attributes for schema parsing - s.attributes.insert( + self.attributes.insert( String::from("class"), SchemaAttribute { name: String::from("class"), @@ -551,7 +714,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("uuid"), SchemaAttribute { name: String::from("uuid"), @@ -566,7 +729,7 @@ impl SchemaInner { syntax: SyntaxType::UUID, }, ); - s.attributes.insert( + self.attributes.insert( String::from("name"), SchemaAttribute { name: String::from("name"), @@ -579,7 +742,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("spn"), SchemaAttribute { name: String::from("spn"), @@ -594,7 +757,7 @@ impl SchemaInner { syntax: SyntaxType::SERVICE_PRINCIPLE_NAME, }, ); - s.attributes.insert( + self.attributes.insert( String::from("attributename"), SchemaAttribute { name: String::from("attributename"), @@ -607,7 +770,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("classname"), SchemaAttribute { name: String::from("classname"), @@ -620,7 +783,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("description"), SchemaAttribute { name: String::from("description"), @@ -633,7 +796,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING, }, ); - s.attributes.insert(String::from("multivalue"), SchemaAttribute { + self.attributes.insert(String::from("multivalue"), SchemaAttribute { name: String::from("multivalue"), uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_MULTIVALUE).expect("unable to parse static uuid"), description: String::from("If true, this attribute is able to store multiple values rather than just a single value."), @@ -642,7 +805,7 @@ impl SchemaInner { index: vec![], syntax: SyntaxType::BOOLEAN, }); - s.attributes.insert(String::from("unique"), SchemaAttribute { + self.attributes.insert(String::from("unique"), SchemaAttribute { name: String::from("unique"), uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_UNIQUE).expect("unable to parse static uuid"), description: String::from("If true, this attribute must store a unique value through out the database."), @@ -651,7 +814,7 @@ impl SchemaInner { index: vec![], syntax: SyntaxType::BOOLEAN, }); - s.attributes.insert( + self.attributes.insert( String::from("index"), SchemaAttribute { name: String::from("index"), @@ -666,7 +829,7 @@ impl SchemaInner { syntax: SyntaxType::INDEX_ID, }, ); - s.attributes.insert( + self.attributes.insert( String::from("syntax"), SchemaAttribute { name: String::from("syntax"), @@ -681,7 +844,7 @@ impl SchemaInner { syntax: SyntaxType::SYNTAX_ID, }, ); - s.attributes.insert( + self.attributes.insert( String::from("systemmay"), SchemaAttribute { name: String::from("systemmay"), @@ -696,7 +859,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("may"), SchemaAttribute { name: String::from("may"), @@ -711,7 +874,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("systemmust"), SchemaAttribute { name: String::from("systemmust"), @@ -726,7 +889,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("must"), SchemaAttribute { name: String::from("must"), @@ -743,7 +906,7 @@ impl SchemaInner { ); // SYSINFO attrs // ACP attributes. - s.attributes.insert( + self.attributes.insert( String::from("acp_enable"), SchemaAttribute { name: String::from("acp_enable"), @@ -757,7 +920,7 @@ impl SchemaInner { }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_receiver"), SchemaAttribute { name: String::from("acp_receiver"), @@ -772,7 +935,7 @@ impl SchemaInner { syntax: SyntaxType::JSON_FILTER, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_targetscope"), SchemaAttribute { name: String::from("acp_targetscope"), @@ -787,7 +950,7 @@ impl SchemaInner { syntax: SyntaxType::JSON_FILTER, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_search_attr"), SchemaAttribute { name: String::from("acp_search_attr"), @@ -800,7 +963,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_create_class"), SchemaAttribute { name: String::from("acp_create_class"), @@ -815,7 +978,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_create_attr"), SchemaAttribute { name: String::from("acp_create_attr"), @@ -831,7 +994,7 @@ impl SchemaInner { }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_modify_removedattr"), SchemaAttribute { name: String::from("acp_modify_removedattr"), @@ -844,7 +1007,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_modify_presentattr"), SchemaAttribute { name: String::from("acp_modify_presentattr"), @@ -857,7 +1020,7 @@ impl SchemaInner { syntax: SyntaxType::UTF8STRING_INSENSITIVE, }, ); - s.attributes.insert( + self.attributes.insert( String::from("acp_modify_class"), SchemaAttribute { name: String::from("acp_modify_class"), @@ -871,7 +1034,7 @@ impl SchemaInner { }, ); // MO/Member - s.attributes.insert( + self.attributes.insert( String::from("memberof"), SchemaAttribute { name: String::from("memberof"), @@ -884,7 +1047,7 @@ impl SchemaInner { syntax: SyntaxType::REFERENCE_UUID, }, ); - s.attributes.insert( + self.attributes.insert( String::from("directmemberof"), SchemaAttribute { name: String::from("directmemberof"), @@ -897,7 +1060,7 @@ impl SchemaInner { syntax: SyntaxType::REFERENCE_UUID, }, ); - s.attributes.insert( + self.attributes.insert( String::from("member"), SchemaAttribute { name: String::from("member"), @@ -911,7 +1074,7 @@ impl SchemaInner { }, ); // Migration related - s.attributes.insert( + self.attributes.insert( String::from("version"), SchemaAttribute { name: String::from("version"), @@ -927,7 +1090,7 @@ impl SchemaInner { }, ); // Domain for sysinfo - s.attributes.insert( + self.attributes.insert( String::from("domain"), SchemaAttribute { name: String::from("domain"), @@ -941,7 +1104,7 @@ impl SchemaInner { }, ); - s.classes.insert( + self.classes.insert( String::from("attributetype"), SchemaClass { name: String::from("attributetype"), @@ -961,7 +1124,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("classtype"), SchemaClass { name: String::from("classtype"), @@ -983,7 +1146,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("object"), SchemaClass { name: String::from("object"), @@ -998,7 +1161,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("memberof"), SchemaClass { name: String::from("memberof"), @@ -1014,7 +1177,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("extensibleobject"), SchemaClass { name: String::from("extensibleobject"), @@ -1030,7 +1193,7 @@ impl SchemaInner { }, ); /* These two classes are core to the entry lifecycle for recycling and tombstoning */ - s.classes.insert( + self.classes.insert( String::from("recycled"), SchemaClass { name: String::from("recycled"), @@ -1042,7 +1205,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("tombstone"), SchemaClass { name: String::from("tombstone"), @@ -1058,7 +1221,7 @@ impl SchemaInner { }, ); // sysinfo - s.classes.insert( + self.classes.insert( String::from("system_info"), SchemaClass { name: String::from("system_info"), @@ -1077,7 +1240,7 @@ impl SchemaInner { }, ); // ACP - s.classes.insert( + self.classes.insert( String::from("access_control_profile"), SchemaClass { name: String::from("access_control_profile"), @@ -1090,7 +1253,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("access_control_search"), SchemaClass { name: String::from("access_control_search"), @@ -1103,7 +1266,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("access_control_delete"), SchemaClass { name: String::from("access_control_delete"), @@ -1116,7 +1279,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("access_control_modify"), SchemaClass { name: String::from("access_control_modify"), @@ -1133,7 +1296,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("access_control_create"), SchemaClass { name: String::from("access_control_create"), @@ -1149,7 +1312,7 @@ impl SchemaInner { must: vec![], }, ); - s.classes.insert( + self.classes.insert( String::from("system"), SchemaClass { name: String::from("system"), @@ -1163,10 +1326,10 @@ impl SchemaInner { }, ); - let r = s.validate(&mut au); + let r = self.validate(&mut au); if r.is_empty() { - s.reload_idxmeta(); - Ok(s) + self.reload_idxmeta(); + Ok(()) } else { Err(OperationError::ConsistencyError(r)) } @@ -1175,205 +1338,65 @@ impl SchemaInner { audit.append_scope(au); r } - - fn reload_idxmeta(&mut self) { - let mut idxmeta_new: BTreeSet<_> = self - .attributes - .values() - .flat_map(|a| { - a.index - .iter() - .map(move |itype: &IndexType| (a.name.clone(), (*itype).clone())) - }) - .collect(); - - std::mem::swap(&mut self.idxmeta, &mut idxmeta_new); - } - - pub fn validate(&self, _audit: &mut AuditScope) -> Vec> { - let mut res = Vec::new(); - // Does this need to validate anything further at all? The UUID - // will be checked as part of the schema migration on startup, so I think - // just that all the content is sane is fine. - for class in self.classes.values() { - // report the class we are checking - for a in &class.systemmay { - // report the attribute. - /* - audit_log!( - audit, - "validate systemmay class:attr -> {}:{}", - class.name, - a - ); - */ - if !self.attributes.contains_key(a) { - res.push(Err(ConsistencyError::SchemaClassMissingAttribute( - class.name.clone(), - a.clone(), - ))) - } - } - for a in &class.may { - // report the attribute. - /* - audit_log!(audit, "validate may class:attr -> {}:{}", class.name, a); - */ - if !self.attributes.contains_key(a) { - res.push(Err(ConsistencyError::SchemaClassMissingAttribute( - class.name.clone(), - a.clone(), - ))) - } - } - for a in &class.systemmust { - // report the attribute. - /* - audit_log!( - audit, - "validate systemmust class:attr -> {}:{}", - class.name, - a - ); - */ - if !self.attributes.contains_key(a) { - res.push(Err(ConsistencyError::SchemaClassMissingAttribute( - class.name.clone(), - a.clone(), - ))) - } - } - for a in &class.must { - // report the attribute. - /* - audit_log!(audit, "validate must class:attr -> {}:{}", class.name, a); - */ - if !self.attributes.contains_key(a) { - res.push(Err(ConsistencyError::SchemaClassMissingAttribute( - class.name.clone(), - a.clone(), - ))) - } - } - } - - res - } - - fn is_multivalue(&self, attr_name: &str) -> Result { - match self.attributes.get(attr_name) { - Some(a_schema) => Ok(a_schema.multivalue), - None => { - debug!("Attribute does not exist?!"); - Err(SchemaError::InvalidAttribute) - } - } - } -} - -pub struct Schema { - inner: CowCell, -} - -pub struct SchemaWriteTransaction<'a> { - inner: CowCellWriteTxn<'a, SchemaInner>, -} - -impl<'a> SchemaWriteTransaction<'a> { - // Schema probably needs to be part of the backend, so that commits are wholly atomic - // but in the current design, we need to open be first, then schema, but we have to commit be - // first, then schema to ensure that the be content matches our schema. Saying this, if your - // schema commit fails we need to roll back still .... How great are transactions. - // At the least, this is what validation is for! - pub fn commit(self) -> Result<(), OperationError> { - self.inner.commit(); - Ok(()) - } - - pub fn update_attributes( - &mut self, - attributetypes: Vec, - ) -> Result<(), OperationError> { - // purge all old attributes. - self.inner.attributes.clear(); - // Update with new ones. - // Do we need to check for dups? - // No, they'll over-write each other ... but we do need name uniqueness. - attributetypes.into_iter().for_each(|a| { - self.inner.attributes.insert(a.name.clone(), a); - }); - // Now update the idxmeta - self.inner.reload_idxmeta(); - - Ok(()) - } - - pub fn update_classes( - &mut self, - attributetypes: Vec, - ) -> Result<(), OperationError> { - // purge all old attributes. - self.inner.classes.clear(); - // Update with new ones. - // Do we need to check for dups? - // No, they'll over-write each other ... but we do need name uniqueness. - attributetypes.into_iter().for_each(|a| { - self.inner.classes.insert(a.name.clone(), a); - }); - Ok(()) - } - - pub fn to_entries(&self) -> Vec> { - let r: Vec<_> = self - .inner - .attributes - .values() - .map(Entry::::from) - .chain( - self.inner - .classes - .values() - .map(Entry::::from), - ) - .collect(); - r - } } impl<'a> SchemaTransaction for SchemaWriteTransaction<'a> { - fn get_inner(&self) -> &SchemaInner { - // Does this deref the CowCell for us? - &self.inner + fn get_classes(&self) -> BptreeMapReadSnapshot { + self.classes.to_snapshot() + } + + fn get_attributes(&self) -> BptreeMapReadSnapshot { + self.attributes.to_snapshot() + } + + fn get_idxmeta(&self) -> BptreeMapReadSnapshot { + self.idxmeta.to_snapshot() } } -pub struct SchemaReadTransaction { - inner: CowCellReadTxn, -} - impl SchemaTransaction for SchemaReadTransaction { - fn get_inner(&self) -> &SchemaInner { - // Does this deref the CowCell for us? - &self.inner + fn get_classes(&self) -> BptreeMapReadSnapshot { + self.classes.to_snapshot() + } + + fn get_attributes(&self) -> BptreeMapReadSnapshot { + self.attributes.to_snapshot() + } + + fn get_idxmeta(&self) -> BptreeMapReadSnapshot { + self.idxmeta.to_snapshot() } } impl Schema { pub fn new(audit: &mut AuditScope) -> Result { - SchemaInner::new(audit).map(|si| Schema { - inner: CowCell::new(si), - }) + let s = Schema { + classes: BptreeMap::new(), + attributes: BptreeMap::new(), + idxmeta: BptreeMap::new(), + }; + let mut sw = s.write(); + let r1 = sw.generate_in_memory(audit); + debug_assert!(r1.is_ok()); + r1?; + let r2 = sw.commit().map(|_| s); + debug_assert!(r2.is_ok()); + r2 } pub fn read(&self) -> SchemaReadTransaction { SchemaReadTransaction { - inner: self.inner.read(), + classes: self.classes.read(), + attributes: self.attributes.read(), + idxmeta: self.idxmeta.read(), } } pub fn write(&self) -> SchemaWriteTransaction { SchemaWriteTransaction { - inner: self.inner.write(), + classes: self.classes.write(), + attributes: self.attributes.write(), + idxmeta: self.idxmeta.write(), } } } diff --git a/kanidmd/src/lib/server.rs b/kanidmd/src/lib/server.rs index 5cfbe156b..9d2594adf 100644 --- a/kanidmd/src/lib/server.rs +++ b/kanidmd/src/lib/server.rs @@ -95,7 +95,7 @@ pub trait QueryServerTransaction { // NOTE: Filters are validated in event conversion. let schema = self.get_schema(); - let idxmeta = schema.get_idxmeta(); + let idxmeta = schema.get_idxmeta_set(); // Now resolve all references and indexes. let vfr = try_audit!(au, se.filter.resolve(&se.event, Some(&idxmeta))); @@ -132,7 +132,7 @@ pub trait QueryServerTransaction { let mut audit_be = AuditScope::new("backend_exists"); let schema = self.get_schema(); - let idxmeta = schema.get_idxmeta(); + let idxmeta = schema.get_idxmeta_set(); let vfr = try_audit!(au, ee.filter.resolve(&ee.event, Some(&idxmeta))); let res = self @@ -715,7 +715,7 @@ impl QueryServer { pub fn write(&self) -> QueryServerWriteTransaction { // Feed the current schema index metadata to the be write transaction. let schema_write = self.schema.write(); - let idxmeta = schema_write.get_idxmeta(); + let idxmeta = schema_write.get_idxmeta_set(); QueryServerWriteTransaction { // I think this is *not* needed, because commit is mut self which should