From 207d701bdbd21cbd1cc1ad627efd1efa639f4be8 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 29 Oct 2021 21:50:08 -0400 Subject: [PATCH] fix: Have DatabaseState keep track of the UUID after reading it from object storage --- .../server_type/database/rpc/management.rs | 5 +- server/src/database.rs | 51 ++++++++++++++----- server/src/lib.rs | 21 ++++---- 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/influxdb_iox/src/influxdb_ioxd/server_type/database/rpc/management.rs b/influxdb_iox/src/influxdb_ioxd/server_type/database/rpc/management.rs index c20207afd4..f7920c5bbc 100644 --- a/influxdb_iox/src/influxdb_ioxd/server_type/database/rpc/management.rs +++ b/influxdb_iox/src/influxdb_ioxd/server_type/database/rpc/management.rs @@ -143,9 +143,8 @@ where })?; let uuid = database - .provided_rules() - .expect("Database should be initialized or an error should have been returned") - .uuid(); + .uuid() + .expect("Database should be initialized or an error should have been returned"); Ok(Response::new(CreateDatabaseResponse { uuid: uuid.as_bytes().to_vec(), diff --git a/server/src/database.rs b/server/src/database.rs index 0375f4dd8f..76e3e0d181 100644 --- a/server/src/database.rs +++ b/server/src/database.rs @@ -245,11 +245,13 @@ impl Database { let handle = self.shared.state.read().freeze(); let handle = handle.await; - { + let uuid = { let state = self.shared.state.read(); // Can't delete an already deleted database. ensure!(state.is_active(), CannotDeleteInactiveDatabase { db_name }); - } + + state.uuid().expect("Active databases have UUIDs") + }; // If there is an object store for this database, write out a tombstone file. // If there isn't an object store, something is wrong and we shouldn't switch the @@ -266,8 +268,6 @@ impl Database { .await .context(CannotMarkDatabaseDeleted { db_name })?; - let uuid = self.provided_rules().expect("should have rules").uuid(); - let mut state = self.shared.state.write(); let mut state = state.unfreeze(handle); *state = DatabaseState::NoActiveDatabase( @@ -388,6 +388,11 @@ impl Database { self.shared.state.read().provided_rules() } + /// Returns the database UUID if it's loaded + pub fn uuid(&self) -> Option { + self.shared.state.read().uuid() + } + /// Returns the info about the owning server if it has been loaded pub fn owner_info(&self) -> Option { self.shared.state.read().owner_info() @@ -428,10 +433,7 @@ impl Database { // A handle to the object store and a copy of the UUID so we can update the rules // in object store prior to obtaining exclusive write access to the `DatabaseState` // (which we can't hold across the await to write to the object store) - ( - initialized.db.iox_object_store(), - initialized.provided_rules.uuid(), - ) + (initialized.db.iox_object_store(), initialized.uuid) }; // drop read lock // Attempt to persist to object store, if that fails, roll @@ -1073,6 +1075,25 @@ impl DatabaseState { } } + fn uuid(&self) -> Option { + match self { + DatabaseState::Known(_) + | DatabaseState::DatabaseObjectStoreFound(_) + | DatabaseState::DatabaseObjectStoreLookupError(_, _) + | DatabaseState::NoActiveDatabase(_, _) + | DatabaseState::OwnerInfoLoaded(_) + | DatabaseState::OwnerInfoLoadError(_, _) + | DatabaseState::RulesLoadError(_, _) => None, + DatabaseState::RulesLoaded(state) | DatabaseState::CatalogLoadError(state, _) => { + Some(state.uuid) + } + DatabaseState::CatalogLoaded(state) | DatabaseState::ReplayError(state, _) => { + Some(state.uuid) + } + DatabaseState::Initialized(state) => Some(state.uuid), + } + } + fn owner_info(&self) -> Option { match self { DatabaseState::Known(_) @@ -1212,6 +1233,7 @@ impl DatabaseStateOwnerInfoLoaded { Ok(DatabaseStateRulesLoaded { provided_rules: rules.provided_rules(), + uuid: rules.uuid(), owner_info: self.owner_info.clone(), iox_object_store: Arc::clone(&self.iox_object_store), }) @@ -1221,6 +1243,7 @@ impl DatabaseStateOwnerInfoLoaded { #[derive(Debug, Clone)] struct DatabaseStateRulesLoaded { provided_rules: Arc, + uuid: Uuid, owner_info: management::v1::OwnerInfo, iox_object_store: Arc, } @@ -1279,6 +1302,7 @@ impl DatabaseStateRulesLoaded { lifecycle_worker, replay_plan: Arc::new(replay_plan), provided_rules: Arc::clone(&self.provided_rules), + uuid: self.uuid, owner_info: self.owner_info.clone(), }) } @@ -1290,6 +1314,7 @@ struct DatabaseStateCatalogLoaded { lifecycle_worker: Arc, replay_plan: Arc>, provided_rules: Arc, + uuid: Uuid, owner_info: management::v1::OwnerInfo, } @@ -1336,6 +1361,7 @@ impl DatabaseStateCatalogLoaded { write_buffer_consumer, lifecycle_worker: Arc::clone(&self.lifecycle_worker), provided_rules: Arc::clone(&self.provided_rules), + uuid: self.uuid, owner_info: self.owner_info.clone(), }) } @@ -1347,6 +1373,7 @@ pub struct DatabaseStateInitialized { write_buffer_consumer: Option>, lifecycle_worker: Arc, provided_rules: Arc, + uuid: Uuid, owner_info: management::v1::OwnerInfo, } @@ -1371,11 +1398,7 @@ mod tests { write_buffer::{WriteBufferConnection, WriteBufferDirection}, }; use entry::{test_helpers::lp_to_entries, SequencedEntry}; - use std::{ - convert::{TryFrom, TryInto}, - num::NonZeroU32, - time::Instant, - }; + use std::{num::NonZeroU32, time::Instant}; use time::Time; use uuid::Uuid; use write_buffer::mock::MockBufferSharedState; @@ -1506,7 +1529,7 @@ mod tests { async fn database_delete_restore() { let (application, database) = initialized_database().await; let db_name = &database.shared.config.name; - let uuid = database.provided_rules().unwrap().uuid(); + let uuid = database.uuid().unwrap(); let server_id = database.shared.config.server_id; database.delete().await.unwrap(); diff --git a/server/src/lib.rs b/server/src/lib.rs index 88bb7a09f9..f13e917a18 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -866,9 +866,9 @@ where .databases()? .iter() .filter_map(|db| { - db.provided_rules().map(|rules| ActiveDatabase { + db.uuid().map(|uuid| ActiveDatabase { name: db.config().name.clone(), - uuid: rules.uuid(), + uuid, }) }) .collect()) @@ -1557,7 +1557,6 @@ mod tests { worker_cleanup_avg_sleep: Duration::from_secs(2), write_buffer_connection: None, }; - let provided_rules = make_provided_rules(rules); // Create a database @@ -1601,7 +1600,7 @@ mod tests { .create_database(provided_rules2) .await .expect("failed to create 2nd db"); - let awesome_uuid = awesome.provided_rules().unwrap().uuid(); + let awesome_uuid = awesome.uuid().unwrap(); // assert server config file exists and has 2 entries let config = server_config(application.object_store(), server_id).await; @@ -1699,7 +1698,7 @@ mod tests { let bananas = create_simple_database(&server, "bananas") .await .expect("failed to create database"); - let bananas_uuid = bananas.provided_rules().unwrap().uuid(); + let bananas_uuid = bananas.uuid().unwrap(); std::mem::drop(server); @@ -1710,7 +1709,7 @@ mod tests { let apples = create_simple_database(&server, "apples") .await .expect("failed to create database"); - let apples_uuid = apples.provided_rules().unwrap().uuid(); + let apples_uuid = apples.uuid().unwrap(); assert_eq!(server.db_names_sorted(), vec!["apples", "bananas"]); @@ -2290,7 +2289,7 @@ mod tests { let foo = create_simple_database(&server, &foo_db_name) .await .expect("failed to create database"); - let foo_uuid = foo.provided_rules().unwrap().uuid(); + let foo_uuid = foo.uuid().unwrap(); // delete database server @@ -2324,7 +2323,7 @@ mod tests { let new_foo = create_simple_database(&server, &foo_db_name) .await .expect("failed to create database"); - let new_foo_uuid = new_foo.provided_rules().unwrap().uuid(); + let new_foo_uuid = new_foo.uuid().unwrap(); assert_ne!(foo_uuid, new_foo_uuid); // DB names contains foo @@ -2385,10 +2384,12 @@ mod tests { ..Default::default() }), }; + let provided_rules = make_provided_rules(rules); server.update_db_rules(provided_rules).await.unwrap(); } + #[tokio::test] async fn cant_restore_nonexistent_database() { let application = make_application(); @@ -2425,7 +2426,7 @@ mod tests { let foo = create_simple_database(&server, &foo_db_name) .await .expect("failed to create database"); - let first_foo_uuid = foo.provided_rules().unwrap().uuid(); + let first_foo_uuid = foo.uuid().unwrap(); // delete database server @@ -2464,7 +2465,7 @@ mod tests { let foo = create_simple_database(&server, &foo_db_name) .await .expect("failed to create database"); - let foo_uuid = foo.provided_rules().unwrap().uuid(); + let foo_uuid = foo.uuid().unwrap(); // delete database server