fix: Return UUID when a database is deleted

So that it can be immediately used to restore elsewhere.
pull/24376/head
Carol (Nichols || Goulding) 2021-10-22 16:51:18 -04:00 committed by Carol (Nichols || Goulding)
parent e80b902d15
commit 95ba84edca
No known key found for this signature in database
GPG Key ID: E907EE5A736F87D4
11 changed files with 91 additions and 59 deletions

1
Cargo.lock generated
View File

@ -1685,6 +1685,7 @@ dependencies = [
"thiserror", "thiserror",
"tokio", "tokio",
"tonic", "tonic",
"uuid",
] ]
[[package]] [[package]]

View File

@ -174,7 +174,9 @@ message DeleteDatabaseRequest {
string db_name = 1; string db_name = 1;
} }
message DeleteDatabaseResponse {} message DeleteDatabaseResponse {
bytes uuid = 1;
}
message RestoreDatabaseRequest { message RestoreDatabaseRequest {
// Was the generation ID of the deleted database. // Was the generation ID of the deleted database.

View File

@ -342,8 +342,9 @@ pub async fn command(connection: Connection, config: Config) -> Result<()> {
} }
Command::Delete(command) => { Command::Delete(command) => {
let mut client = management::Client::new(connection); let mut client = management::Client::new(connection);
client.delete_database(&command.name).await?; let uuid = client.delete_database(&command.name).await?;
println!("Deleted database {}", command.name); eprintln!("Deleted database {}", command.name);
println!("{}", uuid);
} }
Command::Restore(command) => { Command::Restore(command) => {
let mut client = management::Client::new(connection); let mut client = management::Client::new(connection);

View File

@ -130,12 +130,9 @@ pub fn default_database_error_handler(error: server::database::Error) -> tonic::
error!(%source, "Unexpected error deleting database"); error!(%source, "Unexpected error deleting database");
InternalError {}.into() InternalError {}.into()
} }
Error::NoActiveDatabaseToDelete { db_name } => NotFound { Error::CannotDeleteInactiveDatabase { .. } => {
resource_type: "database".to_string(), tonic::Status::failed_precondition(error.to_string())
resource_name: db_name,
..Default::default()
} }
.into(),
Error::CannotRestoreActiveDatabase { .. } => { Error::CannotRestoreActiveDatabase { .. } => {
tonic::Status::failed_precondition(error.to_string()) tonic::Status::failed_precondition(error.to_string())
} }

View File

@ -175,12 +175,15 @@ where
) -> Result<Response<DeleteDatabaseResponse>, Status> { ) -> Result<Response<DeleteDatabaseResponse>, Status> {
let db_name = DatabaseName::new(request.into_inner().db_name).field("db_name")?; let db_name = DatabaseName::new(request.into_inner().db_name).field("db_name")?;
self.server let uuid = self
.server
.delete_database(&db_name) .delete_database(&db_name)
.await .await
.map_err(default_server_error_handler)?; .map_err(default_server_error_handler)?;
Ok(Response::new(DeleteDatabaseResponse {})) Ok(Response::new(DeleteDatabaseResponse {
uuid: uuid.as_bytes().to_vec(),
}))
} }
async fn restore_database( async fn restore_database(

View File

@ -18,8 +18,7 @@ async fn test_querying_deleted_database() {
..Default::default() ..Default::default()
}; };
// Add some data to the first generation of the database and // Add some data to the database and ensure the data is returned.
// ensure the data is returned.
management_client management_client
.create_database(rules.clone()) .create_database(rules.clone())

View File

@ -244,17 +244,24 @@ async fn delete_database() {
.success() .success()
.stdout(active_db_match(db, 0)); .stdout(active_db_match(db, 0));
// Delete the database // Delete the database, returns the UUID
Command::cargo_bin("influxdb_iox") let db_uuid = String::from_utf8(
.unwrap() Command::cargo_bin("influxdb_iox")
.arg("database") .unwrap()
.arg("delete") .arg("database")
.arg(db) .arg("delete")
.arg("--host") .arg(db)
.arg(addr) .arg("--host")
.assert() .arg(addr)
.success() .assert()
.stdout(predicate::str::contains(format!("Deleted database {}", db))); .success()
.stderr(predicate::str::contains(format!("Deleted database {}", db)))
.get_output()
.stdout
.clone(),
)
.unwrap();
let db_uuid = db_uuid.trim();
// Listing the databases does not include the deleted database // Listing the databases does not include the deleted database
Command::cargo_bin("influxdb_iox") Command::cargo_bin("influxdb_iox")
@ -316,19 +323,7 @@ async fn delete_database() {
.success() .success()
.stdout(predicate::str::contains(db)); .stdout(predicate::str::contains(db));
// Listing detailed database info includes both active and deleted // Delete the 2nd database, returns its UUID
Command::cargo_bin("influxdb_iox")
.unwrap()
.arg("database")
.arg("list")
.arg("--detailed")
.arg("--host")
.arg(addr)
.assert()
.success()
.stdout(deleted_db_match(db, 0).and(active_db_match(db, 1)));
// Delete the 2nd database
Command::cargo_bin("influxdb_iox") Command::cargo_bin("influxdb_iox")
.unwrap() .unwrap()
.arg("database") .arg("database")
@ -338,7 +333,7 @@ async fn delete_database() {
.arg(addr) .arg(addr)
.assert() .assert()
.success() .success()
.stdout(predicate::str::contains(format!("Deleted database {}", db))); .stderr(predicate::str::contains(format!("Deleted database {}", db)));
// This database should no longer be in the active list // This database should no longer be in the active list
Command::cargo_bin("influxdb_iox") Command::cargo_bin("influxdb_iox")

View File

@ -25,6 +25,7 @@ serde = "1.0.128"
serde_json = { version = "1.0.67", optional = true } serde_json = { version = "1.0.67", optional = true }
thiserror = "1.0.30" thiserror = "1.0.30"
tonic = { version = "0.5.0" } tonic = { version = "0.5.0" }
uuid = { version = "0.8", features = ["v4"] }
[dev-dependencies] # In alphabetical order [dev-dependencies] # In alphabetical order
serde_json = "1.0" serde_json = "1.0"

View File

@ -1,13 +1,12 @@
use bytes::Bytes;
use thiserror::Error;
use self::generated_types::{management_service_client::ManagementServiceClient, *}; use self::generated_types::{management_service_client::ManagementServiceClient, *};
use crate::{
use crate::connection::Connection; connection::Connection,
google::{longrunning::IoxOperation, FieldViolation},
use crate::google::{longrunning::IoxOperation, FieldViolation}; };
use std::convert::TryInto; use bytes::Bytes;
use std::num::NonZeroU32; use std::{convert::TryInto, num::NonZeroU32};
use thiserror::Error;
use uuid::Uuid;
/// Re-export generated_types /// Re-export generated_types
pub mod generated_types { pub mod generated_types {
@ -144,6 +143,10 @@ pub enum DeleteDatabaseError {
/// Client received an unexpected error from the server /// Client received an unexpected error from the server
#[error("Unexpected server error: {}: {}", .0.code(), .0.message())] #[error("Unexpected server error: {}: {}", .0.code(), .0.message())]
ServerError(tonic::Status), ServerError(tonic::Status),
/// UUID returned as bytes from server could not be converted to a `Uuid`
#[error("Invalid UUID: {}: {:?}", .0, .0)]
InvalidUuid(uuid::Error),
} }
/// Errors returned by Client::delete_database /// Errors returned by Client::delete_database
@ -656,8 +659,9 @@ impl Client {
pub async fn delete_database( pub async fn delete_database(
&mut self, &mut self,
db_name: impl Into<String> + Send, db_name: impl Into<String> + Send,
) -> Result<(), DeleteDatabaseError> { ) -> Result<Uuid, DeleteDatabaseError> {
self.inner let response = self
.inner
.delete_database(DeleteDatabaseRequest { .delete_database(DeleteDatabaseRequest {
db_name: db_name.into(), db_name: db_name.into(),
}) })
@ -669,7 +673,10 @@ impl Client {
_ => DeleteDatabaseError::ServerError(status), _ => DeleteDatabaseError::ServerError(status),
})?; })?;
Ok(()) let uuid = Uuid::from_slice(&response.into_inner().uuid)
.map_err(DeleteDatabaseError::InvalidUuid)?;
Ok(uuid)
} }
/// Restore database /// Restore database

View File

@ -82,14 +82,17 @@ pub enum Error {
#[snafu(display("cannot persisted updated rules: {}", source))] #[snafu(display("cannot persisted updated rules: {}", source))]
CannotPersistUpdatedRules { source: crate::rules::Error }, CannotPersistUpdatedRules { source: crate::rules::Error },
#[snafu(display("cannot mark database deleted: {}", source))] #[snafu(display("cannot mark database {} deleted: {}", db_name, source))]
CannotMarkDatabaseDeleted { CannotMarkDatabaseDeleted {
db_name: String, db_name: String,
source: object_store::Error, source: object_store::Error,
}, },
#[snafu(display("no active database named {} to delete", db_name))] #[snafu(display(
NoActiveDatabaseToDelete { db_name: String }, "cannot delete database named {} that has already been marked as deleted",
db_name
))]
CannotDeleteInactiveDatabase { db_name: String },
#[snafu(display("cannot restore database named {} that is already active", db_name))] #[snafu(display("cannot restore database named {} that is already active", db_name))]
CannotRestoreActiveDatabase { db_name: String }, CannotRestoreActiveDatabase { db_name: String },
@ -239,7 +242,7 @@ impl Database {
} }
/// Mark this database as deleted. /// Mark this database as deleted.
pub async fn delete(&self) -> Result<(), Error> { pub async fn delete(&self) -> Result<Uuid, Error> {
let db_name = &self.shared.config.name; let db_name = &self.shared.config.name;
info!(%db_name, "marking database deleted"); info!(%db_name, "marking database deleted");
@ -249,7 +252,7 @@ impl Database {
{ {
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(), NoActiveDatabaseToDelete { db_name }); ensure!(state.is_active(), CannotDeleteInactiveDatabase { db_name });
} }
// 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.
@ -259,7 +262,7 @@ impl Database {
.with_context(|| { .with_context(|| {
let state = self.shared.state.read(); let state = self.shared.state.read();
TransitionInProgress { TransitionInProgress {
db_name, db_name: db_name.clone(),
state: state.state_code(), state: state.state_code(),
} }
})? })?
@ -267,6 +270,8 @@ 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(
@ -275,7 +280,7 @@ impl Database {
); );
self.shared.state_notify.notify_waiters(); self.shared.state_notify.notify_waiters();
Ok(()) Ok(uuid)
} }
/// Create a restored database without any any state. /// Create a restored database without any any state.

View File

@ -736,10 +736,31 @@ where
/// Delete an existing, active database with this name. Return an error if no active database /// Delete an existing, active database with this name. Return an error if no active database
/// with this name can be found. /// with this name can be found.
pub async fn delete_database(&self, db_name: &DatabaseName<'static>) -> Result<()> { pub async fn delete_database(&self, db_name: &DatabaseName<'static>) -> Result<Uuid> {
// Wait for exclusive access to mutate server state
let handle_fut = self.shared.state.read().freeze();
let handle = handle_fut.await;
let database = self.database(db_name)?; let database = self.database(db_name)?;
database.delete().await.context(CannotMarkDatabaseDeleted)?; let uuid = database.delete().await.context(CannotMarkDatabaseDeleted)?;
Ok(())
{
let mut state = self.shared.state.write();
// Exchange FreezeHandle for mutable access via WriteGuard
let mut state = state.unfreeze(handle);
match &mut *state {
ServerState::Initialized(initialized) => {
initialized.databases.remove(db_name);
}
_ => unreachable!(),
}
}
self.persist_server_config().await?;
Ok(uuid)
} }
/// Restore a database and generation that has been marked as deleted. Return an error if no /// Restore a database and generation that has been marked as deleted. Return an error if no