fix: Have DatabaseState keep track of the UUID after reading it from object storage

pull/24376/head
Carol (Nichols || Goulding) 2021-10-29 21:50:08 -04:00
parent b1504f17ed
commit 207d701bdb
No known key found for this signature in database
GPG Key ID: E907EE5A736F87D4
3 changed files with 50 additions and 27 deletions

View File

@ -143,9 +143,8 @@ where
})?; })?;
let uuid = database let uuid = database
.provided_rules() .uuid()
.expect("Database should be initialized or an error should have been returned") .expect("Database should be initialized or an error should have been returned");
.uuid();
Ok(Response::new(CreateDatabaseResponse { Ok(Response::new(CreateDatabaseResponse {
uuid: uuid.as_bytes().to_vec(), uuid: uuid.as_bytes().to_vec(),

View File

@ -245,11 +245,13 @@ impl Database {
let handle = self.shared.state.read().freeze(); let handle = self.shared.state.read().freeze();
let handle = handle.await; let handle = handle.await;
{ let uuid = {
let state = self.shared.state.read(); let state = self.shared.state.read();
// Can't delete an already deleted database. // Can't delete an already deleted database.
ensure!(state.is_active(), CannotDeleteInactiveDatabase { db_name }); 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 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 // If there isn't an object store, something is wrong and we shouldn't switch the
@ -266,8 +268,6 @@ impl Database {
.await .await
.context(CannotMarkDatabaseDeleted { db_name })?; .context(CannotMarkDatabaseDeleted { db_name })?;
let uuid = self.provided_rules().expect("should have rules").uuid();
let mut state = self.shared.state.write(); let mut state = self.shared.state.write();
let mut state = state.unfreeze(handle); let mut state = state.unfreeze(handle);
*state = DatabaseState::NoActiveDatabase( *state = DatabaseState::NoActiveDatabase(
@ -388,6 +388,11 @@ impl Database {
self.shared.state.read().provided_rules() self.shared.state.read().provided_rules()
} }
/// Returns the database UUID if it's loaded
pub fn uuid(&self) -> Option<Uuid> {
self.shared.state.read().uuid()
}
/// Returns the info about the owning server if it has been loaded /// Returns the info about the owning server if it has been loaded
pub fn owner_info(&self) -> Option<management::v1::OwnerInfo> { pub fn owner_info(&self) -> Option<management::v1::OwnerInfo> {
self.shared.state.read().owner_info() 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 // 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` // 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) // (which we can't hold across the await to write to the object store)
( (initialized.db.iox_object_store(), initialized.uuid)
initialized.db.iox_object_store(),
initialized.provided_rules.uuid(),
)
}; // drop read lock }; // drop read lock
// Attempt to persist to object store, if that fails, roll // Attempt to persist to object store, if that fails, roll
@ -1073,6 +1075,25 @@ impl DatabaseState {
} }
} }
fn uuid(&self) -> Option<Uuid> {
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<management::v1::OwnerInfo> { fn owner_info(&self) -> Option<management::v1::OwnerInfo> {
match self { match self {
DatabaseState::Known(_) DatabaseState::Known(_)
@ -1212,6 +1233,7 @@ impl DatabaseStateOwnerInfoLoaded {
Ok(DatabaseStateRulesLoaded { Ok(DatabaseStateRulesLoaded {
provided_rules: rules.provided_rules(), provided_rules: rules.provided_rules(),
uuid: rules.uuid(),
owner_info: self.owner_info.clone(), owner_info: self.owner_info.clone(),
iox_object_store: Arc::clone(&self.iox_object_store), iox_object_store: Arc::clone(&self.iox_object_store),
}) })
@ -1221,6 +1243,7 @@ impl DatabaseStateOwnerInfoLoaded {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
struct DatabaseStateRulesLoaded { struct DatabaseStateRulesLoaded {
provided_rules: Arc<ProvidedDatabaseRules>, provided_rules: Arc<ProvidedDatabaseRules>,
uuid: Uuid,
owner_info: management::v1::OwnerInfo, owner_info: management::v1::OwnerInfo,
iox_object_store: Arc<IoxObjectStore>, iox_object_store: Arc<IoxObjectStore>,
} }
@ -1279,6 +1302,7 @@ impl DatabaseStateRulesLoaded {
lifecycle_worker, lifecycle_worker,
replay_plan: Arc::new(replay_plan), replay_plan: Arc::new(replay_plan),
provided_rules: Arc::clone(&self.provided_rules), provided_rules: Arc::clone(&self.provided_rules),
uuid: self.uuid,
owner_info: self.owner_info.clone(), owner_info: self.owner_info.clone(),
}) })
} }
@ -1290,6 +1314,7 @@ struct DatabaseStateCatalogLoaded {
lifecycle_worker: Arc<LifecycleWorker>, lifecycle_worker: Arc<LifecycleWorker>,
replay_plan: Arc<Option<ReplayPlan>>, replay_plan: Arc<Option<ReplayPlan>>,
provided_rules: Arc<ProvidedDatabaseRules>, provided_rules: Arc<ProvidedDatabaseRules>,
uuid: Uuid,
owner_info: management::v1::OwnerInfo, owner_info: management::v1::OwnerInfo,
} }
@ -1336,6 +1361,7 @@ impl DatabaseStateCatalogLoaded {
write_buffer_consumer, write_buffer_consumer,
lifecycle_worker: Arc::clone(&self.lifecycle_worker), lifecycle_worker: Arc::clone(&self.lifecycle_worker),
provided_rules: Arc::clone(&self.provided_rules), provided_rules: Arc::clone(&self.provided_rules),
uuid: self.uuid,
owner_info: self.owner_info.clone(), owner_info: self.owner_info.clone(),
}) })
} }
@ -1347,6 +1373,7 @@ pub struct DatabaseStateInitialized {
write_buffer_consumer: Option<Arc<WriteBufferConsumer>>, write_buffer_consumer: Option<Arc<WriteBufferConsumer>>,
lifecycle_worker: Arc<LifecycleWorker>, lifecycle_worker: Arc<LifecycleWorker>,
provided_rules: Arc<ProvidedDatabaseRules>, provided_rules: Arc<ProvidedDatabaseRules>,
uuid: Uuid,
owner_info: management::v1::OwnerInfo, owner_info: management::v1::OwnerInfo,
} }
@ -1371,11 +1398,7 @@ mod tests {
write_buffer::{WriteBufferConnection, WriteBufferDirection}, write_buffer::{WriteBufferConnection, WriteBufferDirection},
}; };
use entry::{test_helpers::lp_to_entries, SequencedEntry}; use entry::{test_helpers::lp_to_entries, SequencedEntry};
use std::{ use std::{num::NonZeroU32, time::Instant};
convert::{TryFrom, TryInto},
num::NonZeroU32,
time::Instant,
};
use time::Time; use time::Time;
use uuid::Uuid; use uuid::Uuid;
use write_buffer::mock::MockBufferSharedState; use write_buffer::mock::MockBufferSharedState;
@ -1506,7 +1529,7 @@ mod tests {
async fn database_delete_restore() { async fn database_delete_restore() {
let (application, database) = initialized_database().await; let (application, database) = initialized_database().await;
let db_name = &database.shared.config.name; 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; let server_id = database.shared.config.server_id;
database.delete().await.unwrap(); database.delete().await.unwrap();

View File

@ -866,9 +866,9 @@ where
.databases()? .databases()?
.iter() .iter()
.filter_map(|db| { .filter_map(|db| {
db.provided_rules().map(|rules| ActiveDatabase { db.uuid().map(|uuid| ActiveDatabase {
name: db.config().name.clone(), name: db.config().name.clone(),
uuid: rules.uuid(), uuid,
}) })
}) })
.collect()) .collect())
@ -1557,7 +1557,6 @@ mod tests {
worker_cleanup_avg_sleep: Duration::from_secs(2), worker_cleanup_avg_sleep: Duration::from_secs(2),
write_buffer_connection: None, write_buffer_connection: None,
}; };
let provided_rules = make_provided_rules(rules); let provided_rules = make_provided_rules(rules);
// Create a database // Create a database
@ -1601,7 +1600,7 @@ mod tests {
.create_database(provided_rules2) .create_database(provided_rules2)
.await .await
.expect("failed to create 2nd db"); .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 // assert server config file exists and has 2 entries
let config = server_config(application.object_store(), server_id).await; let config = server_config(application.object_store(), server_id).await;
@ -1699,7 +1698,7 @@ mod tests {
let bananas = create_simple_database(&server, "bananas") let bananas = create_simple_database(&server, "bananas")
.await .await
.expect("failed to create database"); .expect("failed to create database");
let bananas_uuid = bananas.provided_rules().unwrap().uuid(); let bananas_uuid = bananas.uuid().unwrap();
std::mem::drop(server); std::mem::drop(server);
@ -1710,7 +1709,7 @@ mod tests {
let apples = create_simple_database(&server, "apples") let apples = create_simple_database(&server, "apples")
.await .await
.expect("failed to create database"); .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"]); assert_eq!(server.db_names_sorted(), vec!["apples", "bananas"]);
@ -2290,7 +2289,7 @@ mod tests {
let foo = create_simple_database(&server, &foo_db_name) let foo = create_simple_database(&server, &foo_db_name)
.await .await
.expect("failed to create database"); .expect("failed to create database");
let foo_uuid = foo.provided_rules().unwrap().uuid(); let foo_uuid = foo.uuid().unwrap();
// delete database // delete database
server server
@ -2324,7 +2323,7 @@ mod tests {
let new_foo = create_simple_database(&server, &foo_db_name) let new_foo = create_simple_database(&server, &foo_db_name)
.await .await
.expect("failed to create database"); .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); assert_ne!(foo_uuid, new_foo_uuid);
// DB names contains foo // DB names contains foo
@ -2385,10 +2384,12 @@ mod tests {
..Default::default() ..Default::default()
}), }),
}; };
let provided_rules = make_provided_rules(rules); let provided_rules = make_provided_rules(rules);
server.update_db_rules(provided_rules).await.unwrap(); server.update_db_rules(provided_rules).await.unwrap();
} }
#[tokio::test] #[tokio::test]
async fn cant_restore_nonexistent_database() { async fn cant_restore_nonexistent_database() {
let application = make_application(); let application = make_application();
@ -2425,7 +2426,7 @@ mod tests {
let foo = create_simple_database(&server, &foo_db_name) let foo = create_simple_database(&server, &foo_db_name)
.await .await
.expect("failed to create database"); .expect("failed to create database");
let first_foo_uuid = foo.provided_rules().unwrap().uuid(); let first_foo_uuid = foo.uuid().unwrap();
// delete database // delete database
server server
@ -2464,7 +2465,7 @@ mod tests {
let foo = create_simple_database(&server, &foo_db_name) let foo = create_simple_database(&server, &foo_db_name)
.await .await
.expect("failed to create database"); .expect("failed to create database");
let foo_uuid = foo.provided_rules().unwrap().uuid(); let foo_uuid = foo.uuid().unwrap();
// delete database // delete database
server server