From 16f06222228ffeaef9d3bbfe701035d2b78bf20d Mon Sep 17 00:00:00 2001 From: Joonas Koivunen Date: Tue, 26 Sep 2023 17:59:25 +0300 Subject: [PATCH] fix: real_s3 flakyness with rust tests (#5386) Fixes #5072. See proof from https://github.com/neondatabase/neon/issues/5072#issuecomment-1735580798. Turns out multiple threads can get the same nanoseconds since epoch, so switch to using millis (for finding the prefix later on) and randomness via `thread_rng` (protect against adversial ci runners). Also changes the "per test looking alike" prefix to more "general" prefix. --- Cargo.lock | 1 + libs/remote_storage/Cargo.toml | 1 + libs/remote_storage/tests/test_real_s3.rs | 15 ++++++++++++--- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2055f001af..55c80e30a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3417,6 +3417,7 @@ dependencies = [ "metrics", "once_cell", "pin-project-lite", + "rand", "scopeguard", "serde", "serde_json", diff --git a/libs/remote_storage/Cargo.toml b/libs/remote_storage/Cargo.toml index a4adae6146..2b808779f4 100644 --- a/libs/remote_storage/Cargo.toml +++ b/libs/remote_storage/Cargo.toml @@ -29,3 +29,4 @@ workspace_hack.workspace = true [dev-dependencies] tempfile.workspace = true test-context.workspace = true +rand.workspace = true diff --git a/libs/remote_storage/tests/test_real_s3.rs b/libs/remote_storage/tests/test_real_s3.rs index 982c01a9be..b220349829 100644 --- a/libs/remote_storage/tests/test_real_s3.rs +++ b/libs/remote_storage/tests/test_real_s3.rs @@ -378,21 +378,30 @@ impl AsyncTestContext for MaybeEnabledS3WithSimpleTestBlobs { fn create_s3_client( max_keys_per_list_response: Option, ) -> anyhow::Result> { + use rand::Rng; + let remote_storage_s3_bucket = env::var("REMOTE_STORAGE_S3_BUCKET") .context("`REMOTE_STORAGE_S3_BUCKET` env var is not set, but real S3 tests are enabled")?; let remote_storage_s3_region = env::var("REMOTE_STORAGE_S3_REGION") .context("`REMOTE_STORAGE_S3_REGION` env var is not set, but real S3 tests are enabled")?; - let random_prefix_part = std::time::SystemTime::now() + + // due to how time works, we've had test runners use the same nanos as bucket prefixes. + // millis is just a debugging aid for easier finding the prefix later. + let millis = std::time::SystemTime::now() .duration_since(UNIX_EPOCH) .context("random s3 test prefix part calculation")? - .as_nanos(); + .as_millis(); + + // because nanos can be the same for two threads so can millis, add randomness + let random = rand::thread_rng().gen::(); + let remote_storage_config = RemoteStorageConfig { max_concurrent_syncs: NonZeroUsize::new(100).unwrap(), max_sync_errors: NonZeroU32::new(5).unwrap(), storage: RemoteStorageKind::AwsS3(S3Config { bucket_name: remote_storage_s3_bucket, bucket_region: remote_storage_s3_region, - prefix_in_bucket: Some(format!("pagination_should_work_test_{random_prefix_part}/")), + prefix_in_bucket: Some(format!("test_{millis}_{random:08x}/")), endpoint: None, concurrency_limit: NonZeroUsize::new(100).unwrap(), max_keys_per_list_response,