From 5b322561ce26aaf51c797f51f8e935f8343063eb Mon Sep 17 00:00:00 2001 From: Jason Volk Date: Thu, 10 Apr 2025 20:55:41 +0000 Subject: [PATCH] simplify database backup interface related Signed-off-by: Jason Volk --- src/admin/server/commands.rs | 35 ++++++++------- src/database/engine/backup.rs | 80 ++++++++++++++++++++++------------- src/service/globals/data.rs | 6 --- 3 files changed, 69 insertions(+), 52 deletions(-) diff --git a/src/admin/server/commands.rs b/src/admin/server/commands.rs index b01e9296..6027a9eb 100644 --- a/src/admin/server/commands.rs +++ b/src/admin/server/commands.rs @@ -1,6 +1,11 @@ use std::{fmt::Write, path::PathBuf, sync::Arc}; -use conduwuit::{Err, Result, info, utils::time, warn}; +use conduwuit::{ + Err, Result, info, + utils::{stream::IterStream, time}, + warn, +}; +use futures::TryStreamExt; use crate::admin_command; @@ -81,33 +86,31 @@ pub(super) async fn clear_caches(&self) -> Result { #[admin_command] pub(super) async fn list_backups(&self) -> Result { - let result = self.services.db.db.backup_list()?; - - if result.is_empty() { - return Err!("No backups found."); - } - - self.write_str(&result).await + self.services + .db + .db + .backup_list()? + .try_stream() + .try_for_each(|result| write!(self, "{result}")) + .await } #[admin_command] pub(super) async fn backup_database(&self) -> Result { let db = Arc::clone(&self.services.db); - let mut result = self + let result = self .services .server .runtime() .spawn_blocking(move || match db.db.backup() { - | Ok(()) => String::new(), - | Err(e) => e.to_string(), + | Ok(()) => "Done".to_owned(), + | Err(e) => format!("Failed: {e}"), }) .await?; - if result.is_empty() { - result = self.services.db.db.backup_list()?; - } - - self.write_str(&result).await + let count = self.services.db.db.backup_count()?; + self.write_str(&format!("{result}. Currently have {count} backups.")) + .await } #[admin_command] diff --git a/src/database/engine/backup.rs b/src/database/engine/backup.rs index bb110630..ac72e6d4 100644 --- a/src/database/engine/backup.rs +++ b/src/database/engine/backup.rs @@ -1,24 +1,16 @@ -use std::fmt::Write; +use std::{ffi::OsString, path::PathBuf}; -use conduwuit::{Result, error, implement, info, utils::time::rfc2822_from_seconds, warn}; +use conduwuit::{Err, Result, error, implement, info, utils::time::rfc2822_from_seconds, warn}; use rocksdb::backup::{BackupEngine, BackupEngineOptions}; use super::Engine; -use crate::{or_else, util::map_err}; +use crate::util::map_err; #[implement(Engine)] #[tracing::instrument(skip(self))] pub fn backup(&self) -> Result { - let server = &self.ctx.server; - let config = &server.config; - let path = config.database_backup_path.as_ref(); - if path.is_none() || path.is_some_and(|path| path.as_os_str().is_empty()) { - return Ok(()); - } - - let options = - BackupEngineOptions::new(path.expect("valid database backup path")).map_err(map_err)?; - let mut engine = BackupEngine::open(&options, &*self.ctx.env.lock()?).map_err(map_err)?; + let mut engine = self.backup_engine()?; + let config = &self.ctx.server.config; if config.database_backups_to_keep > 0 { let flush = !self.is_read_only(); engine @@ -40,34 +32,62 @@ pub fn backup(&self) -> Result { } } + if config.database_backups_to_keep == 0 { + warn!("Configuration item `database_backups_to_keep` is set to 0."); + } + Ok(()) } #[implement(Engine)] -pub fn backup_list(&self) -> Result { - let server = &self.ctx.server; - let config = &server.config; - let path = config.database_backup_path.as_ref(); - if path.is_none() || path.is_some_and(|path| path.as_os_str().is_empty()) { - return Ok("Configure database_backup_path to enable backups, or the path specified is \ - not valid" - .to_owned()); +pub fn backup_list(&self) -> Result + Send> { + let info = self.backup_engine()?.get_backup_info(); + + if info.is_empty() { + return Err!("No backups found."); } - let mut res = String::new(); - let options = - BackupEngineOptions::new(path.expect("valid database backup path")).or_else(or_else)?; - let engine = BackupEngine::open(&options, &*self.ctx.env.lock()?).or_else(or_else)?; - for info in engine.get_backup_info() { - writeln!( - res, + let list = info.into_iter().map(|info| { + format!( "#{} {}: {} bytes, {} files", info.backup_id, rfc2822_from_seconds(info.timestamp), info.size, info.num_files, - )?; + ) + }); + + Ok(list) +} + +#[implement(Engine)] +pub fn backup_count(&self) -> Result { + let info = self.backup_engine()?.get_backup_info(); + + Ok(info.len()) +} + +#[implement(Engine)] +fn backup_engine(&self) -> Result { + let path = self.backup_path()?; + let options = BackupEngineOptions::new(path).map_err(map_err)?; + BackupEngine::open(&options, &*self.ctx.env.lock()?).map_err(map_err) +} + +#[implement(Engine)] +fn backup_path(&self) -> Result { + let path = self + .ctx + .server + .config + .database_backup_path + .clone() + .map(PathBuf::into_os_string) + .unwrap_or_default(); + + if path.is_empty() { + return Err!(Config("database_backup_path", "Configure path to enable backups")); } - Ok(res) + Ok(path) } diff --git a/src/service/globals/data.rs b/src/service/globals/data.rs index b43b7c5f..21c09252 100644 --- a/src/service/globals/data.rs +++ b/src/service/globals/data.rs @@ -72,10 +72,4 @@ impl Data { pub fn bump_database_version(&self, new_version: u64) { self.global.raw_put(b"version", new_version); } - - #[inline] - pub fn backup(&self) -> Result { self.db.db.backup() } - - #[inline] - pub fn backup_list(&self) -> Result { self.db.db.backup_list() } }