From 0832eb853fd0539423b41f1586ce2b08f2c8f7d2 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Wed, 27 Mar 2024 08:24:44 +0100 Subject: [PATCH 1/8] Add possibility to compress log files with Zstandard --- Cargo.toml | 2 + .../policy/compound/roll/fixed_window.rs | 70 ++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 9f648269..170a5cde 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ all_components = [ ] gzip = ["flate2"] +zstandard = ["zstd"] [[bench]] name = "rotation" @@ -58,6 +59,7 @@ harness = false arc-swap = "1.6" chrono = { version = "0.4.23", optional = true, features = ["clock"], default-features = false } flate2 = { version = "1.0", optional = true } +zstd = { version = "0.13", optional = true } fnv = "1.0" humantime = { version = "2.1", optional = true } log = { version = "0.4.20", features = ["std"] } diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 340ddd05..78d3b321 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -31,6 +31,8 @@ enum Compression { None, #[cfg(feature = "gzip")] Gzip, + #[cfg(feature = "zstandard")] + Zstd, } impl Compression { @@ -52,6 +54,19 @@ impl Compression { drop(o.finish()?); drop(i); // needs to happen before remove_file call on Windows + fs::remove_file(src) + }, + #[cfg(feature = "zstandard")] + Compression::Zstd => { + use std::fs::File; + let mut i = File::open(src)?; + let mut o = { + let target = File::create(dst)?; + zstd::Encoder::new(target, zstd::DEFAULT_COMPRESSION_LEVEL)? + }; + io::copy(&mut i, &mut o)?; + drop(o.finish()?); + drop(i); fs::remove_file(src) } } @@ -274,7 +289,13 @@ impl FixedWindowRollerBuilder { #[cfg(not(feature = "gzip"))] Some(e) if e == "gz" => { bail!("gzip compression requires the `gzip` feature"); - } + }, + #[cfg(feature = "zstandard")] + Some(e) if e == "zst" => Compression::Zstd, + #[cfg(not(feature = "zstandard"))] + Some(e) if e == "zst" => { + bail!("zstd compression requires the `zstandard` feature"); + }, _ => Compression::None, }; @@ -560,6 +581,53 @@ mod test { assert_eq!(contents, actual); } + #[test] + #[cfg_attr(feature = "zstandard", ignore)] + fn unsupported_zstd() { + let dir = tempfile::tempdir().unwrap(); + + let pattern = dir.path().join("{}.zst"); + assert!(FixedWindowRoller::builder() + .build(pattern.to_str().unwrap(), 2) + .is_err()); + } + + #[test] + #[cfg_attr(not(feature = "zstandard"), ignore)] + // or should we force windows user to install zstd + #[cfg(not(windows))] + fn supported_zstd() { + use std::process::Command; + + let dir = tempfile::tempdir().unwrap(); + + let pattern = dir.path().join("{}.zst"); + let roller = FixedWindowRoller::builder() + .build(pattern.to_str().unwrap(), 2) + .unwrap(); + + let contents = (0..10000).map(|i| i as u8).collect::>(); + + let file = dir.path().join("foo.log"); + File::create(&file).unwrap().write_all(&contents).unwrap(); + + roller.roll(&file).unwrap(); + wait_for_roller(&roller); + + assert!(Command::new("zstd") + .arg("-d") + .arg(dir.path().join("0.zst")) + .status() + .unwrap() + .success()); + + let mut file = File::open(dir.path().join("0")).unwrap(); + let mut actual = vec![]; + file.read_to_end(&mut actual).unwrap(); + + assert_eq!(contents, actual); + } + #[test] fn roll_with_env_var() { std::env::set_var("LOG_DIR", "test_log_dir"); From f3da15c17b539c2c06b5932fe0de51de534055fc Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Sat, 30 Mar 2024 17:02:58 +0100 Subject: [PATCH 2/8] add zstd documentation and benchmark --- README.md | 3 ++- benches/rotation.rs | 13 +++++++++---- docs/Configuration.md | 3 +++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c35baf4d..c9881671 100644 --- a/README.md +++ b/README.md @@ -73,11 +73,12 @@ substantial performance issue with the `gzip` feature. When rolling files it will zip log archives automatically. This is a problem when the log archives are large as the zip happens in the main thread and will halt the process while the zip is completed. +You might face the same issue also with the `zstandard` feature. The methods to mitigate this are as follows. 1. Use the `background_rotation` feature which spawns an os thread to do the compression. -2. Do not enable the `gzip` feature. +2. Do not enable neither the `gzip` nor the `zstandard` feature. 3. Ensure the archives are small enough that the compression time is acceptable. For more information see the PR that added [`background_rotation`](https://github.com/estk/log4rs/pull/117). diff --git a/benches/rotation.rs b/benches/rotation.rs index 4167331f..f2a92be1 100644 --- a/benches/rotation.rs +++ b/benches/rotation.rs @@ -60,10 +60,15 @@ fn mk_config(file_size: u64, file_count: u32) -> log4rs::config::Config { let log_path = LOGDIR.path(); let log_pattern = log_path.join("log.log"); - #[cfg(feature = "gzip")] - let roll_pattern = format!("{}/{}", log_path.to_string_lossy(), "log.{}.gz"); - #[cfg(not(feature = "gzip"))] - let roll_pattern = format!("{}/{}", log_path.to_string_lossy(), "log.{}"); + let roll_pattern = { + if cfg!(feature = "gzip") { + format!("{}/{}", log_path.to_string_lossy(), "log.{}.gz") + } else if cfg!(feature = "zstandard") { + format!("{}/{}", log_path.to_string_lossy(), "log.{}.zst") + } else { + format!("{}/{}", log_path.to_string_lossy(), "log.{}") + } + }; use log::LevelFilter; use log4rs::{ diff --git a/docs/Configuration.md b/docs/Configuration.md index 5ec2ce65..515c4060 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -243,6 +243,9 @@ double curly brace `{}`. For example `archive/foo.{}.log`. Each instance of `{}` will be replaced with the index number of the configuration file. Note that if the file extension of the pattern is `.gz` and the `gzip` Cargo feature is enabled, the archive files will be gzip-compressed. +If the file extension of the pattern is `.zst` and the `zstandard` Cargo +feature is enabled, the archive files will be compressed the +[Zstandard](https://facebook.github.io/zstd/) compression algorithm. > Note: This pattern field is only used for archived files. The `path` field > of the higher level `rolling_file` will be used for the active log file. From 659a5e3d85850799ddf8592a91dd762e919bab74 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Sat, 30 Mar 2024 17:15:32 +0100 Subject: [PATCH 3/8] check we hit the error we expect when zstandard feature is disabled --- .../rolling_file/policy/compound/roll/fixed_window.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 78d3b321..1d6ab37a 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -587,9 +587,13 @@ mod test { let dir = tempfile::tempdir().unwrap(); let pattern = dir.path().join("{}.zst"); - assert!(FixedWindowRoller::builder() - .build(pattern.to_str().unwrap(), 2) - .is_err()); + let roller = FixedWindowRoller::builder() + .build(pattern.to_str().unwrap(), 2); + assert!(roller.is_err()); + assert!(roller + .unwrap_err() + .to_string() + .contains("zstd compression requires the `zstandard` feature")); } #[test] From fd5d842b1a75e3200513dba5b08961437ace5fc8 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Mon, 1 Apr 2024 10:23:20 +0200 Subject: [PATCH 4/8] review documentation --- README.md | 12 ++++++------ docs/Configuration.md | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index c9881671..dd7b4c72 100644 --- a/README.md +++ b/README.md @@ -69,16 +69,16 @@ fn main() { ## Compression If you are using the file rotation in your configuration there is a known -substantial performance issue with the `gzip` feature. When rolling files -it will zip log archives automatically. This is a problem when the log archives -are large as the zip happens in the main thread and will halt the process while -the zip is completed. -You might face the same issue also with the `zstandard` feature. +substantial performance issue with either the `gzip` or `zstandard` +features. When rolling files it will zip log archives automatically. This is +a problem when the log archives are large as the zip process occurs in +the main thread and will halt the process until the zip process +completes. The methods to mitigate this are as follows. 1. Use the `background_rotation` feature which spawns an os thread to do the compression. -2. Do not enable neither the `gzip` nor the `zstandard` feature. +2. Do not enable the `gzip` nor the `zstandard` features. 3. Ensure the archives are small enough that the compression time is acceptable. For more information see the PR that added [`background_rotation`](https://github.com/estk/log4rs/pull/117). diff --git a/docs/Configuration.md b/docs/Configuration.md index 515c4060..c726ee10 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -244,7 +244,7 @@ double curly brace `{}`. For example `archive/foo.{}.log`. Each instance of that if the file extension of the pattern is `.gz` and the `gzip` Cargo feature is enabled, the archive files will be gzip-compressed. If the file extension of the pattern is `.zst` and the `zstandard` Cargo -feature is enabled, the archive files will be compressed the +feature is enabled, the archive files will be compressed using the [Zstandard](https://facebook.github.io/zstd/) compression algorithm. > Note: This pattern field is only used for archived files. The `path` field From bd712a062e9d2bf26f6415f17decb7a4d9eb684b Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Tue, 2 Apr 2024 08:22:46 +0200 Subject: [PATCH 5/8] change feature name from zstandard to zstd --- Cargo.toml | 2 +- README.md | 4 ++-- benches/rotation.rs | 2 +- docs/Configuration.md | 2 +- .../policy/compound/roll/fixed_window.rs | 16 ++++++++-------- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 170a5cde..91dbf579 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ all_components = [ ] gzip = ["flate2"] -zstandard = ["zstd"] +zstd = ["dep:zstd"] [[bench]] name = "rotation" diff --git a/README.md b/README.md index dd7b4c72..3b2cb368 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ fn main() { ## Compression If you are using the file rotation in your configuration there is a known -substantial performance issue with either the `gzip` or `zstandard` +substantial performance issue with either the `gzip` or `zstd` features. When rolling files it will zip log archives automatically. This is a problem when the log archives are large as the zip process occurs in the main thread and will halt the process until the zip process @@ -78,7 +78,7 @@ completes. The methods to mitigate this are as follows. 1. Use the `background_rotation` feature which spawns an os thread to do the compression. -2. Do not enable the `gzip` nor the `zstandard` features. +2. Do not enable the `gzip` nor the `zstd` features. 3. Ensure the archives are small enough that the compression time is acceptable. For more information see the PR that added [`background_rotation`](https://github.com/estk/log4rs/pull/117). diff --git a/benches/rotation.rs b/benches/rotation.rs index f2a92be1..d647cfd1 100644 --- a/benches/rotation.rs +++ b/benches/rotation.rs @@ -63,7 +63,7 @@ fn mk_config(file_size: u64, file_count: u32) -> log4rs::config::Config { let roll_pattern = { if cfg!(feature = "gzip") { format!("{}/{}", log_path.to_string_lossy(), "log.{}.gz") - } else if cfg!(feature = "zstandard") { + } else if cfg!(feature = "zstd") { format!("{}/{}", log_path.to_string_lossy(), "log.{}.zst") } else { format!("{}/{}", log_path.to_string_lossy(), "log.{}") diff --git a/docs/Configuration.md b/docs/Configuration.md index c726ee10..57906bbb 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -243,7 +243,7 @@ double curly brace `{}`. For example `archive/foo.{}.log`. Each instance of `{}` will be replaced with the index number of the configuration file. Note that if the file extension of the pattern is `.gz` and the `gzip` Cargo feature is enabled, the archive files will be gzip-compressed. -If the file extension of the pattern is `.zst` and the `zstandard` Cargo +If the file extension of the pattern is `.zst` and the `zstd` Cargo feature is enabled, the archive files will be compressed using the [Zstandard](https://facebook.github.io/zstd/) compression algorithm. diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 1d6ab37a..46ce14e7 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -31,7 +31,7 @@ enum Compression { None, #[cfg(feature = "gzip")] Gzip, - #[cfg(feature = "zstandard")] + #[cfg(feature = "zstd")] Zstd, } @@ -56,7 +56,7 @@ impl Compression { fs::remove_file(src) }, - #[cfg(feature = "zstandard")] + #[cfg(feature = "zstd")] Compression::Zstd => { use std::fs::File; let mut i = File::open(src)?; @@ -290,11 +290,11 @@ impl FixedWindowRollerBuilder { Some(e) if e == "gz" => { bail!("gzip compression requires the `gzip` feature"); }, - #[cfg(feature = "zstandard")] + #[cfg(feature = "zstd")] Some(e) if e == "zst" => Compression::Zstd, - #[cfg(not(feature = "zstandard"))] + #[cfg(not(feature = "zstd"))] Some(e) if e == "zst" => { - bail!("zstd compression requires the `zstandard` feature"); + bail!("zstd compression requires the `zstd` feature"); }, _ => Compression::None, }; @@ -582,7 +582,7 @@ mod test { } #[test] - #[cfg_attr(feature = "zstandard", ignore)] + #[cfg_attr(feature = "zstd", ignore)] fn unsupported_zstd() { let dir = tempfile::tempdir().unwrap(); @@ -593,11 +593,11 @@ mod test { assert!(roller .unwrap_err() .to_string() - .contains("zstd compression requires the `zstandard` feature")); + .contains("zstd compression requires the `zstd` feature")); } #[test] - #[cfg_attr(not(feature = "zstandard"), ignore)] + #[cfg_attr(not(feature = "zstd"), ignore)] // or should we force windows user to install zstd #[cfg(not(windows))] fn supported_zstd() { From f0cc78de2e7760020c60ac4fbe51d08944ec18c7 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Tue, 2 Apr 2024 10:20:45 +0200 Subject: [PATCH 6/8] use zstd library instead of zstd command in test --- .../policy/compound/roll/fixed_window.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 46ce14e7..6dd71c33 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -598,11 +598,7 @@ mod test { #[test] #[cfg_attr(not(feature = "zstd"), ignore)] - // or should we force windows user to install zstd - #[cfg(not(windows))] fn supported_zstd() { - use std::process::Command; - let dir = tempfile::tempdir().unwrap(); let pattern = dir.path().join("{}.zst"); @@ -618,16 +614,8 @@ mod test { roller.roll(&file).unwrap(); wait_for_roller(&roller); - assert!(Command::new("zstd") - .arg("-d") - .arg(dir.path().join("0.zst")) - .status() - .unwrap() - .success()); - - let mut file = File::open(dir.path().join("0")).unwrap(); - let mut actual = vec![]; - file.read_to_end(&mut actual).unwrap(); + let compressed_data = fs::read(dir.path().join("0.zst")).unwrap(); + let actual = zstd::decode_all(compressed_data.as_slice()).unwrap(); assert_eq!(contents, actual); } From ffbb070848191b64212e2008ca9b104a40948281 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Tue, 2 Apr 2024 10:27:40 +0200 Subject: [PATCH 7/8] do not try to compile zstd test if feature is not enabled --- src/append/rolling_file/policy/compound/roll/fixed_window.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 6dd71c33..0158b06b 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -597,7 +597,7 @@ mod test { } #[test] - #[cfg_attr(not(feature = "zstd"), ignore)] + #[cfg(feature = "zstd")] fn supported_zstd() { let dir = tempfile::tempdir().unwrap(); From 8c7cade4edff1dbb25e3a4a432ff25f1828d74f2 Mon Sep 17 00:00:00 2001 From: Cristiano Prato Date: Wed, 17 Jul 2024 12:20:25 +0200 Subject: [PATCH 8/8] fix linter errors --- .../rolling_file/policy/compound/roll/fixed_window.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/append/rolling_file/policy/compound/roll/fixed_window.rs b/src/append/rolling_file/policy/compound/roll/fixed_window.rs index 0158b06b..40fc3222 100644 --- a/src/append/rolling_file/policy/compound/roll/fixed_window.rs +++ b/src/append/rolling_file/policy/compound/roll/fixed_window.rs @@ -55,7 +55,7 @@ impl Compression { drop(i); // needs to happen before remove_file call on Windows fs::remove_file(src) - }, + } #[cfg(feature = "zstd")] Compression::Zstd => { use std::fs::File; @@ -289,13 +289,13 @@ impl FixedWindowRollerBuilder { #[cfg(not(feature = "gzip"))] Some(e) if e == "gz" => { bail!("gzip compression requires the `gzip` feature"); - }, + } #[cfg(feature = "zstd")] Some(e) if e == "zst" => Compression::Zstd, #[cfg(not(feature = "zstd"))] Some(e) if e == "zst" => { bail!("zstd compression requires the `zstd` feature"); - }, + } _ => Compression::None, }; @@ -587,8 +587,7 @@ mod test { let dir = tempfile::tempdir().unwrap(); let pattern = dir.path().join("{}.zst"); - let roller = FixedWindowRoller::builder() - .build(pattern.to_str().unwrap(), 2); + let roller = FixedWindowRoller::builder().build(pattern.to_str().unwrap(), 2); assert!(roller.is_err()); assert!(roller .unwrap_err()