From c73e9e40e96170186776f33ed8aa45814254c98e Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Wed, 20 Nov 2024 18:30:48 +0100 Subject: [PATCH] try async-timer 1.0.0-beta15 (still signal-based timers) Results unchanged to 0.7.4 https://www.notion.so/neondatabase/benchmarking-notes-143f189e004780c4a630cb5f426e39ba?pvs=4#144f189e004780e18416cc0faf2aca65 --- Cargo.lock | 13 ++++++++++--- Cargo.toml | 2 +- pageserver/src/page_service.rs | 15 ++++----------- .../pageserver/test_pageserver_getpage_merge.py | 4 +++- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9196015057..e4f6a87b76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -246,13 +246,14 @@ dependencies = [ [[package]] name = "async-timer" -version = "0.7.4" +version = "1.0.0-beta.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba5fa6ed76cb2aa820707b4eb9ec46f42da9ce70b0eafab5e5e34942b38a44d5" +checksum = "1d420af8e042475e58a20d91af8eda7d6528989418c03f3f527e1c3415696f70" dependencies = [ + "error-code", "libc", "wasm-bindgen", - "winapi", + "web-time", ] [[package]] @@ -1931,6 +1932,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "error-code" +version = "3.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5d9305ccc6942a704f4335694ecd3de2ea531b114ac2d51f5f843750787a92f" + [[package]] name = "event-listener" version = "2.5.3" diff --git a/Cargo.toml b/Cargo.toml index d74efd51f8..bf4ed0ed82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ anyhow = { version = "1.0", features = ["backtrace"] } arc-swap = "1.6" async-compression = { version = "0.4.0", features = ["tokio", "gzip", "zstd"] } atomic-take = "1.1.0" -async-timer = "0.7.4" +async-timer = "1.0.0-beta.15" azure_core = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls", "hmac_rust"] } azure_identity = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls"] } azure_storage = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls"] } diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index c32f889958..3e01887355 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -3,7 +3,7 @@ use anyhow::{bail, Context}; use async_compression::tokio::write::GzipEncoder; -use async_timer::Oneshot; +use async_timer::Timer; use bytes::Buf; use futures::FutureExt; use itertools::Itertools; @@ -319,7 +319,7 @@ struct PageServerHandler { /// See [`PageServerConf::server_side_batch_timeout`] server_side_batch_timeout: Option, - server_side_batch_timer: Pin>, + server_side_batch_timer: Pin>, } struct Carry { @@ -589,9 +589,7 @@ impl PageServerHandler { timeline_handles: TimelineHandles::new(tenant_manager), cancel, server_side_batch_timeout, - server_side_batch_timer: Box::pin(async_timer::oneshot::Timer::new( - Duration::from_secs(999), - )), // reset each iteration + server_side_batch_timer: Box::pin(async_timer::new_timer(Duration::from_secs(999))), // reset each iteration } } @@ -668,12 +666,7 @@ impl PageServerHandler { .msg ])); } else { - std::future::poll_fn(|ctx| { - self.server_side_batch_timer - .restart(batch_timeout, ctx.waker()); - std::task::Poll::Ready(()) - }) - .await; + self.server_side_batch_timer.restart(batch_timeout); batching_deadline_storage = Some(&mut self.server_side_batch_timer); Either::Right( batching_deadline_storage.as_mut().expect("we just set it"), diff --git a/test_runner/performance/pageserver/test_pageserver_getpage_merge.py b/test_runner/performance/pageserver/test_pageserver_getpage_merge.py index be7cf66c79..850bc1b8df 100644 --- a/test_runner/performance/pageserver/test_pageserver_getpage_merge.py +++ b/test_runner/performance/pageserver/test_pageserver_getpage_merge.py @@ -9,7 +9,7 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnvBuilder from fixtures.utils import humantime_to_ms -TARGET_RUNTIME = 60 +TARGET_RUNTIME = 5 @pytest.mark.parametrize( @@ -18,10 +18,12 @@ TARGET_RUNTIME = 60 # the next 4 cases demonstrate how not-batchable workloads suffer from batching timeout (50, None, TARGET_RUNTIME, 1, 128, "not batchable no batching"), (50, "10us", TARGET_RUNTIME, 1, 128, "not batchable 10us timeout"), + (50, "20us", TARGET_RUNTIME, 1, 128, "not batchable 20us timeout"), (50, "1ms", TARGET_RUNTIME, 1, 128, "not batchable 1ms timeout"), # the next 4 cases demonstrate how batchable workloads benefit from batching (50, None, TARGET_RUNTIME, 100, 128, "batchable no batching"), (50, "10us", TARGET_RUNTIME, 100, 128, "batchable 10us timeout"), + (50, "20us", TARGET_RUNTIME, 100, 128, "batchable 20us timeout"), (50, "100us", TARGET_RUNTIME, 100, 128, "batchable 100us timeout"), (50, "1ms", TARGET_RUNTIME, 100, 128, "batchable 1ms timeout"), ],