From 05fb00bb858b65ba7631486ff885451509554730 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:02:08 +0100 Subject: [PATCH 01/18] refactor: use a `DummyCardanoDbBuilder` to make the creation of the Cardano database directory more consistent. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/artifact_builder/cardano_database.rs | 6 +- mithril-aggregator/src/snapshotter.rs | 6 +- mithril-client/tests/extensions/fake.rs | 16 +-- .../snapshot_list_get_show_download_verify.rs | 6 +- .../digesters/cardano_immutable_digester.rs | 82 ++++++------ ...able_db_builder.rs => dummy_cardano_db.rs} | 122 ++++++++++++------ mithril-common/src/digesters/mod.rs | 4 +- .../src/bin/load-aggregator/main.rs | 16 +-- .../src/stress_test/payload_builder.rs | 10 +- 9 files changed, 154 insertions(+), 114 deletions(-) rename mithril-common/src/digesters/{dummy_immutable_db_builder.rs => dummy_cardano_db.rs} (73%) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database.rs b/mithril-aggregator/src/artifact_builder/cardano_database.rs index 7f6cd988699..fdb3a86bb67 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database.rs @@ -111,7 +111,7 @@ mod tests { use std::path::PathBuf; use mithril_common::{ - digesters::DummyImmutablesDbBuilder, + digesters::DummyCardanoDbBuilder, entities::{ProtocolMessage, ProtocolMessagePartKey}, test_utils::{fake_data, TempDir}, }; @@ -129,7 +129,7 @@ mod tests { let immutable_trio_file_size = 777; let ledger_file_size = 6666; let volatile_file_size = 99; - DummyImmutablesDbBuilder::new(test_dir.as_os_str().to_str().unwrap()) + DummyCardanoDbBuilder::new(test_dir.as_os_str().to_str().unwrap()) .with_immutables(&[1, 2]) .set_immutable_trio_file_size(immutable_trio_file_size) .with_ledger_files(&["blocks-0.dat", "blocks-1.dat", "blocks-2.dat"]) @@ -152,7 +152,7 @@ mod tests { let immutable_trio_file_size = 777; let ledger_file_size = 6666; let volatile_file_size = 99; - DummyImmutablesDbBuilder::new(test_dir.as_os_str().to_str().unwrap()) + DummyCardanoDbBuilder::new(test_dir.as_os_str().to_str().unwrap()) .with_immutables(&[1]) .set_immutable_trio_file_size(immutable_trio_file_size) .with_ledger_files(&["blocks-0.dat"]) diff --git a/mithril-aggregator/src/snapshotter.rs b/mithril-aggregator/src/snapshotter.rs index a917d979fc4..9af9c0de897 100644 --- a/mithril-aggregator/src/snapshotter.rs +++ b/mithril-aggregator/src/snapshotter.rs @@ -495,7 +495,7 @@ mod tests { use uuid::Uuid; - use mithril_common::{digesters::DummyImmutablesDbBuilder, test_utils::TempDir}; + use mithril_common::{digesters::DummyCardanoDbBuilder, test_utils::TempDir}; use crate::test_tools::TestLogger; @@ -658,7 +658,7 @@ mod tests { let pending_snapshot_archive_file = "archive.tar.gz"; let db_directory = test_dir.join("db"); - DummyImmutablesDbBuilder::new(db_directory.as_os_str().to_str().unwrap()) + DummyCardanoDbBuilder::new(db_directory.as_os_str().to_str().unwrap()) .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); @@ -699,7 +699,7 @@ mod tests { let pending_snapshot_archive_file = "archive.tar.zst"; let db_directory = test_dir.join("db"); - DummyImmutablesDbBuilder::new(db_directory.as_os_str().to_str().unwrap()) + DummyCardanoDbBuilder::new(db_directory.as_os_str().to_str().unwrap()) .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); diff --git a/mithril-client/tests/extensions/fake.rs b/mithril-client/tests/extensions/fake.rs index c4bb7746ade..72fb9812438 100644 --- a/mithril-client/tests/extensions/fake.rs +++ b/mithril-client/tests/extensions/fake.rs @@ -195,7 +195,7 @@ mod proof { mod file { use super::*; use mithril_client::{MessageBuilder, Snapshot, SnapshotListItem}; - use mithril_common::digesters::DummyImmutableDb; + use mithril_common::digesters::DummyCardanoDb; use mithril_common::entities::{CardanoDbBeacon, CompressionAlgorithm}; use mithril_common::messages::{CardanoDbBeaconMessagePart, SignedEntityTypeMessagePart}; use mithril_common::test_utils::fake_data; @@ -210,11 +210,11 @@ mod file { &self, snapshot_digest: &str, certificate_hash: &str, - immutable_db: &DummyImmutableDb, + cardano_db: &DummyCardanoDb, work_dir: &Path, ) -> TestHttpServer { let beacon = CardanoDbBeacon { - immutable_file_number: immutable_db.last_immutable_number().unwrap(), + immutable_file_number: cardano_db.last_immutable_number().unwrap(), ..fake_data::beacon() }; // Note: network doesn't matter here and will be removed in the future @@ -254,7 +254,7 @@ mod file { ..MithrilCertificate::dummy() }; certificate.signed_message = MessageBuilder::new() - .compute_snapshot_message(&certificate, &immutable_db.dir) + .compute_snapshot_message(&certificate, cardano_db.get_immutable_dir()) .await .expect("Computing snapshot message should not fail") .compute_hash(); @@ -269,7 +269,7 @@ mod file { )) .or(routes::statistics::routes(self.calls.clone())); - let snapshot_archive_path = build_fake_zstd_snapshot(immutable_db, work_dir); + let snapshot_archive_path = build_fake_zstd_snapshot(cardano_db, work_dir); let routes = routes.or(routes::snapshot::download( self.calls.clone(), @@ -286,12 +286,12 @@ mod file { /// Compress the given db into an zstd archive in the given target directory. /// /// return the path to the compressed archive. - pub fn build_fake_zstd_snapshot(immutable_db: &DummyImmutableDb, target_dir: &Path) -> PathBuf { + pub fn build_fake_zstd_snapshot(cardano_db: &DummyCardanoDb, target_dir: &Path) -> PathBuf { use std::fs::File; let snapshot_name = format!( "db-i{}.{}", - immutable_db.immutables_files.len(), + cardano_db.get_immutable_files().len(), CompressionAlgorithm::Zstandard.tar_file_extension() ); let target_file = target_dir.join(snapshot_name); @@ -299,7 +299,7 @@ mod file { let enc = zstd::Encoder::new(tar_file, 3).unwrap(); let mut tar = tar::Builder::new(enc); - tar.append_dir_all(".", immutable_db.dir.parent().unwrap()) + tar.append_dir_all(".", cardano_db.get_immutable_dir().parent().unwrap()) .unwrap(); let zstd = tar.into_inner().unwrap(); diff --git a/mithril-client/tests/snapshot_list_get_show_download_verify.rs b/mithril-client/tests/snapshot_list_get_show_download_verify.rs index 6adfe1477f8..ab5c9153b6f 100644 --- a/mithril-client/tests/snapshot_list_get_show_download_verify.rs +++ b/mithril-client/tests/snapshot_list_get_show_download_verify.rs @@ -4,7 +4,7 @@ use crate::extensions::fake::{FakeAggregator, FakeCertificateVerifier}; use mithril_client::aggregator_client::AggregatorRequest; use mithril_client::feedback::SlogFeedbackReceiver; use mithril_client::{ClientBuilder, MessageBuilder}; -use mithril_common::digesters::DummyImmutablesDbBuilder; +use mithril_common::digesters::DummyCardanoDbBuilder; use std::sync::Arc; #[tokio::test] @@ -14,13 +14,13 @@ async fn snapshot_list_get_show_download_verify() { mithril_common::test_utils::fake_keys::genesis_verification_key()[0]; let digest = "snapshot_digest"; let certificate_hash = "certificate_hash"; - let immutable_db = DummyImmutablesDbBuilder::new("snapshot_list_get_show_download_verify_db") + let cardano_db = DummyCardanoDbBuilder::new("snapshot_list_get_show_download_verify_db") .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); let fake_aggregator = FakeAggregator::new(); let test_http_server = fake_aggregator - .spawn_with_snapshot(digest, certificate_hash, &immutable_db, &work_dir) + .spawn_with_snapshot(digest, certificate_hash, &cardano_db, &work_dir) .await; let client = ClientBuilder::aggregator(&test_http_server.url(), genesis_verification_key) .with_certificate_verifier(FakeCertificateVerifier::build_that_validate_any_certificate()) diff --git a/mithril-common/src/digesters/cardano_immutable_digester.rs b/mithril-common/src/digesters/cardano_immutable_digester.rs index d27c26d3568..0ccdd66cb59 100644 --- a/mithril-common/src/digesters/cardano_immutable_digester.rs +++ b/mithril-common/src/digesters/cardano_immutable_digester.rs @@ -259,7 +259,7 @@ mod tests { ImmutableDigesterCacheStoreError, MemoryImmutableFileDigestCacheProvider, MockImmutableFileDigestCacheProvider, }, - DummyImmutablesDbBuilder, + DummyCardanoDbBuilder, }, entities::ImmutableFileNumber, test_utils::TestLogger, @@ -267,8 +267,8 @@ mod tests { use super::*; - fn db_builder(dir_name: &str) -> DummyImmutablesDbBuilder { - DummyImmutablesDbBuilder::new(&format!("cardano_immutable_digester/{dir_name}")) + fn db_builder(dir_name: &str) -> DummyCardanoDbBuilder { + DummyCardanoDbBuilder::new(&format!("cardano_immutable_digester/{dir_name}")) } #[test] @@ -325,9 +325,9 @@ mod tests { #[tokio::test] async fn fail_if_no_file_in_folder() { - let immutable_db = db_builder("fail_if_no_file_in_folder").build(); + let cardano_db = db_builder("fail_if_no_file_in_folder").build(); - let result = list_immutable_files_to_process(&immutable_db.dir, 1) + let result = list_immutable_files_to_process(cardano_db.get_immutable_dir(), 1) .expect_err("list_immutable_files_to_process should have failed"); assert_eq!( @@ -336,7 +336,7 @@ mod tests { ImmutableDigesterError::NotEnoughImmutable { expected_number: 1, found_number: None, - db_dir: immutable_db.dir, + db_dir: cardano_db.get_immutable_dir().to_path_buf(), } ), format!("{result:?}") @@ -345,20 +345,20 @@ mod tests { #[tokio::test] async fn fail_if_a_invalid_file_is_in_immutable_folder() { - let immutable_db = db_builder("fail_if_no_immutable_exist") + let cardano_db = db_builder("fail_if_no_immutable_exist") .with_non_immutables(&["not_immutable"]) .build(); - assert!(list_immutable_files_to_process(&immutable_db.dir, 1).is_err()); + assert!(list_immutable_files_to_process(cardano_db.get_immutable_dir(), 1).is_err()); } #[tokio::test] async fn fail_if_theres_only_the_uncompleted_immutable_trio() { - let immutable_db = db_builder("fail_if_theres_only_the_uncompleted_immutable_trio") + let cardano_db = db_builder("fail_if_theres_only_the_uncompleted_immutable_trio") .append_immutable_trio() .build(); - let result = list_immutable_files_to_process(&immutable_db.dir, 1) + let result = list_immutable_files_to_process(cardano_db.get_immutable_dir(), 1) .expect_err("list_immutable_files_to_process should've failed"); assert_eq!( @@ -367,7 +367,7 @@ mod tests { ImmutableDigesterError::NotEnoughImmutable { expected_number: 1, found_number: None, - db_dir: immutable_db.dir, + db_dir: cardano_db.get_immutable_dir().to_path_buf(), } ), format!("{result:?}") @@ -376,12 +376,12 @@ mod tests { #[tokio::test] async fn fail_if_less_immutable_than_what_required_in_beacon() { - let immutable_db = db_builder("fail_if_less_immutable_than_what_required_in_beacon") + let cardano_db = db_builder("fail_if_less_immutable_than_what_required_in_beacon") .with_immutables(&[1, 2, 3, 4, 5]) .append_immutable_trio() .build(); - let result = list_immutable_files_to_process(&immutable_db.dir, 10) + let result = list_immutable_files_to_process(cardano_db.get_immutable_dir(), 10) .expect_err("list_immutable_files_to_process should've failed"); assert_eq!( @@ -390,7 +390,7 @@ mod tests { ImmutableDigesterError::NotEnoughImmutable { expected_number: 10, found_number: Some(5), - db_dir: immutable_db.dir, + db_dir: cardano_db.get_immutable_dir().to_path_buf(), } ), format!("{result:?}") @@ -399,7 +399,7 @@ mod tests { #[tokio::test] async fn can_compute_merkle_tree_of_a_hundred_immutable_file_trio() { - let immutable_db = db_builder("can_compute_merkle_tree_of_a_hundred_immutable_file_trio") + let cardano_db = db_builder("can_compute_merkle_tree_of_a_hundred_immutable_file_trio") .with_immutables(&(1..=100).collect::>()) .append_immutable_trio() .build(); @@ -412,7 +412,7 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 100); let result = digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail"); @@ -426,7 +426,7 @@ mod tests { #[tokio::test] async fn can_compute_hash_of_a_hundred_immutable_file_trio() { - let immutable_db = db_builder("can_compute_hash_of_a_hundred_immutable_file_trio") + let cardano_db = db_builder("can_compute_hash_of_a_hundred_immutable_file_trio") .with_immutables(&(1..=100).collect::>()) .append_immutable_trio() .build(); @@ -439,7 +439,7 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 100); let result = digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); @@ -451,11 +451,11 @@ mod tests { #[tokio::test] async fn compute_digest_store_digests_into_cache_provider() { - let immutable_db = db_builder("compute_digest_store_digests_into_cache_provider") + let cardano_db = db_builder("compute_digest_store_digests_into_cache_provider") .with_immutables(&[1, 2]) .append_immutable_trio() .build(); - let immutables = immutable_db.immutables_files; + let immutables = cardano_db.get_immutable_files().clone(); let cache = Arc::new(MemoryImmutableFileDigestCacheProvider::default()); let logger = TestLogger::stdout(); let digester = CardanoImmutableDigester::new( @@ -466,7 +466,7 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 2); digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); @@ -487,11 +487,11 @@ mod tests { #[tokio::test] async fn compute_merkle_tree_store_digests_into_cache_provider() { - let immutable_db = db_builder("compute_merkle_tree_store_digests_into_cache_provider") + let cardano_db = db_builder("compute_merkle_tree_store_digests_into_cache_provider") .with_immutables(&[1, 2]) .append_immutable_trio() .build(); - let immutables = immutable_db.immutables_files; + let immutables = cardano_db.get_immutable_files().clone(); let cache = Arc::new(MemoryImmutableFileDigestCacheProvider::default()); let logger = TestLogger::stdout(); let digester = CardanoImmutableDigester::new( @@ -502,7 +502,7 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 2); digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); @@ -523,7 +523,7 @@ mod tests { #[tokio::test] async fn computed_digest_with_cold_or_hot_or_without_any_cache_are_equals() { - let immutable_db = DummyImmutablesDbBuilder::new( + let cardano_db = DummyCardanoDbBuilder::new( "computed_digest_with_cold_or_hot_or_without_any_cache_are_equals", ) .with_immutables(&[1, 2, 3]) @@ -540,17 +540,17 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 3); let without_cache_digest = no_cache_digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); let cold_cache_digest = cache_digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); let full_cache_digest = cache_digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); @@ -567,7 +567,7 @@ mod tests { #[tokio::test] async fn computed_merkle_tree_with_cold_or_hot_or_without_any_cache_are_equals() { - let immutable_db = DummyImmutablesDbBuilder::new( + let cardano_db = DummyCardanoDbBuilder::new( "computed_merkle_tree_with_cold_or_hot_or_without_any_cache_are_equals", ) .with_immutables(&[1, 2, 3]) @@ -584,17 +584,17 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 3); let without_cache_digest = no_cache_digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail"); let cold_cache_digest = cache_digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail"); let full_cache_digest = cache_digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail"); @@ -614,7 +614,7 @@ mod tests { #[tokio::test] async fn hash_computation_is_quicker_with_a_full_cache() { - let immutable_db = db_builder("hash_computation_is_quicker_with_a_full_cache") + let cardano_db = db_builder("hash_computation_is_quicker_with_a_full_cache") .with_immutables(&(1..=50).collect::>()) .append_immutable_trio() .set_immutable_trio_file_size(65538) @@ -630,14 +630,14 @@ mod tests { let now = Instant::now(); digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); let elapsed_without_cache = now.elapsed(); let now = Instant::now(); digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail"); let elapsed_with_cache = now.elapsed(); @@ -655,7 +655,7 @@ mod tests { #[tokio::test] async fn cache_read_failure_dont_block_computations() { - let immutable_db = db_builder("cache_read_failure_dont_block_computation") + let cardano_db = db_builder("cache_read_failure_dont_block_computation") .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); @@ -675,19 +675,19 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 3); digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail even with cache write failure"); digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail even with cache write failure"); } #[tokio::test] async fn cache_write_failure_dont_block_computation() { - let immutable_db = db_builder("cache_write_failure_dont_block_computation") + let cardano_db = db_builder("cache_write_failure_dont_block_computation") .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); @@ -707,12 +707,12 @@ mod tests { let beacon = CardanoDbBeacon::new(1, 3); digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_digest must not fail even with cache read failure"); digester - .compute_merkle_tree(&immutable_db.dir, &beacon) + .compute_merkle_tree(cardano_db.get_immutable_dir(), &beacon) .await .expect("compute_merkle_tree must not fail even with cache read failure"); } diff --git a/mithril-common/src/digesters/dummy_immutable_db_builder.rs b/mithril-common/src/digesters/dummy_cardano_db.rs similarity index 73% rename from mithril-common/src/digesters/dummy_immutable_db_builder.rs rename to mithril-common/src/digesters/dummy_cardano_db.rs index 9e5ca6d750f..cbfe390e6cc 100644 --- a/mithril-common/src/digesters/dummy_immutable_db_builder.rs +++ b/mithril-common/src/digesters/dummy_cardano_db.rs @@ -6,31 +6,21 @@ use std::{ path::{Path, PathBuf}, }; -const IMMUTABLE_DIR: &str = "immutable"; -const LEDGER_DIR: &str = "ledger"; -const VOLATILE_DIR: &str = "volatile"; - -/// A [DummyImmutableDb] builder. -pub struct DummyImmutablesDbBuilder { - dir: PathBuf, - immutables_to_write: Vec, - non_immutables_to_write: Vec, - append_uncompleted_trio: bool, - immutable_file_size: Option, - ledger_files_to_write: Vec, - ledger_file_size: Option, - volatile_files_to_write: Vec, - volatile_file_size: Option, -} +/// Directory name for the immutable files. +pub const IMMUTABLE_DIR: &str = "immutable"; +/// Directory name for the ledger files. +pub const LEDGER_DIR: &str = "ledger"; +/// Directory name for the volatile files. +pub const VOLATILE_DIR: &str = "volatile"; /// A dummy cardano immutable db. -pub struct DummyImmutableDb { +struct DummyImmutableDb { /// The dummy cardano db directory path. - pub dir: PathBuf, + dir: PathBuf, /// The [immutables files][ImmutableFile] in the dummy cardano db. - pub immutables_files: Vec, + immutables_files: Vec, /// Files that doesn't follow the immutable file name scheme in the dummy cardano db. - pub non_immutables_files: Vec, + non_immutables_files: Vec, } impl DummyImmutableDb { @@ -50,12 +40,66 @@ impl DummyImmutableDb { } } -impl DummyImmutablesDbBuilder { - /// [DummyImmutablesDbBuilder] factory, will create a folder with the given `dirname` in the +/// A dummy cardano db. +pub struct DummyCardanoDb { + /// The dummy cardano db directory path. + dir: PathBuf, + + /// Dummy immutable db + immutable_db: DummyImmutableDb, +} + +impl DummyCardanoDb { + /// Return the cardano db directory path. + pub fn get_dir(&self) -> &PathBuf { + &self.dir + } + + /// Return the immutable db directory path. + pub fn get_immutable_dir(&self) -> &PathBuf { + &self.immutable_db.dir + } + + /// Return the file number of the last immutable + pub fn get_immutable_files(&self) -> &Vec { + &self.immutable_db.immutables_files + } + + /// Add an immutable chunk file and its primary & secondary to the dummy DB. + pub fn add_immutable_file(&mut self) -> ImmutableFileNumber { + self.immutable_db.add_immutable_file() + } + + /// Return the file number of the last immutable + pub fn last_immutable_number(&self) -> Option { + self.immutable_db.last_immutable_number() + } + + /// Return the non immutables files in the immutables directory + pub fn get_non_immutables_files(&self) -> &Vec { + &self.immutable_db.non_immutables_files + } +} + +/// A [DummyCardanoDbBuilder] builder. +pub struct DummyCardanoDbBuilder { + sub_dir: String, + immutables_to_write: Vec, + non_immutables_to_write: Vec, + append_uncompleted_trio: bool, + immutable_file_size: Option, + ledger_files_to_write: Vec, + ledger_file_size: Option, + volatile_files_to_write: Vec, + volatile_file_size: Option, +} + +impl DummyCardanoDbBuilder { + /// [DummyCardanoDbBuilder] factory, will create a folder with the given `dirname` in the /// system temp directory, if it exists already it will be cleaned. pub fn new(dir_name: &str) -> Self { Self { - dir: get_test_dir(dir_name), + sub_dir: dir_name.to_string(), immutables_to_write: vec![], non_immutables_to_write: vec![], append_uncompleted_trio: false, @@ -125,8 +169,10 @@ impl DummyImmutablesDbBuilder { self } - /// Build a [DummyImmutableDb]. - pub fn build(&self) -> DummyImmutableDb { + /// Build a [DummyCardanoDb]. + pub fn build(&self) -> DummyCardanoDb { + let dir = get_test_dir(&self.sub_dir); + let mut non_immutables_files = vec![]; let mut immutable_numbers = self.immutables_to_write.clone(); immutable_numbers.sort(); @@ -134,7 +180,7 @@ impl DummyImmutablesDbBuilder { if self.append_uncompleted_trio { write_immutable_trio( self.immutable_file_size, - &self.dir.join(IMMUTABLE_DIR), + &dir.join(IMMUTABLE_DIR), match immutable_numbers.last() { None => 0, Some(last) => last + 1, @@ -145,37 +191,31 @@ impl DummyImmutablesDbBuilder { for non_immutable in &self.non_immutables_to_write { non_immutables_files.push(write_dummy_file( self.immutable_file_size, - &self.dir.join(IMMUTABLE_DIR), + &dir.join(IMMUTABLE_DIR), non_immutable, )); } for filename in &self.ledger_files_to_write { - write_dummy_file(self.ledger_file_size, &self.dir.join(LEDGER_DIR), filename); + write_dummy_file(self.ledger_file_size, &dir.join(LEDGER_DIR), filename); } for filename in &self.volatile_files_to_write { - write_dummy_file( - self.volatile_file_size, - &self.dir.join(VOLATILE_DIR), - filename, - ); + write_dummy_file(self.volatile_file_size, &dir.join(VOLATILE_DIR), filename); } - DummyImmutableDb { - dir: self.dir.join(IMMUTABLE_DIR), + let immutable_db = DummyImmutableDb { + dir: dir.join(IMMUTABLE_DIR), immutables_files: immutable_numbers .into_iter() .flat_map(|ifn| { - write_immutable_trio( - self.immutable_file_size, - &self.dir.join(IMMUTABLE_DIR), - ifn, - ) + write_immutable_trio(self.immutable_file_size, &dir.join(IMMUTABLE_DIR), ifn) }) .collect::>(), non_immutables_files, - } + }; + + DummyCardanoDb { dir, immutable_db } } } diff --git a/mithril-common/src/digesters/mod.rs b/mithril-common/src/digesters/mod.rs index 415663341a0..917418dd73a 100644 --- a/mithril-common/src/digesters/mod.rs +++ b/mithril-common/src/digesters/mod.rs @@ -18,7 +18,7 @@ pub use immutable_file_observer::{ pub use dumb_immutable_digester::DumbImmutableDigester; cfg_test_tools! { - mod dummy_immutable_db_builder; + mod dummy_cardano_db; - pub use dummy_immutable_db_builder::{DummyImmutableDb, DummyImmutablesDbBuilder}; + pub use dummy_cardano_db::{DummyCardanoDb, DummyCardanoDbBuilder, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}; } diff --git a/mithril-test-lab/mithril-end-to-end/src/bin/load-aggregator/main.rs b/mithril-test-lab/mithril-end-to-end/src/bin/load-aggregator/main.rs index 14624be5e1d..279c79b2063 100644 --- a/mithril-test-lab/mithril-end-to-end/src/bin/load-aggregator/main.rs +++ b/mithril-test-lab/mithril-end-to-end/src/bin/load-aggregator/main.rs @@ -4,7 +4,7 @@ use std::{ops::Deref, sync::Arc, time::Duration}; use tokio::sync::oneshot; use mithril_common::{ - digesters::{DummyImmutableDb, DummyImmutablesDbBuilder}, + digesters::{DummyCardanoDb, DummyCardanoDbBuilder}, entities::{CardanoDbBeacon, Epoch, ProtocolParameters, SignedEntityType, SingleSignatures}, test_utils::MithrilFixture, StdResult, @@ -35,13 +35,13 @@ async fn main() -> StdResult<()> { let mut reporter: Reporter = Reporter::new(opts.num_signers, opts.num_clients); reporter.start("stress tests bootstrap"); // configure a dummy immutable db - let immutable_db = DummyImmutablesDbBuilder::new("load-tester") + let cardano_db = DummyCardanoDbBuilder::new("load-tester") .with_immutables(&[1, 2, 3]) .append_immutable_trio() .build(); let _logger_guard = init_logger(&opts); - let aggregator_parameters = AggregatorParameters::new(&opts, &immutable_db.dir)?; + let aggregator_parameters = AggregatorParameters::new(&opts, cardano_db.get_immutable_dir())?; let mut current_epoch = Epoch(1); let protocol_parameters = ProtocolParameters::new(2422, 20973, 0.20); info!(">> Starting stress test with options: {opts:?}"); @@ -71,7 +71,7 @@ async fn main() -> StdResult<()> { aggregator, aggregator_parameters, signers_fixture, - immutable_db, + cardano_db, reporter, precomputed_mithril_stake_distribution_signatures: mithril_stake_distribution_signatures, }; @@ -121,7 +121,7 @@ struct ScenarioParameters { aggregator: Aggregator, aggregator_parameters: AggregatorParameters, signers_fixture: MithrilFixture, - immutable_db: DummyImmutableDb, + cardano_db: DummyCardanoDb, precomputed_mithril_stake_distribution_signatures: Vec, reporter: Reporter, } @@ -154,7 +154,7 @@ async fn main_scenario( // Creating the new immutable file early will avoid time effects due to the aggregator runtime design when high client traffic is sent info!(">> Add new immutable file"); - parameters.immutable_db.add_immutable_file(); + parameters.cardano_db.add_immutable_file(); wait::for_epoch_settings_at_epoch( ¶meters.aggregator, @@ -227,7 +227,7 @@ async fn main_scenario( Duration::from_secs(60), &SignedEntityType::CardanoImmutableFilesFull(CardanoDbBeacon::new( *current_epoch.deref(), - parameters.immutable_db.last_immutable_number().unwrap() - 1, + parameters.cardano_db.last_immutable_number().unwrap() - 1, )), ) .await?; @@ -235,7 +235,7 @@ async fn main_scenario( info!(">> Compute the immutable files signature"); let (current_beacon, immutable_files_signatures) = payload_builder::compute_immutable_files_signatures( - ¶meters.immutable_db, + ¶meters.cardano_db, current_epoch, ¶meters.signers_fixture, Duration::from_secs(60), diff --git a/mithril-test-lab/mithril-end-to-end/src/stress_test/payload_builder.rs b/mithril-test-lab/mithril-end-to-end/src/stress_test/payload_builder.rs index e21f66218fe..af389d42397 100644 --- a/mithril-test-lab/mithril-end-to-end/src/stress_test/payload_builder.rs +++ b/mithril-test-lab/mithril-end-to-end/src/stress_test/payload_builder.rs @@ -2,7 +2,7 @@ use std::time::Duration; use anyhow::Context; use mithril_common::{ - digesters::{CardanoImmutableDigester, DummyImmutableDb, ImmutableDigester}, + digesters::{CardanoImmutableDigester, DummyCardanoDb, ImmutableDigester}, entities::{ CardanoDbBeacon, Epoch, ProtocolMessage, ProtocolMessagePartKey, ProtocolParameters, SignedEntityType, Signer, SingleSignatures, @@ -96,7 +96,7 @@ pub async fn precompute_mithril_stake_distribution_signatures( /// Compute all signers single signatures for the given fixture pub async fn compute_immutable_files_signatures( - immutable_db: &DummyImmutableDb, + cardano_db: &DummyCardanoDb, epoch: Epoch, signers_fixture: &MithrilFixture, timeout: Duration, @@ -106,17 +106,17 @@ pub async fn compute_immutable_files_signatures( let beacon = CardanoDbBeacon::new( *epoch, // Minus one because the last immutable isn't "finished" - immutable_db.last_immutable_number().unwrap() - 1, + cardano_db.last_immutable_number().unwrap() - 1, ); let digester = CardanoImmutableDigester::new("devnet".to_string(), None, slog_scope::logger()); let digest = digester - .compute_digest(&immutable_db.dir, &beacon) + .compute_digest(cardano_db.get_immutable_dir(), &beacon) .await .with_context(|| { format!( "Payload Builder can not compute digest of '{}'", - &immutable_db.dir.display() + cardano_db.get_immutable_dir().display() ) })?; let signers_fixture = signers_fixture.clone(); From 3ba6e22fd9e54c828972474a55df702d4b52ca69 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:58:35 +0100 Subject: [PATCH 02/18] feat: create archive for ancillary files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/artifact_builder/cardano_database.rs | 9 +- .../cardano_database_artifacts/ancillary.rs | 180 +++++++++++++++++- .../src/dependency_injection/builder.rs | 19 +- .../src/digesters/dummy_cardano_db.rs | 12 +- mithril-common/src/digesters/mod.rs | 9 +- 5 files changed, 207 insertions(+), 22 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database.rs b/mithril-aggregator/src/artifact_builder/cardano_database.rs index fdb3a86bb67..68bf93dd7d0 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database.rs @@ -116,6 +116,8 @@ mod tests { test_utils::{fake_data, TempDir}, }; + use crate::{test_tools::TestLogger, DumbSnapshotter}; + use super::*; fn get_test_directory(dir_name: &str) -> PathBuf { @@ -166,7 +168,12 @@ mod tests { test_dir, &Version::parse("1.0.0").unwrap(), CompressionAlgorithm::Zstandard, - Arc::new(AncillaryArtifactBuilder::new(vec![])), + Arc::new(AncillaryArtifactBuilder::new( + vec![], + Arc::new(DumbSnapshotter::new()), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + )), ); let beacon = fake_data::beacon(); diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 24c3c5f98e5..885e2d5d1cf 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -1,10 +1,21 @@ #![allow(dead_code)] +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; + +use anyhow::Context; use async_trait::async_trait; -use std::{path::Path, sync::Arc}; +use slog::{debug, Logger}; -use mithril_common::{entities::AncillaryLocation, StdResult}; +use mithril_common::{ + digesters::{IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}, + entities::{AncillaryLocation, CompressionAlgorithm, ImmutableFileNumber}, + logging::LoggerExtensions, + StdResult, +}; -use crate::{FileUploader, LocalUploader}; +use crate::{FileUploader, LocalUploader, Snapshotter}; /// The [AncillaryFileUploader] trait allows identifying uploaders that return locations for ancillary archive files. #[cfg_attr(test, mockall::automock)] @@ -27,13 +38,81 @@ impl AncillaryFileUploader for LocalUploader { /// The archive is uploaded with the provided uploaders. pub struct AncillaryArtifactBuilder { uploaders: Vec>, + snapshotter: Arc, + compression_algorithm: CompressionAlgorithm, + logger: Logger, } impl AncillaryArtifactBuilder { - pub fn new(uploaders: Vec>) -> Self { - Self { uploaders } + /// Creates a new [AncillaryArtifactBuilder]. + pub fn new( + uploaders: Vec>, + snapshotter: Arc, + compression_algorithm: CompressionAlgorithm, + logger: Logger, + ) -> Self { + Self { + uploaders, + logger: logger.new_with_component_name::(), + compression_algorithm, + snapshotter, + } + } + + /// Returns the list of files and directories to include in the snapshot. + /// The immutable file number is incremented by 1 to include the not yet finalized immutable file. + fn get_files_and_directories_to_snapshot(immutable_file_number: u64) -> Vec { + let next_immutable_file_number = immutable_file_number + 1; + let chunk_filename = format!("{:05}.chunk", next_immutable_file_number); + let primary_filename = format!("{:05}.primary", next_immutable_file_number); + let secondary_filename = format!("{:05}.secondary", next_immutable_file_number); + + vec![ + PathBuf::from(VOLATILE_DIR), + PathBuf::from(LEDGER_DIR), + PathBuf::from(IMMUTABLE_DIR).join(chunk_filename), + PathBuf::from(IMMUTABLE_DIR).join(primary_filename), + PathBuf::from(IMMUTABLE_DIR).join(secondary_filename), + ] } + /// Creates an archive for the Cardano database ancillary files for the given immutable file number. + fn create_ancillary_archive( + &self, + immutable_file_number: ImmutableFileNumber, + ) -> StdResult { + debug!( + self.logger, + "Creating ancillary archive for immutable file number: {}", immutable_file_number + ); + + let paths_to_include = Self::get_files_and_directories_to_snapshot(immutable_file_number); + + let archive_name = format!( + "ancillary.{}", + self.compression_algorithm.tar_file_extension() + ); + + let snapshot = self + .snapshotter + .snapshot_subset(&archive_name, paths_to_include) + .with_context(|| { + format!( + "Failed to create snapshot for immutable file number: {}", + immutable_file_number + ) + })?; + + debug!( + self.logger, + "Ancillary archive created at path: {:?}", + snapshot.get_file_path() + ); + + Ok(snapshot.get_file_path().to_path_buf()) + } + + /// Uploads the ancillary archive and returns the locations of the uploaded files. pub async fn upload_archive(&self, db_directory: &Path) -> StdResult> { let mut locations = Vec::new(); for uploader in &self.uploaders { @@ -48,13 +127,31 @@ impl AncillaryArtifactBuilder { #[cfg(test)] mod tests { + use std::fs::File; + + use flate2::read::GzDecoder; use mockall::predicate::eq; + use tar::Archive; + + use mithril_common::digesters::{ + DummyCardanoDbBuilder, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR, + }; + + use crate::{ + test_tools::TestLogger, CompressedArchiveSnapshotter, DumbSnapshotter, + SnapshotterCompressionAlgorithm, + }; use super::*; #[tokio::test] async fn upload_archive_should_return_empty_locations_with_no_uploader() { - let builder = AncillaryArtifactBuilder::new(vec![]); + let builder = AncillaryArtifactBuilder::new( + vec![], + Arc::new(DumbSnapshotter::new()), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ); let locations = builder.upload_archive(Path::new("whatever")).await.unwrap(); @@ -88,7 +185,12 @@ mod tests { let uploaders: Vec> = vec![Arc::new(first_uploader), Arc::new(second_uploader)]; - let builder = AncillaryArtifactBuilder::new(uploaders); + let builder = AncillaryArtifactBuilder::new( + uploaders, + Arc::new(DumbSnapshotter::new()), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ); let locations = builder .upload_archive(Path::new("archive_path")) @@ -107,4 +209,68 @@ mod tests { ] ); } + + #[tokio::test] + async fn create_archive_should_embed_ledger_volatile_directories_and_last_immutables() { + let test_dir = "cardano_database/create_archive"; + let cardano_db = DummyCardanoDbBuilder::new(test_dir) + .with_immutables(&[1, 2, 3]) + .with_ledger_files(&["blocks-0.dat", "blocks-1.dat", "blocks-2.dat"]) + .with_volatile_files(&["437", "537", "637", "737"]) + .build(); + std::fs::create_dir(cardano_db.get_dir().join("whatever")).unwrap(); + + let db_directory = cardano_db.get_dir().to_path_buf(); + let snapshotter = { + CompressedArchiveSnapshotter::new( + db_directory.clone(), + db_directory.parent().unwrap().join("snapshot_dest"), + SnapshotterCompressionAlgorithm::Gzip, + TestLogger::stdout(), + ) + .unwrap() + }; + + let builder = AncillaryArtifactBuilder::new( + vec![], + Arc::new(snapshotter), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ); + + let archive_file_path = builder.create_ancillary_archive(1).unwrap(); + + let mut archive = { + let file_tar_gz = File::open(archive_file_path).unwrap(); + let file_tar_gz_decoder = GzDecoder::new(file_tar_gz); + Archive::new(file_tar_gz_decoder) + }; + + let dst = cardano_db.get_dir().join("unpack_dir"); + archive.unpack(dst.clone()).unwrap(); + + let expected_immutable_path = dst.join(IMMUTABLE_DIR); + assert!(expected_immutable_path.join("00002.chunk").exists()); + assert!(expected_immutable_path.join("00002.primary").exists()); + assert!(expected_immutable_path.join("00002.secondary").exists()); + let immutables_nb = std::fs::read_dir(expected_immutable_path).unwrap().count(); + assert_eq!(3, immutables_nb); + + let expected_ledger_path = dst.join(LEDGER_DIR); + assert!(expected_ledger_path.join("blocks-0.dat").exists()); + assert!(expected_ledger_path.join("blocks-1.dat").exists()); + assert!(expected_ledger_path.join("blocks-2.dat").exists()); + let ledger_nb = std::fs::read_dir(expected_ledger_path).unwrap().count(); + assert_eq!(3, ledger_nb); + + let expected_volatile_path = dst.join(VOLATILE_DIR); + assert!(expected_volatile_path.join("437").exists()); + assert!(expected_volatile_path.join("537").exists()); + assert!(expected_volatile_path.join("637").exists()); + assert!(expected_volatile_path.join("737").exists()); + let volatile_nb = std::fs::read_dir(expected_volatile_path).unwrap().count(); + assert_eq!(4, volatile_nb); + + assert!(!dst.join("whatever").exists()); + } } diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 49e008cadc1..3558e7f6d6f 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1186,6 +1186,7 @@ impl DependenciesBuilder { &self, logger: &Logger, cardano_node_version: Version, + snapshotter: Arc, ) -> Result { let artifacts_dir = Path::new("cardano-database").join("ancillary"); let snapshot_dir = self @@ -1203,9 +1204,12 @@ impl DependenciesBuilder { &snapshot_dir, logger.clone(), ); - let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new(vec![Arc::new( - local_uploader, - )])); + let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new( + vec![Arc::new(local_uploader)], + snapshotter, + self.configuration.snapshot_compression_algorithm, + logger.clone(), + )); Ok(CardanoDatabaseArtifactBuilder::new( self.configuration.db_directory.clone(), @@ -1230,7 +1234,7 @@ impl DependenciesBuilder { Arc::new(CardanoImmutableFilesFullArtifactBuilder::new( self.configuration.get_network()?, &cardano_node_version, - snapshotter, + snapshotter.clone(), snapshot_uploader, self.configuration.snapshot_compression_algorithm, logger.clone(), @@ -1243,7 +1247,11 @@ impl DependenciesBuilder { let cardano_stake_distribution_artifact_builder = Arc::new(CardanoStakeDistributionArtifactBuilder::new(stake_store)); let cardano_database_artifact_builder = - Arc::new(self.create_cardano_database_artifact_builder(&logger, cardano_node_version)?); + Arc::new(self.create_cardano_database_artifact_builder( + &logger, + cardano_node_version, + snapshotter, + )?); let dependencies = SignedEntityServiceArtifactsDependencies::new( mithril_stake_distribution_artifact_builder, cardano_immutable_files_full_artifact_builder, @@ -1820,6 +1828,7 @@ mod tests { .create_cardano_database_artifact_builder( &TestLogger::stdout(), Version::parse("1.0.0").unwrap(), + Arc::new(DumbSnapshotter::new()), ) .unwrap(); diff --git a/mithril-common/src/digesters/dummy_cardano_db.rs b/mithril-common/src/digesters/dummy_cardano_db.rs index cbfe390e6cc..a34a986275d 100644 --- a/mithril-common/src/digesters/dummy_cardano_db.rs +++ b/mithril-common/src/digesters/dummy_cardano_db.rs @@ -1,18 +1,14 @@ use crate::test_utils::TempDir; -use crate::{digesters::ImmutableFile, entities::ImmutableFileNumber}; +use crate::{ + digesters::{ImmutableFile, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}, + entities::ImmutableFileNumber, +}; use std::{ fs::File, io::prelude::Write, path::{Path, PathBuf}, }; -/// Directory name for the immutable files. -pub const IMMUTABLE_DIR: &str = "immutable"; -/// Directory name for the ledger files. -pub const LEDGER_DIR: &str = "ledger"; -/// Directory name for the volatile files. -pub const VOLATILE_DIR: &str = "volatile"; - /// A dummy cardano immutable db. struct DummyImmutableDb { /// The dummy cardano db directory path. diff --git a/mithril-common/src/digesters/mod.rs b/mithril-common/src/digesters/mod.rs index 917418dd73a..cb51a97d9cb 100644 --- a/mithril-common/src/digesters/mod.rs +++ b/mithril-common/src/digesters/mod.rs @@ -17,8 +17,15 @@ pub use immutable_file_observer::{ pub use dumb_immutable_digester::DumbImmutableDigester; +/// Directory name for the immutable files. +pub const IMMUTABLE_DIR: &str = "immutable"; +/// Directory name for the ledger files. +pub const LEDGER_DIR: &str = "ledger"; +/// Directory name for the volatile files. +pub const VOLATILE_DIR: &str = "volatile"; + cfg_test_tools! { mod dummy_cardano_db; - pub use dummy_cardano_db::{DummyCardanoDb, DummyCardanoDbBuilder, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}; + pub use dummy_cardano_db::{DummyCardanoDb, DummyCardanoDbBuilder}; } From b294513a9ac78435b63879cb859c86720bc712e5 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 23 Dec 2024 16:52:06 +0100 Subject: [PATCH 03/18] feat: integrate ancillary archive creation into the upload process --- .../cardano_database_artifacts/ancillary.rs | 68 +++++++++++++++---- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 885e2d5d1cf..1d969922e15 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use std::{ path::{Path, PathBuf}, sync::Arc, @@ -15,7 +14,7 @@ use mithril_common::{ StdResult, }; -use crate::{FileUploader, LocalUploader, Snapshotter}; +use crate::{snapshotter::OngoingSnapshot, FileUploader, LocalUploader, Snapshotter}; /// The [AncillaryFileUploader] trait allows identifying uploaders that return locations for ancillary archive files. #[cfg_attr(test, mockall::automock)] @@ -59,6 +58,16 @@ impl AncillaryArtifactBuilder { } } + pub async fn upload(&self, immutable_file_number: u64) -> StdResult> { + let snapshot = self.create_ancillary_archive(immutable_file_number)?; + + let locations = self + .upload_ancillary_archive(snapshot.get_file_path()) + .await?; + + Ok(locations) + } + /// Returns the list of files and directories to include in the snapshot. /// The immutable file number is incremented by 1 to include the not yet finalized immutable file. fn get_files_and_directories_to_snapshot(immutable_file_number: u64) -> Vec { @@ -80,7 +89,7 @@ impl AncillaryArtifactBuilder { fn create_ancillary_archive( &self, immutable_file_number: ImmutableFileNumber, - ) -> StdResult { + ) -> StdResult { debug!( self.logger, "Creating ancillary archive for immutable file number: {}", immutable_file_number @@ -109,15 +118,17 @@ impl AncillaryArtifactBuilder { snapshot.get_file_path() ); - Ok(snapshot.get_file_path().to_path_buf()) + Ok(snapshot) } /// Uploads the ancillary archive and returns the locations of the uploaded files. - pub async fn upload_archive(&self, db_directory: &Path) -> StdResult> { + async fn upload_ancillary_archive( + &self, + archive_filepath: &Path, + ) -> StdResult> { let mut locations = Vec::new(); for uploader in &self.uploaders { - // TODO: Temporary preparation work, `db_directory` is used as the ancillary archive path for now. - let location = uploader.upload(db_directory).await?; + let location = uploader.upload(archive_filepath).await?; locations.push(location); } @@ -145,7 +156,7 @@ mod tests { use super::*; #[tokio::test] - async fn upload_archive_should_return_empty_locations_with_no_uploader() { + async fn upload_ancillary_archive_should_return_empty_locations_with_no_uploader() { let builder = AncillaryArtifactBuilder::new( vec![], Arc::new(DumbSnapshotter::new()), @@ -153,13 +164,16 @@ mod tests { TestLogger::stdout(), ); - let locations = builder.upload_archive(Path::new("whatever")).await.unwrap(); + let locations = builder + .upload_ancillary_archive(Path::new("whatever")) + .await + .unwrap(); assert!(locations.is_empty()); } #[tokio::test] - async fn upload_archive_should_return_all_uploaders_returned_locations() { + async fn upload_ancillary_archive_should_return_all_uploaders_returned_locations() { let mut first_uploader = MockAncillaryFileUploader::new(); first_uploader .expect_upload() @@ -193,7 +207,7 @@ mod tests { ); let locations = builder - .upload_archive(Path::new("archive_path")) + .upload_ancillary_archive(Path::new("archive_path")) .await .unwrap(); @@ -238,10 +252,10 @@ mod tests { TestLogger::stdout(), ); - let archive_file_path = builder.create_ancillary_archive(1).unwrap(); + let snapshot = builder.create_ancillary_archive(1).unwrap(); let mut archive = { - let file_tar_gz = File::open(archive_file_path).unwrap(); + let file_tar_gz = File::open(snapshot.get_file_path()).unwrap(); let file_tar_gz_decoder = GzDecoder::new(file_tar_gz); Archive::new(file_tar_gz_decoder) }; @@ -273,4 +287,32 @@ mod tests { assert!(!dst.join("whatever").exists()); } + + #[tokio::test] + async fn upload_should_return_error_and_not_upload_when_archive_creation_fails() { + let snapshotter = { + CompressedArchiveSnapshotter::new( + PathBuf::from("directory_not_existing"), + PathBuf::from("whatever"), + SnapshotterCompressionAlgorithm::Gzip, + TestLogger::stdout(), + ) + .unwrap() + }; + + let mut uploader = MockAncillaryFileUploader::new(); + uploader.expect_upload().never(); + + let builder = AncillaryArtifactBuilder::new( + vec![Arc::new(uploader)], + Arc::new(snapshotter), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ); + + builder + .upload(1) + .await + .expect_err("Should return an error when archive creation fails"); + } } From 6f955671e792632bc356f5328045f0f01e78929d Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 23 Dec 2024 17:20:41 +0100 Subject: [PATCH 04/18] feat: reference the ancillary artifact location in the artifact --- .../src/artifact_builder/cardano_database.rs | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database.rs b/mithril-aggregator/src/artifact_builder/cardano_database.rs index 68bf93dd7d0..4b13d7f505e 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database.rs @@ -21,7 +21,6 @@ pub struct CardanoDatabaseArtifactBuilder { db_directory: PathBuf, cardano_node_version: Version, compression_algorithm: CompressionAlgorithm, - #[allow(dead_code)] ancillary_builder: Arc, } @@ -62,8 +61,13 @@ impl ArtifactBuilder for CardanoDataba })?; let total_db_size_uncompressed = compute_uncompressed_database_size(&self.db_directory)?; + let ancillary_locations = self + .ancillary_builder + .upload(beacon.immutable_file_number) + .await?; + let locations = ArtifactsLocations { - ancillary: vec![], + ancillary: ancillary_locations, digests: vec![], immutables: vec![], }; @@ -112,11 +116,13 @@ mod tests { use mithril_common::{ digesters::DummyCardanoDbBuilder, - entities::{ProtocolMessage, ProtocolMessagePartKey}, + entities::{AncillaryLocation, ProtocolMessage, ProtocolMessagePartKey}, test_utils::{fake_data, TempDir}, }; - use crate::{test_tools::TestLogger, DumbSnapshotter}; + use crate::{ + artifact_builder::MockAncillaryFileUploader, test_tools::TestLogger, DumbSnapshotter, + }; use super::*; @@ -164,12 +170,18 @@ mod tests { .build(); let expected_total_size = immutable_trio_file_size + ledger_file_size + volatile_file_size; + let mut ancillary_uploader = MockAncillaryFileUploader::new(); + ancillary_uploader.expect_upload().return_once(|_| { + Ok(AncillaryLocation::CloudStorage { + uri: "ancillary_uri".to_string(), + }) + }); let cardano_database_artifact_builder = CardanoDatabaseArtifactBuilder::new( test_dir, &Version::parse("1.0.0").unwrap(), CompressionAlgorithm::Zstandard, Arc::new(AncillaryArtifactBuilder::new( - vec![], + vec![Arc::new(ancillary_uploader)], Arc::new(DumbSnapshotter::new()), CompressionAlgorithm::Gzip, TestLogger::stdout(), @@ -194,12 +206,15 @@ mod tests { .await .unwrap(); + let expected_ancillary_locations = vec![AncillaryLocation::CloudStorage { + uri: "ancillary_uri".to_string(), + }]; let artifact_expected = CardanoDatabaseSnapshot::new( "merkleroot".to_string(), beacon, expected_total_size, ArtifactsLocations { - ancillary: vec![], + ancillary: expected_ancillary_locations, digests: vec![], immutables: vec![], }, From 86ef2abdc7c1e30c5e57291d30824cc037a862d0 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 24 Dec 2024 16:31:22 +0100 Subject: [PATCH 05/18] feat: make the ancillary archive name more precise --- .../src/artifact_builder/cardano_database.rs | 7 ++-- .../cardano_database_artifacts/ancillary.rs | 39 ++++++++++++------- .../src/dependency_injection/builder.rs | 1 + 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database.rs b/mithril-aggregator/src/artifact_builder/cardano_database.rs index 4b13d7f505e..b7da766c124 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database.rs @@ -61,10 +61,7 @@ impl ArtifactBuilder for CardanoDataba })?; let total_db_size_uncompressed = compute_uncompressed_database_size(&self.db_directory)?; - let ancillary_locations = self - .ancillary_builder - .upload(beacon.immutable_file_number) - .await?; + let ancillary_locations = self.ancillary_builder.upload(&beacon).await?; let locations = ArtifactsLocations { ancillary: ancillary_locations, @@ -118,6 +115,7 @@ mod tests { digesters::DummyCardanoDbBuilder, entities::{AncillaryLocation, ProtocolMessage, ProtocolMessagePartKey}, test_utils::{fake_data, TempDir}, + CardanoNetwork, }; use crate::{ @@ -183,6 +181,7 @@ mod tests { Arc::new(AncillaryArtifactBuilder::new( vec![Arc::new(ancillary_uploader)], Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), )), diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 1d969922e15..89aa77ef9f1 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -9,9 +9,9 @@ use slog::{debug, Logger}; use mithril_common::{ digesters::{IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}, - entities::{AncillaryLocation, CompressionAlgorithm, ImmutableFileNumber}, + entities::{AncillaryLocation, CardanoDbBeacon, CompressionAlgorithm}, logging::LoggerExtensions, - StdResult, + CardanoNetwork, StdResult, }; use crate::{snapshotter::OngoingSnapshot, FileUploader, LocalUploader, Snapshotter}; @@ -38,6 +38,7 @@ impl AncillaryFileUploader for LocalUploader { pub struct AncillaryArtifactBuilder { uploaders: Vec>, snapshotter: Arc, + cardano_network: CardanoNetwork, compression_algorithm: CompressionAlgorithm, logger: Logger, } @@ -47,19 +48,21 @@ impl AncillaryArtifactBuilder { pub fn new( uploaders: Vec>, snapshotter: Arc, + cardano_network: CardanoNetwork, compression_algorithm: CompressionAlgorithm, logger: Logger, ) -> Self { Self { uploaders, logger: logger.new_with_component_name::(), + cardano_network, compression_algorithm, snapshotter, } } - pub async fn upload(&self, immutable_file_number: u64) -> StdResult> { - let snapshot = self.create_ancillary_archive(immutable_file_number)?; + pub async fn upload(&self, beacon: &CardanoDbBeacon) -> StdResult> { + let snapshot = self.create_ancillary_archive(beacon)?; let locations = self .upload_ancillary_archive(snapshot.get_file_path()) @@ -86,19 +89,21 @@ impl AncillaryArtifactBuilder { } /// Creates an archive for the Cardano database ancillary files for the given immutable file number. - fn create_ancillary_archive( - &self, - immutable_file_number: ImmutableFileNumber, - ) -> StdResult { + fn create_ancillary_archive(&self, beacon: &CardanoDbBeacon) -> StdResult { debug!( self.logger, - "Creating ancillary archive for immutable file number: {}", immutable_file_number + "Creating ancillary archive for immutable file number: {}", + beacon.immutable_file_number ); - let paths_to_include = Self::get_files_and_directories_to_snapshot(immutable_file_number); + let paths_to_include = + Self::get_files_and_directories_to_snapshot(beacon.immutable_file_number); let archive_name = format!( - "ancillary.{}", + "{}-e{}-i{}.ancillary.{}", + self.cardano_network, + *beacon.epoch, + beacon.immutable_file_number, self.compression_algorithm.tar_file_extension() ); @@ -108,7 +113,7 @@ impl AncillaryArtifactBuilder { .with_context(|| { format!( "Failed to create snapshot for immutable file number: {}", - immutable_file_number + beacon.immutable_file_number ) })?; @@ -160,6 +165,7 @@ mod tests { let builder = AncillaryArtifactBuilder::new( vec![], Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); @@ -202,6 +208,7 @@ mod tests { let builder = AncillaryArtifactBuilder::new( uploaders, Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); @@ -248,11 +255,14 @@ mod tests { let builder = AncillaryArtifactBuilder::new( vec![], Arc::new(snapshotter), + CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); - let snapshot = builder.create_ancillary_archive(1).unwrap(); + let snapshot = builder + .create_ancillary_archive(&CardanoDbBeacon::new(99, 1)) + .unwrap(); let mut archive = { let file_tar_gz = File::open(snapshot.get_file_path()).unwrap(); @@ -306,12 +316,13 @@ mod tests { let builder = AncillaryArtifactBuilder::new( vec![Arc::new(uploader)], Arc::new(snapshotter), + CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), ); builder - .upload(1) + .upload(&CardanoDbBeacon::new(99, 1)) .await .expect_err("Should return an error when archive creation fails"); } diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 3558e7f6d6f..7c0ab657755 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1207,6 +1207,7 @@ impl DependenciesBuilder { let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new( vec![Arc::new(local_uploader)], snapshotter, + self.configuration.get_network()?, self.configuration.snapshot_compression_algorithm, logger.clone(), )); From 6294a1ee59ba5442404ce1e7d6ff630818e8ba09 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 2 Jan 2025 12:15:41 +0100 Subject: [PATCH 06/18] refactor: remove `http_server` dependency from `LocalUploader` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/builder.rs | 17 +++++++++-- .../src/file_uploaders/local_uploader.rs | 29 ++++++++----------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 7c0ab657755..c83b0c8e5f8 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -65,7 +65,10 @@ use crate::{ entities::AggregatorEpochSettings, event_store::{EventMessage, EventStore, TransmitterService}, file_uploaders::{FileUploader, GcpUploader}, - http_server::routes::router::{self, RouterConfig, RouterState}, + http_server::{ + routes::router::{self, RouterConfig, RouterState}, + SERVER_BASE_PATH, + }, services::{ AggregatorSignableSeedBuilder, AggregatorUpkeepService, BufferedCertifierService, CardanoTransactionsImporter, CertifierService, EpochServiceDependencies, MessageService, @@ -469,7 +472,11 @@ impl DependenciesBuilder { ))) } SnapshotUploaderType::Local => Ok(Arc::new(LocalUploader::new( - self.configuration.get_server_url(), + format!( + "{}{}", + self.configuration.get_server_url(), + SERVER_BASE_PATH + ), &self.configuration.get_snapshot_dir()?, logger, ))), @@ -1200,7 +1207,11 @@ impl DependenciesBuilder { } })?; let local_uploader = LocalUploader::new( - self.configuration.get_server_url(), + format!( + "{}{}", + self.configuration.get_server_url(), + SERVER_BASE_PATH + ), &snapshot_dir, logger.clone(), ); diff --git a/mithril-aggregator/src/file_uploaders/local_uploader.rs b/mithril-aggregator/src/file_uploaders/local_uploader.rs index 48499714815..95d414a22da 100644 --- a/mithril-aggregator/src/file_uploaders/local_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_uploader.rs @@ -7,13 +7,12 @@ use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; use crate::file_uploaders::{FileUploader, FileUri}; -use crate::http_server; use crate::tools; /// LocalUploader is a file uploader working using local files pub struct LocalUploader { - /// File server listening IP - file_server_url: String, + /// File server URL prefix + server_url_prefix: String, /// Target folder where to store files archive target_location: PathBuf, @@ -23,11 +22,11 @@ pub struct LocalUploader { impl LocalUploader { /// LocalUploader factory - pub(crate) fn new(file_server_url: String, target_location: &Path, logger: Logger) -> Self { + pub(crate) fn new(server_url_prefix: String, target_location: &Path, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New LocalUploader created"; "file_server_url" => &file_server_url); + debug!(logger, "New LocalUploader created"; "server_url_prefix" => &server_url_prefix); Self { - file_server_url, + server_url_prefix, target_location: target_location.to_path_buf(), logger, } @@ -46,9 +45,8 @@ impl FileUploader for LocalUploader { let digest = tools::extract_digest_from_path(Path::new(archive_name)); let specific_route = "artifact/snapshot"; let location = format!( - "{}{}/{}/{}/download", - self.file_server_url, - http_server::SERVER_BASE_PATH, + "{}/{}/{}/download", + self.server_url_prefix, specific_route, digest.unwrap() ); @@ -66,7 +64,6 @@ mod tests { use tempfile::tempdir; use crate::file_uploaders::{FileUploader, FileUri}; - use crate::http_server; use crate::test_tools::TestLogger; use super::LocalUploader; @@ -87,17 +84,15 @@ mod tests { async fn should_extract_digest_to_deduce_location() { let source_dir = tempdir().unwrap(); let target_dir = tempdir().unwrap(); - let url = "http://test.com:8080/".to_string(); let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); let expected_location = format!( - "{}{}/artifact/snapshot/{}/download", - url, - http_server::SERVER_BASE_PATH, + "http://test.com:8080/base-root/artifact/snapshot/{}/download", &digest ); - let uploader = LocalUploader::new(url, target_dir.path(), TestLogger::stdout()); + let url_prefix = "http://test.com:8080/base-root".to_string(); + let uploader = LocalUploader::new(url_prefix, target_dir.path(), TestLogger::stdout()); let location = uploader .upload(&archive) .await @@ -113,7 +108,7 @@ mod tests { let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); let uploader = LocalUploader::new( - "http://test.com:8080/".to_string(), + "http://test.com:8080/base-root/".to_string(), target_dir.path(), TestLogger::stdout(), ); @@ -132,7 +127,7 @@ mod tests { create_fake_archive(source_dir.path(), digest); let target_dir = tempdir().unwrap(); let uploader = LocalUploader::new( - "http://test.com:8080/".to_string(), + "http://test.com:8080/base-root/".to_string(), target_dir.path(), TestLogger::stdout(), ); From 9298db2550bdc3e8853266d9c92b4d091d91e8e5 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 2 Jan 2025 12:28:42 +0100 Subject: [PATCH 07/18] refactor: rename `LocalUploader` to `LocalSnapshotUploader` --- .../cardano_database_artifacts/ancillary.rs | 4 ++-- .../src/dependency_injection/builder.rs | 6 ++--- ...uploader.rs => local_snapshot_uploader.rs} | 23 +++++++++++-------- mithril-aggregator/src/file_uploaders/mod.rs | 4 ++-- mithril-aggregator/src/lib.rs | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) rename mithril-aggregator/src/file_uploaders/{local_uploader.rs => local_snapshot_uploader.rs} (84%) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 89aa77ef9f1..cde88d09ded 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -14,7 +14,7 @@ use mithril_common::{ CardanoNetwork, StdResult, }; -use crate::{snapshotter::OngoingSnapshot, FileUploader, LocalUploader, Snapshotter}; +use crate::{snapshotter::OngoingSnapshot, FileUploader, LocalSnapshotUploader, Snapshotter}; /// The [AncillaryFileUploader] trait allows identifying uploaders that return locations for ancillary archive files. #[cfg_attr(test, mockall::automock)] @@ -25,7 +25,7 @@ pub trait AncillaryFileUploader: Send + Sync { } #[async_trait] -impl AncillaryFileUploader for LocalUploader { +impl AncillaryFileUploader for LocalSnapshotUploader { async fn upload(&self, filepath: &Path) -> StdResult { let uri = FileUploader::upload(self, filepath).await?.into(); diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index c83b0c8e5f8..c5a6f2c23cb 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -81,7 +81,7 @@ use crate::{ tools::{CExplorerSignerRetriever, GenesisToolsDependency, SignersImporter}, AggregatorConfig, AggregatorRunner, AggregatorRuntime, CompressedArchiveSnapshotter, Configuration, DependencyContainer, DumbSnapshotter, DumbUploader, EpochSettingsStorer, - LocalUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, MultiSignerImpl, + LocalSnapshotUploader, MetricsService, MithrilSignerRegisterer, MultiSigner, MultiSignerImpl, SingleSignatureAuthenticator, SnapshotUploaderType, Snapshotter, SnapshotterCompressionAlgorithm, VerificationKeyStorer, }; @@ -471,7 +471,7 @@ impl DependenciesBuilder { logger.clone(), ))) } - SnapshotUploaderType::Local => Ok(Arc::new(LocalUploader::new( + SnapshotUploaderType::Local => Ok(Arc::new(LocalSnapshotUploader::new( format!( "{}{}", self.configuration.get_server_url(), @@ -1206,7 +1206,7 @@ impl DependenciesBuilder { error: Some(e.into()), } })?; - let local_uploader = LocalUploader::new( + let local_uploader = LocalSnapshotUploader::new( format!( "{}{}", self.configuration.get_server_url(), diff --git a/mithril-aggregator/src/file_uploaders/local_uploader.rs b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs similarity index 84% rename from mithril-aggregator/src/file_uploaders/local_uploader.rs rename to mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs index 95d414a22da..a1a6a7901ff 100644 --- a/mithril-aggregator/src/file_uploaders/local_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs @@ -9,8 +9,10 @@ use mithril_common::StdResult; use crate::file_uploaders::{FileUploader, FileUri}; use crate::tools; -/// LocalUploader is a file uploader working using local files -pub struct LocalUploader { +// TODO: This specific local uploader will be removed. +// It's only used by the legacy snapshot that uploads the entire Cardano database. +/// LocalSnapshotUploader is a file uploader working using local files +pub struct LocalSnapshotUploader { /// File server URL prefix server_url_prefix: String, @@ -20,11 +22,11 @@ pub struct LocalUploader { logger: Logger, } -impl LocalUploader { - /// LocalUploader factory +impl LocalSnapshotUploader { + /// LocalSnapshotUploader factory pub(crate) fn new(server_url_prefix: String, target_location: &Path, logger: Logger) -> Self { let logger = logger.new_with_component_name::(); - debug!(logger, "New LocalUploader created"; "server_url_prefix" => &server_url_prefix); + debug!(logger, "New LocalSnapshotUploader created"; "server_url_prefix" => &server_url_prefix); Self { server_url_prefix, target_location: target_location.to_path_buf(), @@ -34,7 +36,7 @@ impl LocalUploader { } #[async_trait] -impl FileUploader for LocalUploader { +impl FileUploader for LocalSnapshotUploader { async fn upload(&self, filepath: &Path) -> StdResult { let archive_name = filepath.file_name().unwrap().to_str().unwrap(); let target_path = &self.target_location.join(archive_name); @@ -66,7 +68,7 @@ mod tests { use crate::file_uploaders::{FileUploader, FileUri}; use crate::test_tools::TestLogger; - use super::LocalUploader; + use super::LocalSnapshotUploader; fn create_fake_archive(dir: &Path, digest: &str) -> PathBuf { let file_path = dir.join(format!("test.{digest}.tar.gz")); @@ -92,7 +94,8 @@ mod tests { ); let url_prefix = "http://test.com:8080/base-root".to_string(); - let uploader = LocalUploader::new(url_prefix, target_dir.path(), TestLogger::stdout()); + let uploader = + LocalSnapshotUploader::new(url_prefix, target_dir.path(), TestLogger::stdout()); let location = uploader .upload(&archive) .await @@ -107,7 +110,7 @@ mod tests { let target_dir = tempdir().unwrap(); let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); - let uploader = LocalUploader::new( + let uploader = LocalSnapshotUploader::new( "http://test.com:8080/base-root/".to_string(), target_dir.path(), TestLogger::stdout(), @@ -126,7 +129,7 @@ mod tests { let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; create_fake_archive(source_dir.path(), digest); let target_dir = tempdir().unwrap(); - let uploader = LocalUploader::new( + let uploader = LocalSnapshotUploader::new( "http://test.com:8080/base-root/".to_string(), target_dir.path(), TestLogger::stdout(), diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 5a6e2a44e1e..3cfb003eb44 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -1,13 +1,13 @@ mod dumb_uploader; mod gcp_uploader; mod interface; -mod local_uploader; +mod local_snapshot_uploader; pub use dumb_uploader::*; pub use gcp_uploader::GcpUploader; pub use interface::FileUploader; pub use interface::FileUri; -pub use local_uploader::LocalUploader; +pub use local_snapshot_uploader::LocalSnapshotUploader; #[cfg(test)] pub use interface::MockFileUploader; diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index 4d7c4c03d07..17e2b4cbbe4 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -38,7 +38,7 @@ pub use crate::configuration::{ pub use crate::multi_signer::{MultiSigner, MultiSignerImpl}; pub use commands::{CommandType, MainOpts}; pub use dependency_injection::DependencyContainer; -pub use file_uploaders::{DumbUploader, FileUploader, LocalUploader}; +pub use file_uploaders::{DumbUploader, FileUploader, LocalSnapshotUploader}; pub use message_adapters::{FromRegisterSignerAdapter, ToCertificatePendingMessageAdapter}; pub use metrics::*; pub use runtime::{ From 135c6ef25f69a61946466a39ecf3873af5bce2a8 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Thu, 2 Jan 2025 18:38:04 +0100 Subject: [PATCH 08/18] refactor: type `server_url_prefix` from `LocalSnapshotUploader` to `Url` instead of `String` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jean-Philippe Raynaud Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/builder.rs | 41 ++++++++---- .../file_uploaders/local_snapshot_uploader.rs | 51 +++++++++------ mithril-aggregator/src/file_uploaders/mod.rs | 1 + .../src/file_uploaders/url_sanitizer.rs | 64 +++++++++++++++++++ 4 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 mithril-aggregator/src/file_uploaders/url_sanitizer.rs diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index c5a6f2c23cb..45b3603fdc9 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1,4 +1,5 @@ use anyhow::Context; +use reqwest::Url; use semver::Version; use slog::{debug, Logger}; use std::{collections::BTreeSet, path::Path, sync::Arc}; @@ -471,15 +472,24 @@ impl DependenciesBuilder { logger.clone(), ))) } - SnapshotUploaderType::Local => Ok(Arc::new(LocalSnapshotUploader::new( - format!( + SnapshotUploaderType::Local => { + let url = format!( "{}{}", self.configuration.get_server_url(), SERVER_BASE_PATH - ), - &self.configuration.get_snapshot_dir()?, - logger, - ))), + ); + let server_url_prefix = + Url::parse(&url).map_err(|e| DependenciesBuilderError::Initialization { + message: format!("Could not parse server url:'{url}'."), + error: Some(e.into()), + })?; + + Ok(Arc::new(LocalSnapshotUploader::new( + server_url_prefix, + &self.configuration.get_snapshot_dir()?, + logger, + )?)) + } } } else { Ok(Arc::new(DumbUploader::new())) @@ -1206,15 +1216,18 @@ impl DependenciesBuilder { error: Some(e.into()), } })?; - let local_uploader = LocalSnapshotUploader::new( - format!( - "{}{}", - self.configuration.get_server_url(), - SERVER_BASE_PATH - ), - &snapshot_dir, - logger.clone(), + let url = format!( + "{}{}", + self.configuration.get_server_url(), + SERVER_BASE_PATH ); + let server_url_prefix = + Url::parse(&url).map_err(|e| DependenciesBuilderError::Initialization { + message: format!("Could not parse server url:'{url}'."), + error: Some(e.into()), + })?; + let local_uploader = + LocalSnapshotUploader::new(server_url_prefix, &snapshot_dir, logger.clone())?; let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new( vec![Arc::new(local_uploader)], snapshotter, diff --git a/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs index a1a6a7901ff..1aaf9d9206f 100644 --- a/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs +++ b/mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs @@ -1,12 +1,13 @@ use anyhow::Context; use async_trait::async_trait; +use reqwest::Url; use slog::{debug, Logger}; use std::path::{Path, PathBuf}; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::file_uploaders::{FileUploader, FileUri}; +use crate::file_uploaders::{url_sanitizer::sanitize_url_path, FileUploader, FileUri}; use crate::tools; // TODO: This specific local uploader will be removed. @@ -14,7 +15,7 @@ use crate::tools; /// LocalSnapshotUploader is a file uploader working using local files pub struct LocalSnapshotUploader { /// File server URL prefix - server_url_prefix: String, + server_url_prefix: Url, /// Target folder where to store files archive target_location: PathBuf, @@ -24,14 +25,20 @@ pub struct LocalSnapshotUploader { impl LocalSnapshotUploader { /// LocalSnapshotUploader factory - pub(crate) fn new(server_url_prefix: String, target_location: &Path, logger: Logger) -> Self { + pub(crate) fn new( + server_url_prefix: Url, + target_location: &Path, + logger: Logger, + ) -> StdResult { let logger = logger.new_with_component_name::(); - debug!(logger, "New LocalSnapshotUploader created"; "server_url_prefix" => &server_url_prefix); - Self { + debug!(logger, "New LocalSnapshotUploader created"; "server_url_prefix" => &server_url_prefix.as_str()); + let server_url_prefix = sanitize_url_path(&server_url_prefix)?; + + Ok(Self { server_url_prefix, target_location: target_location.to_path_buf(), logger, - } + }) } } @@ -44,14 +51,13 @@ impl FileUploader for LocalSnapshotUploader { .await .with_context(|| "File copy failure")?; - let digest = tools::extract_digest_from_path(Path::new(archive_name)); - let specific_route = "artifact/snapshot"; - let location = format!( - "{}/{}/{}/download", - self.server_url_prefix, - specific_route, - digest.unwrap() - ); + let digest = tools::extract_digest_from_path(Path::new(archive_name))?; + let location = &self + .server_url_prefix + .join("artifact/snapshot/")? + .join(&format!("{digest}/"))? + .join("download")?; + let location = location.as_str().to_string(); debug!(self.logger, "File 'uploaded' to local storage"; "location" => &location); Ok(FileUri(location)) @@ -68,7 +74,7 @@ mod tests { use crate::file_uploaders::{FileUploader, FileUri}; use crate::test_tools::TestLogger; - use super::LocalSnapshotUploader; + use super::*; fn create_fake_archive(dir: &Path, digest: &str) -> PathBuf { let file_path = dir.join(format!("test.{digest}.tar.gz")); @@ -93,9 +99,10 @@ mod tests { &digest ); - let url_prefix = "http://test.com:8080/base-root".to_string(); + let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap(); let uploader = - LocalSnapshotUploader::new(url_prefix, target_dir.path(), TestLogger::stdout()); + LocalSnapshotUploader::new(url_prefix, target_dir.path(), TestLogger::stdout()) + .unwrap(); let location = uploader .upload(&archive) .await @@ -111,10 +118,11 @@ mod tests { let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7"; let archive = create_fake_archive(source_dir.path(), digest); let uploader = LocalSnapshotUploader::new( - "http://test.com:8080/base-root/".to_string(), + Url::parse("http://test.com:8080/base-root/").unwrap(), target_dir.path(), TestLogger::stdout(), - ); + ) + .unwrap(); uploader.upload(&archive).await.unwrap(); assert!(target_dir @@ -130,10 +138,11 @@ mod tests { create_fake_archive(source_dir.path(), digest); let target_dir = tempdir().unwrap(); let uploader = LocalSnapshotUploader::new( - "http://test.com:8080/base-root/".to_string(), + Url::parse("http://test.com:8080/base-root/").unwrap(), target_dir.path(), TestLogger::stdout(), - ); + ) + .unwrap(); uploader .upload(source_dir.path()) .await diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 3cfb003eb44..31ddab38fd5 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -2,6 +2,7 @@ mod dumb_uploader; mod gcp_uploader; mod interface; mod local_snapshot_uploader; +pub mod url_sanitizer; pub use dumb_uploader::*; pub use gcp_uploader::GcpUploader; diff --git a/mithril-aggregator/src/file_uploaders/url_sanitizer.rs b/mithril-aggregator/src/file_uploaders/url_sanitizer.rs new file mode 100644 index 00000000000..846fd42a029 --- /dev/null +++ b/mithril-aggregator/src/file_uploaders/url_sanitizer.rs @@ -0,0 +1,64 @@ +use anyhow::{anyhow, Context}; +use reqwest::Url; + +use mithril_common::StdResult; + +/// Sanitize URL path by removing empty segments and adding trailing slash +pub fn sanitize_url_path(url: &Url) -> StdResult { + let segments_non_empty = url + .path_segments() + .map(|s| s.into_iter().filter(|s| !s.is_empty()).collect::>()) + .unwrap_or_default(); + let mut url = url.clone(); + { + let mut url_path_segments = url + .path_segments_mut() + .map_err(|e| anyhow!("error parsing URL: {e:?}")) + .with_context(|| "while sanitizing URL path: {url}")?; + let url_path_segments_cleared = url_path_segments.clear(); + for segment in segments_non_empty { + url_path_segments_cleared.push(segment); + } + url_path_segments_cleared.push(""); + } + + Ok(url) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sanitize_url_path() { + let url = Url::parse("http://example.com/a//b/c.ext?test=123").unwrap(); + assert_eq!( + "http://example.com/a/b/c.ext/?test=123", + sanitize_url_path(&url).unwrap().as_str() + ); + + let url = Url::parse("http://example.com/a//b/c.ext").unwrap(); + assert_eq!( + "http://example.com/a/b/c.ext/", + sanitize_url_path(&url).unwrap().as_str() + ); + + let url = Url::parse("http://example.com/a//b/c").unwrap(); + assert_eq!( + "http://example.com/a/b/c/", + sanitize_url_path(&url).unwrap().as_str() + ); + + let url = Url::parse("http://example.com/").unwrap(); + assert_eq!( + "http://example.com/", + sanitize_url_path(&url).unwrap().as_str() + ); + + let url = Url::parse("http://example.com").unwrap(); + assert_eq!( + "http://example.com/", + sanitize_url_path(&url).unwrap().as_str() + ); + } +} From b6cf1b2770ee4367ed63060de00ce0f0a0e3b481 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 3 Jan 2025 12:04:06 +0100 Subject: [PATCH 09/18] feat: implement standard `LocalUploader` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../cardano_database_artifacts/ancillary.rs | 6 +- .../src/dependency_injection/builder.rs | 5 +- .../src/file_uploaders/local_uploader.rs | 157 ++++++++++++++++++ mithril-aggregator/src/file_uploaders/mod.rs | 2 + 4 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 mithril-aggregator/src/file_uploaders/local_uploader.rs diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index cde88d09ded..3bec4702b3c 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -14,7 +14,9 @@ use mithril_common::{ CardanoNetwork, StdResult, }; -use crate::{snapshotter::OngoingSnapshot, FileUploader, LocalSnapshotUploader, Snapshotter}; +use crate::{ + file_uploaders::LocalUploader, snapshotter::OngoingSnapshot, FileUploader, Snapshotter, +}; /// The [AncillaryFileUploader] trait allows identifying uploaders that return locations for ancillary archive files. #[cfg_attr(test, mockall::automock)] @@ -25,7 +27,7 @@ pub trait AncillaryFileUploader: Send + Sync { } #[async_trait] -impl AncillaryFileUploader for LocalSnapshotUploader { +impl AncillaryFileUploader for LocalUploader { async fn upload(&self, filepath: &Path) -> StdResult { let uri = FileUploader::upload(self, filepath).await?.into(); diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 45b3603fdc9..ca3bbb8b731 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -65,7 +65,7 @@ use crate::{ }, entities::AggregatorEpochSettings, event_store::{EventMessage, EventStore, TransmitterService}, - file_uploaders::{FileUploader, GcpUploader}, + file_uploaders::{FileUploader, GcpUploader, LocalUploader}, http_server::{ routes::router::{self, RouterConfig, RouterState}, SERVER_BASE_PATH, @@ -1226,8 +1226,7 @@ impl DependenciesBuilder { message: format!("Could not parse server url:'{url}'."), error: Some(e.into()), })?; - let local_uploader = - LocalSnapshotUploader::new(server_url_prefix, &snapshot_dir, logger.clone())?; + let local_uploader = LocalUploader::new(server_url_prefix, &snapshot_dir, logger.clone())?; let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new( vec![Arc::new(local_uploader)], snapshotter, diff --git a/mithril-aggregator/src/file_uploaders/local_uploader.rs b/mithril-aggregator/src/file_uploaders/local_uploader.rs new file mode 100644 index 00000000000..21e2a1157b4 --- /dev/null +++ b/mithril-aggregator/src/file_uploaders/local_uploader.rs @@ -0,0 +1,157 @@ +use anyhow::Context; +use async_trait::async_trait; +use reqwest::Url; +use slog::{debug, Logger}; +use std::path::{Path, PathBuf}; + +use mithril_common::logging::LoggerExtensions; +use mithril_common::StdResult; + +use crate::file_uploaders::{url_sanitizer::sanitize_url_path, FileUploader, FileUri}; + +/// LocalUploader is a file uploader working using local files +pub struct LocalUploader { + /// File server URL prefix + server_url_prefix: Url, + + /// Target folder where to store files archive + target_location: PathBuf, + + logger: Logger, +} + +impl LocalUploader { + /// LocalUploader factory + pub(crate) fn new( + server_url_prefix: Url, + target_location: &Path, + logger: Logger, + ) -> StdResult { + let logger = logger.new_with_component_name::(); + debug!(logger, "New LocalUploader created"; "server_url_prefix" => &server_url_prefix.as_str()); + let server_url_prefix = sanitize_url_path(&server_url_prefix)?; + + Ok(Self { + server_url_prefix, + target_location: target_location.to_path_buf(), + logger, + }) + } +} + +#[async_trait] +impl FileUploader for LocalUploader { + async fn upload(&self, filepath: &Path) -> StdResult { + let archive_name = filepath.file_name().unwrap().to_str().unwrap(); + let target_path = &self.target_location.join(archive_name); + tokio::fs::copy(filepath, target_path) + .await + .with_context(|| "File copy failure")?; + + let location = &self + .server_url_prefix + .join("artifact/snapshot/")? + .join(archive_name)?; + let location = location.as_str().to_string(); + + debug!(self.logger, "File 'uploaded' to local storage"; "location" => &location); + Ok(FileUri(location)) + } +} + +#[cfg(test)] +mod tests { + use std::fs::File; + use std::io::Write; + use std::path::{Path, PathBuf}; + + use mithril_common::test_utils::TempDir; + + use crate::test_tools::TestLogger; + + use super::*; + + fn create_fake_archive(dir: &Path, name: &str) -> PathBuf { + let file_path = dir.join(format!("{name}.tar.gz")); + let mut file = File::create(&file_path).unwrap(); + writeln!( + file, + "I swear, this is an archive, not a temporary test file." + ) + .unwrap(); + + file_path + } + + #[tokio::test] + async fn should_extract_archive_name_to_deduce_location() { + let source_dir = TempDir::create( + "local_uploader", + "should_extract_archive_name_to_deduce_location_source", + ); + let target_dir = TempDir::create( + "local_uploader", + "should_extract_archive_name_to_deduce_location_target", + ); + let archive_name = "an_archive"; + let archive = create_fake_archive(&source_dir, archive_name); + let expected_location = format!( + "http://test.com:8080/base-root/artifact/snapshot/{}", + &archive.file_name().unwrap().to_str().unwrap() + ); + + let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap(); + let uploader = LocalUploader::new(url_prefix, &target_dir, TestLogger::stdout()).unwrap(); + let location = uploader + .upload(&archive) + .await + .expect("local upload should not fail"); + + assert_eq!(FileUri(expected_location), location); + } + + #[tokio::test] + async fn should_copy_file_to_target_location() { + let source_dir = TempDir::create( + "local_uploader", + "should_copy_file_to_target_location_source", + ); + let target_dir = TempDir::create( + "local_uploader", + "should_copy_file_to_target_location_target", + ); + println!("target_dir: {:?}", target_dir); + let archive = create_fake_archive(&source_dir, "an_archive"); + let uploader = LocalUploader::new( + Url::parse("http://test.com:8080/base-root/").unwrap(), + &target_dir, + TestLogger::stdout(), + ) + .unwrap(); + uploader.upload(&archive).await.unwrap(); + + assert!(target_dir.join(archive.file_name().unwrap()).exists()); + } + + #[tokio::test] + async fn should_error_if_path_is_a_directory() { + let source_dir = TempDir::create( + "local_uploader", + "should_error_if_path_is_a_directory_source", + ); + let target_dir = TempDir::create( + "local_uploader", + "should_error_if_path_is_a_directory_target", + ); + let uploader = LocalUploader::new( + Url::parse("http://test.com:8080/base-root/").unwrap(), + &target_dir, + TestLogger::stdout(), + ) + .unwrap(); + uploader + .upload(&source_dir) + .await + .expect_err("Uploading a directory should fail"); + } +} diff --git a/mithril-aggregator/src/file_uploaders/mod.rs b/mithril-aggregator/src/file_uploaders/mod.rs index 31ddab38fd5..428d3851361 100644 --- a/mithril-aggregator/src/file_uploaders/mod.rs +++ b/mithril-aggregator/src/file_uploaders/mod.rs @@ -2,6 +2,7 @@ mod dumb_uploader; mod gcp_uploader; mod interface; mod local_snapshot_uploader; +mod local_uploader; pub mod url_sanitizer; pub use dumb_uploader::*; @@ -9,6 +10,7 @@ pub use gcp_uploader::GcpUploader; pub use interface::FileUploader; pub use interface::FileUri; pub use local_snapshot_uploader::LocalSnapshotUploader; +pub use local_uploader::LocalUploader; #[cfg(test)] pub use interface::MockFileUploader; From b8668e85d9481c27281209fc660b40a37b3c32f3 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 3 Jan 2025 15:47:51 +0100 Subject: [PATCH 10/18] refactor: create function to build the server url prefix in the aggregator builder --- .../src/dependency_injection/builder.rs | 50 ++++++++----------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index ca3bbb8b731..892c8b661a5 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -307,6 +307,19 @@ impl DependenciesBuilder { } } + fn get_server_url_prefix(&self) -> Result { + let url = format!( + "{}{}", + self.configuration.get_server_url(), + SERVER_BASE_PATH + ); + + Url::parse(&url).map_err(|e| DependenciesBuilderError::Initialization { + message: format!("Could not parse server url:'{url}'."), + error: Some(e.into()), + }) + } + /// Get the allowed signed entity types discriminants fn get_allowed_signed_entity_types_discriminants( &self, @@ -472,24 +485,11 @@ impl DependenciesBuilder { logger.clone(), ))) } - SnapshotUploaderType::Local => { - let url = format!( - "{}{}", - self.configuration.get_server_url(), - SERVER_BASE_PATH - ); - let server_url_prefix = - Url::parse(&url).map_err(|e| DependenciesBuilderError::Initialization { - message: format!("Could not parse server url:'{url}'."), - error: Some(e.into()), - })?; - - Ok(Arc::new(LocalSnapshotUploader::new( - server_url_prefix, - &self.configuration.get_snapshot_dir()?, - logger, - )?)) - } + SnapshotUploaderType::Local => Ok(Arc::new(LocalSnapshotUploader::new( + self.get_server_url_prefix()?, + &self.configuration.get_snapshot_dir()?, + logger, + )?)), } } else { Ok(Arc::new(DumbUploader::new())) @@ -1216,17 +1216,9 @@ impl DependenciesBuilder { error: Some(e.into()), } })?; - let url = format!( - "{}{}", - self.configuration.get_server_url(), - SERVER_BASE_PATH - ); - let server_url_prefix = - Url::parse(&url).map_err(|e| DependenciesBuilderError::Initialization { - message: format!("Could not parse server url:'{url}'."), - error: Some(e.into()), - })?; - let local_uploader = LocalUploader::new(server_url_prefix, &snapshot_dir, logger.clone())?; + + let local_uploader = + LocalUploader::new(self.get_server_url_prefix()?, &snapshot_dir, logger.clone())?; let ancillary_builder = Arc::new(AncillaryArtifactBuilder::new( vec![Arc::new(local_uploader)], snapshotter, From 536dd400e28fba509c2558a8141519b63cecce75 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Fri, 3 Jan 2025 18:02:05 +0100 Subject: [PATCH 11/18] refactor: rewording and adjusting values in tests --- .../cardano_database_artifacts/ancillary.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 3bec4702b3c..c1bb43a31c2 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -74,7 +74,8 @@ impl AncillaryArtifactBuilder { } /// Returns the list of files and directories to include in the snapshot. - /// The immutable file number is incremented by 1 to include the not yet finalized immutable file. + /// The immutable file included in the ancillary archive corresponds to the last one (and not finalized yet) + /// when the immutable file number given to the function corresponds to the penultimate. fn get_files_and_directories_to_snapshot(immutable_file_number: u64) -> Vec { let next_immutable_file_number = immutable_file_number + 1; let chunk_filename = format!("{:05}.chunk", next_immutable_file_number); @@ -114,7 +115,7 @@ impl AncillaryArtifactBuilder { .snapshot_subset(&archive_name, paths_to_include) .with_context(|| { format!( - "Failed to create snapshot for immutable file number: {}", + "Failed to create ancillary archive for immutable file number: {}", beacon.immutable_file_number ) })?; @@ -263,7 +264,7 @@ mod tests { ); let snapshot = builder - .create_ancillary_archive(&CardanoDbBeacon::new(99, 1)) + .create_ancillary_archive(&CardanoDbBeacon::new(99, 2)) .unwrap(); let mut archive = { @@ -276,9 +277,9 @@ mod tests { archive.unpack(dst.clone()).unwrap(); let expected_immutable_path = dst.join(IMMUTABLE_DIR); - assert!(expected_immutable_path.join("00002.chunk").exists()); - assert!(expected_immutable_path.join("00002.primary").exists()); - assert!(expected_immutable_path.join("00002.secondary").exists()); + assert!(expected_immutable_path.join("00003.chunk").exists()); + assert!(expected_immutable_path.join("00003.primary").exists()); + assert!(expected_immutable_path.join("00003.secondary").exists()); let immutables_nb = std::fs::read_dir(expected_immutable_path).unwrap().count(); assert_eq!(3, immutables_nb); From f7c5bada35b225137746efcb901d3dba6f65b78e Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 10:55:25 +0100 Subject: [PATCH 12/18] feat: return error when `AncillaryArtifactBuilder` is created without uploader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/artifact_builder/cardano_database.rs | 17 +++++---- .../cardano_database_artifacts/ancillary.rs | 36 ++++++++++--------- .../src/dependency_injection/builder.rs | 2 +- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database.rs b/mithril-aggregator/src/artifact_builder/cardano_database.rs index b7da766c124..9208b697694 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database.rs @@ -178,13 +178,16 @@ mod tests { test_dir, &Version::parse("1.0.0").unwrap(), CompressionAlgorithm::Zstandard, - Arc::new(AncillaryArtifactBuilder::new( - vec![Arc::new(ancillary_uploader)], - Arc::new(DumbSnapshotter::new()), - CardanoNetwork::DevNet(123), - CompressionAlgorithm::Gzip, - TestLogger::stdout(), - )), + Arc::new( + AncillaryArtifactBuilder::new( + vec![Arc::new(ancillary_uploader)], + Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ) + .unwrap(), + ), ); let beacon = fake_data::beacon(); diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index c1bb43a31c2..75558fda707 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -3,7 +3,7 @@ use std::{ sync::Arc, }; -use anyhow::Context; +use anyhow::{anyhow, Context}; use async_trait::async_trait; use slog::{debug, Logger}; @@ -53,14 +53,20 @@ impl AncillaryArtifactBuilder { cardano_network: CardanoNetwork, compression_algorithm: CompressionAlgorithm, logger: Logger, - ) -> Self { - Self { + ) -> StdResult { + if uploaders.is_empty() { + return Err(anyhow!( + "At least one uploader is required to create an 'AncillaryArtifactBuilder'" + )); + } + + Ok(Self { uploaders, logger: logger.new_with_component_name::(), cardano_network, compression_algorithm, snapshotter, - } + }) } pub async fn upload(&self, beacon: &CardanoDbBeacon) -> StdResult> { @@ -164,8 +170,8 @@ mod tests { use super::*; #[tokio::test] - async fn upload_ancillary_archive_should_return_empty_locations_with_no_uploader() { - let builder = AncillaryArtifactBuilder::new( + async fn create_ancillary_builder_should_error_when_no_uploader() { + let result = AncillaryArtifactBuilder::new( vec![], Arc::new(DumbSnapshotter::new()), CardanoNetwork::DevNet(123), @@ -173,12 +179,7 @@ mod tests { TestLogger::stdout(), ); - let locations = builder - .upload_ancillary_archive(Path::new("whatever")) - .await - .unwrap(); - - assert!(locations.is_empty()); + assert!(result.is_err(), "Should return an error when no uploaders") } #[tokio::test] @@ -214,7 +215,8 @@ mod tests { CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), - ); + ) + .unwrap(); let locations = builder .upload_ancillary_archive(Path::new("archive_path")) @@ -256,12 +258,13 @@ mod tests { }; let builder = AncillaryArtifactBuilder::new( - vec![], + vec![Arc::new(MockAncillaryFileUploader::new())], Arc::new(snapshotter), CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), - ); + ) + .unwrap(); let snapshot = builder .create_ancillary_archive(&CardanoDbBeacon::new(99, 2)) @@ -322,7 +325,8 @@ mod tests { CardanoNetwork::DevNet(123), CompressionAlgorithm::Gzip, TestLogger::stdout(), - ); + ) + .unwrap(); builder .upload(&CardanoDbBeacon::new(99, 1)) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 892c8b661a5..e4ece4eb62e 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1225,7 +1225,7 @@ impl DependenciesBuilder { self.configuration.get_network()?, self.configuration.snapshot_compression_algorithm, logger.clone(), - )); + )?); Ok(CardanoDatabaseArtifactBuilder::new( self.configuration.db_directory.clone(), From a041e51db3dec25c793654cf23c295705f52e58d Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:30:18 +0100 Subject: [PATCH 13/18] feat: log uploader errors and return an error if no location is returned while trying to upload the archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../cardano_database_artifacts/ancillary.rs | 91 +++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 75558fda707..8554d29e67c 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -5,7 +5,7 @@ use std::{ use anyhow::{anyhow, Context}; use async_trait::async_trait; -use slog::{debug, Logger}; +use slog::{debug, error, Logger}; use mithril_common::{ digesters::{IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}, @@ -29,7 +29,10 @@ pub trait AncillaryFileUploader: Send + Sync { #[async_trait] impl AncillaryFileUploader for LocalUploader { async fn upload(&self, filepath: &Path) -> StdResult { - let uri = FileUploader::upload(self, filepath).await?.into(); + let uri = FileUploader::upload(self, filepath) + .await + .with_context(|| "Error while uploading with 'LocalUploader'")? + .into(); Ok(AncillaryLocation::CloudStorage { uri }) } @@ -142,8 +145,25 @@ impl AncillaryArtifactBuilder { ) -> StdResult> { let mut locations = Vec::new(); for uploader in &self.uploaders { - let location = uploader.upload(archive_filepath).await?; - locations.push(location); + let result = uploader.upload(archive_filepath).await; + match result { + Ok(location) => { + locations.push(location); + } + Err(e) => { + error!( + self.logger, + "Failed to upload ancillary archive"; + "error" => e.to_string() + ); + } + } + } + + if locations.is_empty() { + return Err(anyhow!( + "Failed to upload ancillary archive with all uploaders" + )); } Ok(locations) @@ -158,8 +178,9 @@ mod tests { use mockall::predicate::eq; use tar::Archive; - use mithril_common::digesters::{ - DummyCardanoDbBuilder, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR, + use mithril_common::{ + digesters::{DummyCardanoDbBuilder, IMMUTABLE_DIR, LEDGER_DIR, VOLATILE_DIR}, + test_utils::TempDir, }; use crate::{ @@ -182,6 +203,64 @@ mod tests { assert!(result.is_err(), "Should return an error when no uploaders") } + #[tokio::test] + async fn upload_ancillary_archive_should_log_upload_errors() { + let log_path = TempDir::create( + "ancillary", + "upload_ancillary_archive_should_log_upload_errors", + ) + .join("test.log"); + + let mut uploader = MockAncillaryFileUploader::new(); + uploader + .expect_upload() + .return_once(|_| Err(anyhow!("Failure while uploading..."))); + + { + let builder = AncillaryArtifactBuilder::new( + vec![Arc::new(uploader)], + Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), + CompressionAlgorithm::Gzip, + TestLogger::file(&log_path), + ) + .unwrap(); + + let _ = builder + .upload_ancillary_archive(Path::new("archive_path")) + .await; + } + + let logs = std::fs::read_to_string(&log_path).unwrap(); + assert!(logs.contains("Failure while uploading...")); + } + + #[tokio::test] + async fn upload_ancillary_archive_should_error_when_no_location_is_returned() { + let mut uploader = MockAncillaryFileUploader::new(); + uploader + .expect_upload() + .return_once(|_| Err(anyhow!("Failure while uploading, no location returned"))); + + let builder = AncillaryArtifactBuilder::new( + vec![Arc::new(uploader)], + Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ) + .unwrap(); + + let result = builder + .upload_ancillary_archive(Path::new("archive_path")) + .await; + + assert!( + result.is_err(), + "Should return an error when no location is returned" + ); + } + #[tokio::test] async fn upload_ancillary_archive_should_return_all_uploaders_returned_locations() { let mut first_uploader = MockAncillaryFileUploader::new(); From bda496857a54219d7f0e2eb008aa108d404d6644 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:44:59 +0100 Subject: [PATCH 14/18] feat: enable Snapshotter to create archives in subdirectories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../cardano_database_artifacts/ancillary.rs | 6 +- .../cardano_immutable_files_full.rs | 3 +- mithril-aggregator/src/snapshotter.rs | 75 +++++++++++-------- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 8554d29e67c..e4ed6d58cf6 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -119,9 +119,13 @@ impl AncillaryArtifactBuilder { self.compression_algorithm.tar_file_extension() ); + let ancillary_archive_path = Path::new("cardano-database") + .join("ancillary") + .join(&archive_name); + let snapshot = self .snapshotter - .snapshot_subset(&archive_name, paths_to_include) + .snapshot_subset(&ancillary_archive_path, paths_to_include) .with_context(|| { format!( "Failed to create ancillary archive for immutable file number: {}", diff --git a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs index 34006596ac9..cbfa9430ff2 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs @@ -2,6 +2,7 @@ use anyhow::Context; use async_trait::async_trait; use semver::Version; use slog::{debug, warn, Logger}; +use std::path::Path; use std::sync::Arc; use thiserror::Error; @@ -74,7 +75,7 @@ impl CardanoImmutableFilesFullArtifactBuilder { // spawn a separate thread to prevent blocking let ongoing_snapshot = tokio::task::spawn_blocking(move || -> StdResult { - snapshotter.snapshot_all(&snapshot_name) + snapshotter.snapshot_all(Path::new(&snapshot_name)) }) .await??; diff --git a/mithril-aggregator/src/snapshotter.rs b/mithril-aggregator/src/snapshotter.rs index 9af9c0de897..d624ac3b5f0 100644 --- a/mithril-aggregator/src/snapshotter.rs +++ b/mithril-aggregator/src/snapshotter.rs @@ -18,15 +18,11 @@ use crate::ZstandardCompressionParameters; /// Define the ability to create snapshots. pub trait Snapshotter: Sync + Send { - /// Create a new snapshot with the given archive name. - fn snapshot_all(&self, archive_name: &str) -> StdResult; + /// Create a new snapshot with the given filepath. + fn snapshot_all(&self, filepath: &Path) -> StdResult; - /// Create a new snapshot with the given archive name from a subset of directories and files. - fn snapshot_subset( - &self, - archive_name: &str, - files: Vec, - ) -> StdResult; + /// Create a new snapshot with the given filepath from a subset of directories and files. + fn snapshot_subset(&self, filepath: &Path, files: Vec) -> StdResult; } /// Compression algorithm and parameters of the [CompressedArchiveSnapshotter]. @@ -161,17 +157,17 @@ pub enum SnapshotError { } impl Snapshotter for CompressedArchiveSnapshotter { - fn snapshot_all(&self, archive_name: &str) -> StdResult { + fn snapshot_all(&self, filepath: &Path) -> StdResult { let appender = AppenderDirAll { db_directory: self.db_directory.clone(), }; - self.snapshot(archive_name, appender) + self.snapshot(filepath, appender) } fn snapshot_subset( &self, - archive_name: &str, + filepath: &Path, entries: Vec, ) -> StdResult { if entries.is_empty() { @@ -183,7 +179,7 @@ impl Snapshotter for CompressedArchiveSnapshotter { entries, }; - self.snapshot(archive_name, appender) + self.snapshot(filepath, appender) } } @@ -222,12 +218,16 @@ impl CompressedArchiveSnapshotter { }) } - fn snapshot( - &self, - archive_name: &str, - appender: T, - ) -> StdResult { - let archive_path = self.ongoing_snapshot_directory.join(archive_name); + fn snapshot(&self, filepath: &Path, appender: T) -> StdResult { + let archive_path = self.ongoing_snapshot_directory.join(filepath); + if let Some(archive_dir) = archive_path.parent() { + fs::create_dir_all(archive_dir).with_context(|| { + format!( + "Can not create archive directory: '{}'", + archive_dir.display() + ) + })?; + } let filesize = self.create_and_verify_archive(&archive_path, appender).inspect_err(|_err| { if archive_path.exists() { if let Err(remove_error) = fs::remove_file(&archive_path) { @@ -261,7 +261,11 @@ impl CompressedArchiveSnapshotter { archive_path.display() ); - let tar_file = File::create(archive_path).map_err(SnapshotError::CreateArchiveError)?; + let tar_file = File::create(archive_path) + .map_err(SnapshotError::CreateArchiveError) + .with_context(|| { + format!("Error while creating the archive with path: {archive_path:?}") + })?; match self.compression_algorithm { SnapshotterCompressionAlgorithm::Gzip => { @@ -466,13 +470,13 @@ impl Default for DumbSnapshotter { } impl Snapshotter for DumbSnapshotter { - fn snapshot_all(&self, archive_name: &str) -> StdResult { + fn snapshot_all(&self, archive_name: &Path) -> StdResult { let mut value = self .last_snapshot .write() .map_err(|e| SnapshotError::UploadFileError(e.to_string()))?; let snapshot = OngoingSnapshot { - filepath: Path::new(archive_name).to_path_buf(), + filepath: archive_name.to_path_buf(), filesize: 0, }; *value = Some(snapshot.clone()); @@ -482,7 +486,7 @@ impl Snapshotter for DumbSnapshotter { fn snapshot_subset( &self, - archive_name: &str, + archive_name: &Path, _files: Vec, ) -> StdResult { self.snapshot_all(archive_name) @@ -535,13 +539,15 @@ mod tests { #[test] fn test_dumb_snapshotter_snasphot_return_archive_name_with_size_0() { let snapshotter = DumbSnapshotter::new(); - let snapshot = snapshotter.snapshot_all("archive.tar.gz").unwrap(); + let snapshot = snapshotter + .snapshot_all(Path::new("archive.tar.gz")) + .unwrap(); assert_eq!(PathBuf::from("archive.tar.gz"), *snapshot.get_file_path()); assert_eq!(0, *snapshot.get_file_size()); let snapshot = snapshotter - .snapshot_subset("archive.tar.gz", vec![PathBuf::from("whatever")]) + .snapshot_subset(Path::new("archive.tar.gz"), vec![PathBuf::from("whatever")]) .unwrap(); assert_eq!(PathBuf::from("archive.tar.gz"), *snapshot.get_file_path()); assert_eq!(0, *snapshot.get_file_size()); @@ -556,7 +562,7 @@ mod tests { .is_none()); let snapshot = snapshotter - .snapshot_all("whatever") + .snapshot_all(Path::new("whatever")) .expect("Dumb snapshotter::snapshot should not fail."); assert_eq!( Some(snapshot), @@ -566,7 +572,7 @@ mod tests { ); let snapshot = snapshotter - .snapshot_subset("another_whatever", vec![PathBuf::from("subdir")]) + .snapshot_subset(Path::new("another_whatever"), vec![PathBuf::from("subdir")]) .expect("Dumb snapshotter::snapshot should not fail."); assert_eq!( Some(snapshot), @@ -641,7 +647,7 @@ mod tests { File::create(pending_snapshot_directory.join("other-process.file")).unwrap(); let _ = snapshotter - .snapshot_all("whatever.tar.gz") + .snapshot_all(Path::new("whatever.tar.gz")) .expect_err("Snapshotter::snapshot should fail if the db is empty."); let remaining_files: Vec = fs::read_dir(&pending_snapshot_directory) .unwrap() @@ -687,7 +693,7 @@ mod tests { .expect("verify_archive should not fail"); snapshotter - .snapshot_all(pending_snapshot_archive_file) + .snapshot_all(Path::new(pending_snapshot_archive_file)) .expect("Snapshotter::snapshot should not fail."); } @@ -728,7 +734,7 @@ mod tests { .expect("verify_archive should not fail"); snapshotter - .snapshot_all(pending_snapshot_archive_file) + .snapshot_all(Path::new(pending_snapshot_archive_file)) .expect("Snapshotter::snapshot should not fail."); } @@ -753,7 +759,7 @@ mod tests { let snapshot = snapshotter .snapshot_subset( - &random_archive_name(), + Path::new(&random_archive_name()), vec![ directory_to_archive_path.clone(), file_to_archive_path.clone(), @@ -784,7 +790,10 @@ mod tests { .unwrap(); snapshotter - .snapshot_subset(&random_archive_name(), vec![PathBuf::from("not_exist")]) + .snapshot_subset( + Path::new(&random_archive_name()), + vec![PathBuf::from("not_exist")], + ) .expect_err("snapshot_subset should return error when file or directory not exist"); } @@ -803,7 +812,7 @@ mod tests { .unwrap(); snapshotter - .snapshot_subset(&random_archive_name(), vec![]) + .snapshot_subset(Path::new(&random_archive_name()), vec![]) .expect_err("snapshot_subset should return error when entries is empty"); } @@ -826,7 +835,7 @@ mod tests { let snapshot = snapshotter .snapshot_subset( - &random_archive_name(), + Path::new(&random_archive_name()), vec![ directory_to_archive_path.clone(), directory_to_archive_path.clone(), From e7237152ee030e388f9902bae5636d3208b2e734 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:29:35 +0100 Subject: [PATCH 15/18] fix: remove useless async attribute in test --- .../artifact_builder/cardano_database_artifacts/ancillary.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index e4ed6d58cf6..232dc01a846 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -194,8 +194,8 @@ mod tests { use super::*; - #[tokio::test] - async fn create_ancillary_builder_should_error_when_no_uploader() { + #[test] + fn create_ancillary_builder_should_error_when_no_uploader() { let result = AncillaryArtifactBuilder::new( vec![], Arc::new(DumbSnapshotter::new()), From 1eee1942fbb297bbb999f0db9ac10837910b77b0 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 17:41:19 +0100 Subject: [PATCH 16/18] test: verify upload behavior when uploaders are failing and at least one is returning a location --- .../cardano_database_artifacts/ancillary.rs | 85 +++++++++++++------ 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs index 232dc01a846..15d36c65829 100644 --- a/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs +++ b/mithril-aggregator/src/artifact_builder/cardano_database_artifacts/ancillary.rs @@ -179,7 +179,6 @@ mod tests { use std::fs::File; use flate2::read::GzDecoder; - use mockall::predicate::eq; use tar::Archive; use mithril_common::{ @@ -194,6 +193,28 @@ mod tests { use super::*; + fn fake_uploader_returning_error() -> MockAncillaryFileUploader { + let mut uploader = MockAncillaryFileUploader::new(); + uploader + .expect_upload() + .return_once(|_| Err(anyhow!("Failure while uploading..."))); + + uploader + } + + fn fake_uploader(archive_path: &str, location_uri: &str) -> MockAncillaryFileUploader { + let uri = location_uri.to_string(); + let filepath = archive_path.to_string(); + let mut uploader = MockAncillaryFileUploader::new(); + uploader + .expect_upload() + .withf(move |p| p == Path::new(&filepath)) + .times(1) + .return_once(|_| Ok(AncillaryLocation::CloudStorage { uri })); + + uploader + } + #[test] fn create_ancillary_builder_should_error_when_no_uploader() { let result = AncillaryArtifactBuilder::new( @@ -241,10 +262,7 @@ mod tests { #[tokio::test] async fn upload_ancillary_archive_should_error_when_no_location_is_returned() { - let mut uploader = MockAncillaryFileUploader::new(); - uploader - .expect_upload() - .return_once(|_| Err(anyhow!("Failure while uploading, no location returned"))); + let uploader = fake_uploader_returning_error(); let builder = AncillaryArtifactBuilder::new( vec![Arc::new(uploader)], @@ -265,29 +283,44 @@ mod tests { ); } + #[tokio::test] + async fn upload_ancillary_archive_should_return_location_even_with_uploaders_errors() { + let first_uploader = fake_uploader_returning_error(); + let second_uploader = fake_uploader("archive_path", "an_uri"); + let third_uploader = fake_uploader_returning_error(); + + let uploaders: Vec> = vec![ + Arc::new(first_uploader), + Arc::new(second_uploader), + Arc::new(third_uploader), + ]; + + let builder = AncillaryArtifactBuilder::new( + uploaders, + Arc::new(DumbSnapshotter::new()), + CardanoNetwork::DevNet(123), + CompressionAlgorithm::Gzip, + TestLogger::stdout(), + ) + .unwrap(); + + let locations = builder + .upload_ancillary_archive(Path::new("archive_path")) + .await + .unwrap(); + + assert_eq!( + locations, + vec![AncillaryLocation::CloudStorage { + uri: "an_uri".to_string() + }] + ); + } + #[tokio::test] async fn upload_ancillary_archive_should_return_all_uploaders_returned_locations() { - let mut first_uploader = MockAncillaryFileUploader::new(); - first_uploader - .expect_upload() - .with(eq(Path::new("archive_path"))) - .times(1) - .return_once(|_| { - Ok(AncillaryLocation::CloudStorage { - uri: "an_uri".to_string(), - }) - }); - - let mut second_uploader = MockAncillaryFileUploader::new(); - second_uploader - .expect_upload() - .with(eq(Path::new("archive_path"))) - .times(1) - .return_once(|_| { - Ok(AncillaryLocation::CloudStorage { - uri: "another_uri".to_string(), - }) - }); + let first_uploader = fake_uploader("archive_path", "an_uri"); + let second_uploader = fake_uploader("archive_path", "another_uri"); let uploaders: Vec> = vec![Arc::new(first_uploader), Arc::new(second_uploader)]; From 1e776880821a56cbcd49a8d0150659c777fa92a3 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:45:19 +0100 Subject: [PATCH 17/18] chore: reference the feature in the CHANGELOG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7ba18ac225..3fc770dcaa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ As a minor extension, we have adopted a slightly different versioning convention - **UNSTABLE** Cardano database incremental certification: - Implement the artifact routes of the aggregator for the signed entity type `CardanoDatabase`. + - Implement the artifact ancillary builder in the aggregator. - Crates versions: From 8d374ca29e86f0a8d202b0915297870b8a63c68e Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:46:56 +0100 Subject: [PATCH 18/18] chore: upgrade crate versions * mithril-aggregator from `0.6.8` to `0.6.9` * mithril-client from `0.10.6` to `0.10.7` * mithril-common from `0.4.100` to `0.4.101` * mithril-end-to-end from `0.4.59` to `0.4.60` --- Cargo.lock | 8 ++++---- mithril-aggregator/Cargo.toml | 2 +- mithril-client/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- mithril-test-lab/mithril-end-to-end/Cargo.toml | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee0b0741656..e93ae16a3ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3578,7 +3578,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.6.8" +version = "0.6.9" dependencies = [ "anyhow", "async-trait", @@ -3658,7 +3658,7 @@ dependencies = [ [[package]] name = "mithril-client" -version = "0.10.6" +version = "0.10.7" dependencies = [ "anyhow", "async-recursion", @@ -3738,7 +3738,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.4.100" +version = "0.4.101" dependencies = [ "anyhow", "async-trait", @@ -3809,7 +3809,7 @@ dependencies = [ [[package]] name = "mithril-end-to-end" -version = "0.4.59" +version = "0.4.60" dependencies = [ "anyhow", "async-recursion", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index e9b6a5bb73e..570c9f7fc88 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.6.8" +version = "0.6.9" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-client/Cargo.toml b/mithril-client/Cargo.toml index b8a28d2fe86..1541e78cc9d 100644 --- a/mithril-client/Cargo.toml +++ b/mithril-client/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-client" -version = "0.10.6" +version = "0.10.7" description = "Mithril client library" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index 8215e853543..dbc2be24905 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.4.100" +version = "0.4.101" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-test-lab/mithril-end-to-end/Cargo.toml b/mithril-test-lab/mithril-end-to-end/Cargo.toml index fd84c5dde14..1cd57f9f486 100644 --- a/mithril-test-lab/mithril-end-to-end/Cargo.toml +++ b/mithril-test-lab/mithril-end-to-end/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-end-to-end" -version = "0.4.59" +version = "0.4.60" authors = { workspace = true } edition = { workspace = true } documentation = { workspace = true }