From b7bb81bc3dfbf7090f0cb067c72b172dbed08d51 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 10 Aug 2023 18:11:12 +0700 Subject: [PATCH 1/3] improve remote-ext performance on slow connections --- utils/frame/remote-externalities/src/lib.rs | 35 ++++++++++++++------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 761f3c8859046..b2c59955364cd 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -42,7 +42,7 @@ pub use sp_io::TestExternalities; use sp_runtime::{traits::Block as BlockT, StateVersion}; use spinners::{Spinner, Spinners}; use std::{ - cmp::max, + cmp::{max, min}, fs, ops::{Deref, DerefMut}, path::{Path, PathBuf}, @@ -360,7 +360,8 @@ where const PARALLEL_REQUESTS: usize = 4; const BATCH_SIZE_INCREASE_FACTOR: f32 = 1.10; const BATCH_SIZE_DECREASE_FACTOR: f32 = 0.50; - const INITIAL_BATCH_SIZE: usize = 5000; + const REQUEST_DURATION_TARGET: Duration = Duration::from_secs(2); + const INITIAL_BATCH_SIZE: usize = 10; // nodes by default will not return more than 1000 keys per request const DEFAULT_KEY_DOWNLOAD_PAGE: u32 = 1000; const KEYS_PAGE_MAX_RETRIES: usize = 12; @@ -521,6 +522,7 @@ where .insert(method, params.clone()) .map_err(|_| "Invalid batch method and/or params")? } + let request_started = Instant::now(); let batch_response = match client.batch_request::>(batch).await { Ok(batch_response) => batch_response, Err(e) => { @@ -530,20 +532,31 @@ where log::debug!( target: LOG_TARGET, - "Batch request failed, trying again with smaller batch size. {}", + "Batch request failed, resetting batch size to 1. Error: {}", e.to_string() ); - return Self::get_storage_data_dynamic_batch_size( - client, - payloads, - max(1, (batch_size as f32 * Self::BATCH_SIZE_DECREASE_FACTOR) as usize), - bar, - ) - .await + // Request timed out or server errored. Try to get things moving again by starting + // again with just 1 item. + return Self::get_storage_data_dynamic_batch_size(client, payloads, 1, bar).await }, }; + // Request succeeded. Decide whether to increase or decrease the batch size for the next + // request, depending on if the elapsed time was greater than or less than the target. + let request_duration = request_started.elapsed(); + let next_batch_size = if request_duration > Self::REQUEST_DURATION_TARGET { + max(1, (batch_size as f32 * Self::BATCH_SIZE_DECREASE_FACTOR) as usize) + } else { + min(payloads.len(), (batch_size as f32 * Self::BATCH_SIZE_INCREASE_FACTOR) as usize) + }; + + log::debug!( + target: LOG_TARGET, + "Request duration: {:?} Target duration: {:?} Next Batch size: {}", + request_duration, Self::REQUEST_DURATION_TARGET, next_batch_size + ); + // Collect the data from this batch let mut data: Vec> = vec![]; let batch_response_len = batch_response.len(); @@ -560,7 +573,7 @@ where let mut rest = Self::get_storage_data_dynamic_batch_size( client, remaining_payloads, - max(batch_size + 1, (batch_size as f32 * Self::BATCH_SIZE_INCREASE_FACTOR) as usize), + next_batch_size, bar, ) .await?; From 5e8ba8036777ea1d1b41ffcc0d079827fccfdeaa Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 10 Aug 2023 19:45:17 +0700 Subject: [PATCH 2/3] increase by at least 1 --- utils/frame/remote-externalities/src/lib.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index b2c59955364cd..5eb30382bbf45 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -548,13 +548,21 @@ where let next_batch_size = if request_duration > Self::REQUEST_DURATION_TARGET { max(1, (batch_size as f32 * Self::BATCH_SIZE_DECREASE_FACTOR) as usize) } else { - min(payloads.len(), (batch_size as f32 * Self::BATCH_SIZE_INCREASE_FACTOR) as usize) + // Increase the batch size by *at most* the number of remaining payloads + min( + payloads.len(), + // Increase the batch size by *at least* 1 + max( + batch_size + 1, + (batch_size as f32 * Self::BATCH_SIZE_INCREASE_FACTOR) as usize, + ), + ) }; log::debug!( target: LOG_TARGET, - "Request duration: {:?} Target duration: {:?} Next Batch size: {}", - request_duration, Self::REQUEST_DURATION_TARGET, next_batch_size + "Request duration: {:?} Target duration: {:?} Last batch size: {} Next batch size: {}", + request_duration, Self::REQUEST_DURATION_TARGET, batch_size, next_batch_size ); // Collect the data from this batch From 920eb24c976460880fac5853a6a0aeb65f5e799d Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 10 Aug 2023 19:55:00 +0700 Subject: [PATCH 3/3] fix remote tests --- utils/frame/remote-externalities/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 5eb30382bbf45..d846b34a00973 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -1366,7 +1366,7 @@ mod remote_tests { .execute_with(|| {}); } - #[tokio::test(flavor = "multi_thread")] + #[tokio::test] async fn can_create_snapshot() { const CACHE: &'static str = "can_create_snapshot"; init_logger(); @@ -1390,7 +1390,7 @@ mod remote_tests { .filter(|p| p.path().file_name().unwrap_or_default() == CACHE) .collect::>(); - let snap: Snapshot = Builder::::new().load_snapshot(CACHE.into()).unwrap(); + let snap: Snapshot = Snapshot::load(&PathBuf::from(CACHE)).unwrap(); assert!(matches!(snap, Snapshot { raw_storage, .. } if raw_storage.len() > 0)); assert!(to_delete.len() == 1); @@ -1422,7 +1422,7 @@ mod remote_tests { .filter(|p| p.path().file_name().unwrap_or_default() == CACHE) .collect::>(); - let snap: Snapshot = Builder::::new().load_snapshot(CACHE.into()).unwrap(); + let snap: Snapshot = Snapshot::load(&PathBuf::from(CACHE)).unwrap(); assert!(matches!(snap, Snapshot { raw_storage, .. } if raw_storage.len() > 0)); assert!(to_delete.len() == 1);