From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 948EE9EFDF for ; Fri, 3 Nov 2023 16:17:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7DE6E1FDD4 for ; Fri, 3 Nov 2023 16:17:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 3 Nov 2023 16:17:09 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 817B24434C for ; Fri, 3 Nov 2023 16:17:09 +0100 (CET) From: Maximiliano Sandoval R To: pbs-devel@lists.proxmox.com Date: Fri, 3 Nov 2023 16:17:06 +0100 Message-Id: <20231103151707.191010-1-m.sandoval@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.002 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [restore.rs, recover.rs, main.rs, benchmark.rs, snapshot.rs, lib.rs] Subject: [pbs-devel] [RFC proxmox-backup 1/2] datastore: Allow encoding with a set compression level X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Nov 2023 15:17:14 -0000 We replace the boolean flag with an enum. Signed-off-by: Maximiliano Sandoval R --- pbs-client/src/backup_writer.rs | 15 +++---- pbs-datastore/src/backup_info.rs | 4 +- pbs-datastore/src/data_blob.rs | 56 +++++++++++++++---------- pbs-datastore/src/dynamic_index.rs | 3 +- pbs-datastore/src/lib.rs | 2 +- proxmox-backup-client/src/benchmark.rs | 5 ++- proxmox-backup-client/src/main.rs | 17 ++++---- proxmox-backup-client/src/snapshot.rs | 12 ++++-- src/api2/tape/restore.rs | 4 +- src/bin/proxmox_backup_debug/recover.rs | 8 +++- 10 files changed, 76 insertions(+), 50 deletions(-) diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs index 8a03d8ea..cfec0a1e 100644 --- a/pbs-client/src/backup_writer.rs +++ b/pbs-client/src/backup_writer.rs @@ -18,7 +18,7 @@ use pbs_datastore::dynamic_index::DynamicIndexReader; use pbs_datastore::fixed_index::FixedIndexReader; use pbs_datastore::index::IndexFile; use pbs_datastore::manifest::{ArchiveType, BackupManifest, MANIFEST_BLOB_NAME}; -use pbs_datastore::{CATALOG_NAME, PROXMOX_BACKUP_PROTOCOL_ID_V1}; +use pbs_datastore::{Compression, CATALOG_NAME, PROXMOX_BACKUP_PROTOCOL_ID_V1}; use pbs_tools::crypt_config::CryptConfig; use proxmox_human_byte::HumanByte; @@ -48,7 +48,7 @@ pub struct BackupStats { #[derive(Default, Clone)] pub struct UploadOptions { pub previous_manifest: Option>, - pub compress: bool, + pub compression: Compression, pub encrypt: bool, pub fixed_size: Option, } @@ -213,10 +213,10 @@ impl BackupWriter { options: UploadOptions, ) -> Result { let blob = match (options.encrypt, &self.crypt_config) { - (false, _) => DataBlob::encode(&data, None, options.compress)?, + (false, _) => DataBlob::encode(&data, None, options.compression)?, (true, None) => bail!("requested encryption without a crypt config"), (true, Some(crypt_config)) => { - DataBlob::encode(&data, Some(crypt_config), options.compress)? + DataBlob::encode(&data, Some(crypt_config), options.compression)? } }; @@ -340,7 +340,7 @@ impl BackupWriter { } else { None }, - options.compress, + options.compression, ) .await?; @@ -636,7 +636,7 @@ impl BackupWriter { prefix: &str, known_chunks: Arc>>, crypt_config: Option>, - compress: bool, + compression: Compression, ) -> impl Future> { let total_chunks = Arc::new(AtomicUsize::new(0)); let total_chunks2 = total_chunks.clone(); @@ -669,7 +669,8 @@ impl BackupWriter { total_chunks.fetch_add(1, Ordering::SeqCst); let offset = stream_len.fetch_add(chunk_len, Ordering::SeqCst) as u64; - let mut chunk_builder = DataChunkBuilder::new(data.as_ref()).compress(compress); + let mut chunk_builder = + DataChunkBuilder::new(data.as_ref()).compression(compression); if let Some(ref crypt_config) = crypt_config { chunk_builder = chunk_builder.crypt_config(crypt_config); diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 128315ba..a8a8b631 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -15,7 +15,7 @@ use pbs_config::{open_backup_lockfile, BackupLockGuard}; use crate::manifest::{ BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MANIFEST_LOCK_NAME, }; -use crate::{DataBlob, DataStore}; +use crate::{Compression, DataBlob, DataStore}; /// BackupGroup is a directory containing a list of BackupDir #[derive(Clone)] @@ -505,7 +505,7 @@ impl BackupDir { let manifest = serde_json::to_value(manifest)?; let manifest = serde_json::to_string_pretty(&manifest)?; - let blob = DataBlob::encode(manifest.as_bytes(), None, true)?; + let blob = DataBlob::encode(manifest.as_bytes(), None, Compression::default())?; let raw_data = blob.raw_data(); let mut path = self.full_path(); diff --git a/pbs-datastore/src/data_blob.rs b/pbs-datastore/src/data_blob.rs index 4119c4a4..765f5a98 100644 --- a/pbs-datastore/src/data_blob.rs +++ b/pbs-datastore/src/data_blob.rs @@ -12,6 +12,18 @@ use super::file_formats::*; const MAX_BLOB_SIZE: usize = 128 * 1024 * 1024; +#[derive(Copy, Clone, Debug)] +pub enum Compression { + None, + Zstd(i32), +} + +impl Default for Compression { + fn default() -> Self { + Compression::Zstd(1) + } +} + /// Encoded data chunk with digest and positional information pub struct ChunkInfo { pub chunk: DataBlob, @@ -87,7 +99,7 @@ impl DataBlob { pub fn encode( data: &[u8], config: Option<&CryptConfig>, - compress: bool, + compression: Compression, ) -> Result { if data.len() > MAX_BLOB_SIZE { bail!("data blob too large ({} bytes).", data.len()); @@ -95,16 +107,18 @@ impl DataBlob { let mut blob = if let Some(config) = config { let compr_data; - let (_compress, data, magic) = if compress { - compr_data = zstd::bulk::compress(data, 1)?; - // Note: We only use compression if result is shorter - if compr_data.len() < data.len() { - (true, &compr_data[..], ENCR_COMPR_BLOB_MAGIC_1_0) - } else { - (false, data, ENCRYPTED_BLOB_MAGIC_1_0) + + let (_compress, data, magic) = match compression { + Compression::Zstd(level) => { + compr_data = zstd::bulk::compress(data, level)?; + // Note: We only use compression if result is shorter + if compr_data.len() < data.len() { + (true, &compr_data[..], ENCR_COMPR_BLOB_MAGIC_1_0) + } else { + (false, data, ENCRYPTED_BLOB_MAGIC_1_0) + } } - } else { - (false, data, ENCRYPTED_BLOB_MAGIC_1_0) + Compression::None => (false, data, ENCRYPTED_BLOB_MAGIC_1_0), }; let header_len = std::mem::size_of::(); @@ -137,7 +151,7 @@ impl DataBlob { DataBlob { raw_data } } else { let max_data_len = data.len() + std::mem::size_of::(); - if compress { + if let Compression::Zstd(level) = compression { let mut comp_data = Vec::with_capacity(max_data_len); let head = DataBlobHeader { @@ -148,7 +162,7 @@ impl DataBlob { comp_data.write_le_value(head)?; } - zstd::stream::copy_encode(data, &mut comp_data, 1)?; + zstd::stream::copy_encode(data, &mut comp_data, level)?; if comp_data.len() < max_data_len { let mut blob = DataBlob { @@ -479,7 +493,7 @@ pub struct DataChunkBuilder<'a, 'b> { orig_data: &'a [u8], digest_computed: bool, digest: [u8; 32], - compress: bool, + compression: Compression, } impl<'a, 'b> DataChunkBuilder<'a, 'b> { @@ -490,15 +504,13 @@ impl<'a, 'b> DataChunkBuilder<'a, 'b> { config: None, digest_computed: false, digest: [0u8; 32], - compress: true, + compression: Compression::default(), } } - /// Set compression flag. - /// - /// If true, chunk data is compressed using zstd (level 1). - pub fn compress(mut self, value: bool) -> Self { - self.compress = value; + /// Set compression level + pub fn compression(mut self, value: Compression) -> Self { + self.compression = value; self } @@ -543,7 +555,7 @@ impl<'a, 'b> DataChunkBuilder<'a, 'b> { self.compute_digest(); } - let chunk = DataBlob::encode(self.orig_data, self.config, self.compress)?; + let chunk = DataBlob::encode(self.orig_data, self.config, self.compression)?; Ok((chunk, self.digest)) } @@ -551,10 +563,10 @@ impl<'a, 'b> DataChunkBuilder<'a, 'b> { pub fn build_zero_chunk( crypt_config: Option<&CryptConfig>, chunk_size: usize, - compress: bool, + compression: Compression, ) -> Result<(DataBlob, [u8; 32]), Error> { let zero_bytes = vec![0; chunk_size]; - let mut chunk_builder = DataChunkBuilder::new(&zero_bytes).compress(compress); + let mut chunk_builder = DataChunkBuilder::new(&zero_bytes).compression(compression); if let Some(crypt_config) = crypt_config { chunk_builder = chunk_builder.crypt_config(crypt_config); } diff --git a/pbs-datastore/src/dynamic_index.rs b/pbs-datastore/src/dynamic_index.rs index 71a5082e..04e644e0 100644 --- a/pbs-datastore/src/dynamic_index.rs +++ b/pbs-datastore/src/dynamic_index.rs @@ -24,6 +24,7 @@ use crate::file_formats; use crate::index::{ChunkReadInfo, IndexFile}; use crate::read_chunk::ReadChunk; use crate::Chunker; +use crate::Compression; /// Header format definition for dynamic index files (`.dixd`) #[repr(C)] @@ -453,7 +454,7 @@ impl DynamicChunkWriter { self.last_chunk = self.chunk_offset; let (chunk, digest) = DataChunkBuilder::new(&self.chunk_buffer) - .compress(true) + .compression(Compression::default()) .build()?; match self.index.insert_chunk(&chunk, &digest) { diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs index b09fd114..430e2927 100644 --- a/pbs-datastore/src/lib.rs +++ b/pbs-datastore/src/lib.rs @@ -199,7 +199,7 @@ pub use chunk_store::ChunkStore; pub use chunker::Chunker; pub use crypt_reader::CryptReader; pub use crypt_writer::CryptWriter; -pub use data_blob::DataBlob; +pub use data_blob::{Compression, DataBlob}; pub use data_blob_reader::DataBlobReader; pub use data_blob_writer::DataBlobWriter; pub use manifest::BackupManifest; diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs index b3047308..836451a9 100644 --- a/proxmox-backup-client/src/benchmark.rs +++ b/proxmox-backup-client/src/benchmark.rs @@ -18,6 +18,7 @@ use pbs_api_types::{BackupNamespace, BackupType}; use pbs_client::tools::key_source::get_encryption_key_password; use pbs_client::{BackupRepository, BackupWriter}; use pbs_datastore::data_blob::{DataBlob, DataChunkBuilder}; +use pbs_datastore::Compression; use pbs_key_config::{load_and_decrypt_key, KeyDerivationConfig}; use pbs_tools::crypt_config::CryptConfig; @@ -346,7 +347,9 @@ fn test_crypt_speed(benchmark_result: &mut BenchmarkResult) -> Result<(), Error> let start_time = std::time::Instant::now(); - let (chunk, digest) = DataChunkBuilder::new(&random_data).compress(true).build()?; + let (chunk, digest) = DataChunkBuilder::new(&random_data) + .compression(Compression::default()) + .build()?; let mut bytes = 0; loop { diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs index 1a13291a..e5d67c7d 100644 --- a/proxmox-backup-client/src/main.rs +++ b/proxmox-backup-client/src/main.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use std::pin::Pin; use std::sync::{Arc, Mutex}; use std::task::Context; +use std::str::FromStr; use anyhow::{bail, format_err, Error}; use futures::stream::{StreamExt, TryStreamExt}; @@ -57,7 +58,7 @@ use pbs_datastore::manifest::{ archive_type, ArchiveType, BackupManifest, ENCRYPTED_KEY_BLOB_NAME, MANIFEST_BLOB_NAME, }; use pbs_datastore::read_chunk::AsyncReadChunk; -use pbs_datastore::CATALOG_NAME; +use pbs_datastore::{Compression, CATALOG_NAME}; use pbs_key_config::{decrypt_key, rsa_encrypt_key_config, KeyConfig}; use pbs_tools::crypt_config::CryptConfig; use pbs_tools::json; @@ -539,7 +540,7 @@ fn spawn_catalog_upload( let upload_options = UploadOptions { encrypt, - compress: true, + compression: Compression::default(), ..UploadOptions::default() }; @@ -947,7 +948,7 @@ async fn create_backup( // no dry-run (BackupSpecificationType::CONFIG, false) => { let upload_options = UploadOptions { - compress: true, + compression: Compression::default(), encrypt: crypto.mode == CryptMode::Encrypt, ..UploadOptions::default() }; @@ -961,7 +962,7 @@ async fn create_backup( (BackupSpecificationType::LOGFILE, false) => { // fixme: remove - not needed anymore ? let upload_options = UploadOptions { - compress: true, + compression: Compression::default(), encrypt: crypto.mode == CryptMode::Encrypt, ..UploadOptions::default() }; @@ -997,7 +998,7 @@ async fn create_backup( let upload_options = UploadOptions { previous_manifest: previous_manifest.clone(), - compress: true, + compression: Compression::default(), encrypt: crypto.mode == CryptMode::Encrypt, ..UploadOptions::default() }; @@ -1021,7 +1022,7 @@ async fn create_backup( let upload_options = UploadOptions { previous_manifest: previous_manifest.clone(), fixed_size: Some(size), - compress: true, + compression: Compression::default(), encrypt: crypto.mode == CryptMode::Encrypt, }; @@ -1058,7 +1059,7 @@ async fn create_backup( let target = ENCRYPTED_KEY_BLOB_NAME; log::info!("Upload RSA encoded key to '{}' as {}", repo, target); let options = UploadOptions { - compress: false, + compression: Compression::None, encrypt: false, ..UploadOptions::default() }; @@ -1076,7 +1077,7 @@ async fn create_backup( log::debug!("Upload index.json to '{}'", repo); let options = UploadOptions { - compress: true, + compression: Compression::default(), encrypt: false, ..UploadOptions::default() }; diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs index f195c23b..bd5271de 100644 --- a/proxmox-backup-client/src/snapshot.rs +++ b/proxmox-backup-client/src/snapshot.rs @@ -9,7 +9,7 @@ use proxmox_sys::fs::file_get_contents; use pbs_api_types::{BackupGroup, BackupNamespace, CryptMode, SnapshotListItem}; use pbs_client::tools::key_source::get_encryption_key_password; -use pbs_datastore::DataBlob; +use pbs_datastore::{Compression, DataBlob}; use pbs_key_config::decrypt_key; use pbs_tools::crypt_config::CryptConfig; use pbs_tools::json::required_string_param; @@ -257,10 +257,14 @@ async fn upload_log(param: Value) -> Result { // fixme: howto sign log? let blob = match crypto.mode { - CryptMode::None | CryptMode::SignOnly => DataBlob::encode(&data, None, true)?, - CryptMode::Encrypt => { - DataBlob::encode(&data, crypt_config.as_ref().map(Arc::as_ref), true)? + CryptMode::None | CryptMode::SignOnly => { + DataBlob::encode(&data, None, Compression::default())? } + CryptMode::Encrypt => DataBlob::encode( + &data, + crypt_config.as_ref().map(Arc::as_ref), + Compression::default(), + )?, }; let raw_data = blob.into_inner(); diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs index 7b6c8978..37763af1 100644 --- a/src/api2/tape/restore.rs +++ b/src/api2/tape/restore.rs @@ -27,7 +27,7 @@ use pbs_datastore::dynamic_index::DynamicIndexReader; use pbs_datastore::fixed_index::FixedIndexReader; use pbs_datastore::index::IndexFile; use pbs_datastore::manifest::{archive_type, ArchiveType, BackupManifest, MANIFEST_BLOB_NAME}; -use pbs_datastore::{DataBlob, DataStore}; +use pbs_datastore::{Compression, DataBlob, DataStore}; use pbs_tape::{ BlockReadError, MediaContentHeader, TapeRead, PROXMOX_BACKUP_CONTENT_HEADER_MAGIC_1_0, }; @@ -1782,7 +1782,7 @@ fn try_restore_snapshot_archive( .map(|m| m.remove("verify_state")); let old_manifest = serde_json::to_string_pretty(&old_manifest)?; - let blob = DataBlob::encode(old_manifest.as_bytes(), None, true)?; + let blob = DataBlob::encode(old_manifest.as_bytes(), None, Compression::default())?; let options = CreateOptions::new(); replace_file(&tmp_path, blob.raw_data(), options, false)?; diff --git a/src/bin/proxmox_backup_debug/recover.rs b/src/bin/proxmox_backup_debug/recover.rs index ccac476b..0fdbbf98 100644 --- a/src/bin/proxmox_backup_debug/recover.rs +++ b/src/bin/proxmox_backup_debug/recover.rs @@ -12,7 +12,7 @@ use pbs_datastore::dynamic_index::DynamicIndexReader; use pbs_datastore::file_formats::{DYNAMIC_SIZED_CHUNK_INDEX_1_0, FIXED_SIZED_CHUNK_INDEX_1_0}; use pbs_datastore::fixed_index::FixedIndexReader; use pbs_datastore::index::IndexFile; -use pbs_datastore::DataBlob; +use pbs_datastore::{Compression, DataBlob}; use pbs_key_config::load_and_decrypt_key; use pbs_tools::crypt_config::CryptConfig; @@ -125,7 +125,11 @@ fn recover_index( eprintln!("WARN: replacing output file {:?} with '\\0'", info.range,); Ok(( - DataBlob::encode(&vec![0; size as usize], crypt_conf_opt.as_ref(), true)?, + DataBlob::encode( + &vec![0; size as usize], + crypt_conf_opt.as_ref(), + Compression::default(), + )?, None, )) }; -- 2.39.2