From 02a1d4d8c19248ba8cafb4295e060812c4b5293e Mon Sep 17 00:00:00 2001 From: Alek Westover Date: Wed, 21 Jun 2023 11:32:04 -0400 Subject: [PATCH] refactoring a bit --- ALEK_LIST_FILES.txt | 2 +- compute_tools/src/bin/compute_ctl.rs | 33 +++----------- compute_tools/src/compute.rs | 5 +-- compute_tools/src/extension_server.rs | 44 +++++++++++-------- compute_tools/src/http/api.rs | 10 +---- .../regress/test_download_extensions.py | 3 -- 6 files changed, 35 insertions(+), 62 deletions(-) diff --git a/ALEK_LIST_FILES.txt b/ALEK_LIST_FILES.txt index b541f04f94..a1419d1429 100644 --- a/ALEK_LIST_FILES.txt +++ b/ALEK_LIST_FILES.txt @@ -1 +1 @@ -[RemotePath("tenants/d18ac7eff5c6c14559041ac7b8a94506/timelines/87ce5cd6c70a22502fbbc408374e437c/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/d18ac7eff5c6c14559041ac7b8a94506/timelines/87ce5cd6c70a22502fbbc408374e437c/index_part.json"), RemotePath("tenants/d18ac7eff5c6c14559041ac7b8a94506/timelines/ac5a22da4cca24f99a359ea01970d5e6/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/d18ac7eff5c6c14559041ac7b8a94506/timelines/ac5a22da4cca24f99a359ea01970d5e6/index_part.json"), RemotePath("tenants/d76683c8997d65d455a61c195ead377c/timelines/38134bfd974b3de3b6f8b38726cd944d/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/d76683c8997d65d455a61c195ead377c/timelines/38134bfd974b3de3b6f8b38726cd944d/index_part.json"), RemotePath("v15/share/extension/test_ext.control")] \ No newline at end of file +[RemotePath("tenants/9a8ce821f7946ed2f2d58f51f2595024/timelines/62cdc8653864e444171faac9a5c3cea9/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/9a8ce821f7946ed2f2d58f51f2595024/timelines/62cdc8653864e444171faac9a5c3cea9/index_part.json"), RemotePath("tenants/a9982e09ea4a00aff9c61daf12744098/timelines/55efa350b38ff1a9df45726dbaadbb9f/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/a9982e09ea4a00aff9c61daf12744098/timelines/55efa350b38ff1a9df45726dbaadbb9f/index_part.json"), RemotePath("tenants/a9982e09ea4a00aff9c61daf12744098/timelines/a0af9d76650e3477b7bd1a1e8e2793bf/000000000000000000000000000000000000-FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF__000000000169B098-000000000169B111"), RemotePath("tenants/a9982e09ea4a00aff9c61daf12744098/timelines/a0af9d76650e3477b7bd1a1e8e2793bf/index_part.json"), RemotePath("v15/share/extension/test_ext.control")] \ No newline at end of file diff --git a/compute_tools/src/bin/compute_ctl.rs b/compute_tools/src/bin/compute_ctl.rs index 37a3889666..e937a9ba2b 100644 --- a/compute_tools/src/bin/compute_ctl.rs +++ b/compute_tools/src/bin/compute_ctl.rs @@ -27,7 +27,8 @@ //! compute_ctl -D /var/db/postgres/compute \ //! -C 'postgresql://cloud_admin@localhost/postgres' \ //! -S /var/db/postgres/specs/current.json \ -//! -b /usr/local/bin/postgres +//! -b /usr/local/bin/postgres \ +//! -r {"bucket": "my-bucket", "region": "eu-central-1"} //! ``` //! use std::collections::HashMap; @@ -41,7 +42,6 @@ use std::{thread, time::Duration}; use anyhow::{Context, Result}; use chrono::Utc; use clap::Arg; -use serde_json::{self, Value}; use tracing::{error, info}; use url::Url; @@ -49,6 +49,7 @@ use compute_api::responses::ComputeStatus; use compute_tools::compute::{ComputeNode, ComputeState, ParsedSpec}; use compute_tools::configurator::launch_configurator; +use compute_tools::extension_server::download_file; use compute_tools::http::api::launch_http_server; use compute_tools::logger::*; use compute_tools::monitor::launch_monitor; @@ -65,30 +66,12 @@ fn main() -> Result<()> { let remote_ext_config = matches .get_one::("remote-ext-config") .expect("remote-extension-config is required"); - let remote_ext_config: serde_json::Value = serde_json::from_str(&remote_ext_config)?; - let remote_ext_bucket = match &remote_ext_config["bucket"] { - Value::String(x) => x, - _ => panic!("oops"), - }; - let remote_ext_region = match &remote_ext_config["region"] { - Value::String(x) => x, - _ => panic!("oops"), - }; - let remote_ext_endpoint = match &remote_ext_config["endpoint"] { - Value::String(x) => x, - _ => panic!("oops"), - }; let rt = Runtime::new().unwrap(); rt.block_on(async move { - compute_tools::extension_server::download_file( - "test_ext.control", - remote_ext_bucket.into(), - remote_ext_region.into(), - remote_ext_endpoint.into(), - ) - .await - .expect("download should work"); + download_file("test_ext.control", remote_ext_config) + .await + .expect("download should work"); }); let http_port = *matches @@ -208,9 +191,7 @@ fn main() -> Result<()> { live_config_allowed, state: Mutex::new(new_state), state_changed: Condvar::new(), - remote_ext_bucket: remote_ext_bucket.clone(), // TODO: pass more configurations? - remote_ext_region: remote_ext_region.clone(), - remote_ext_endpoint: remote_ext_endpoint.clone(), + remote_ext_config: remote_ext_config.clone(), }; let compute = Arc::new(compute_node); diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 7117e82143..1e166c45b0 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -46,10 +46,7 @@ pub struct ComputeNode { /// `Condvar` to allow notifying waiters about state changes. pub state_changed: Condvar, // S3 configuration variables: - // TODO: pass more args here? - pub remote_ext_bucket: String, - pub remote_ext_region: String, - pub remote_ext_endpoint: String, + pub remote_ext_config: String, } #[derive(Clone, Debug)] diff --git a/compute_tools/src/extension_server.rs b/compute_tools/src/extension_server.rs index 4774697fdb..14f05a2c54 100644 --- a/compute_tools/src/extension_server.rs +++ b/compute_tools/src/extension_server.rs @@ -1,5 +1,6 @@ use anyhow::{self}; use remote_storage::*; +use serde_json::{self, Value}; use std::fs::File; use std::io::{BufWriter, Write}; use std::num::{NonZeroU32, NonZeroUsize}; @@ -8,14 +9,9 @@ use std::str; use tokio::io::AsyncReadExt; use tracing::info; -pub async fn download_file( - filename: &str, - remote_ext_bucket: String, - remote_ext_region: String, - remote_ext_endpoint: String, -) -> anyhow::Result<()> { - println!("requested file {}", filename); - let s3_config = create_s3_config(remote_ext_bucket, remote_ext_region, remote_ext_endpoint); +// TODO: get rid of this function by making s3_config part of ComputeNode +pub async fn download_file(filename: &str, remote_ext_config: &str) -> anyhow::Result<()> { + let s3_config = create_s3_config(remote_ext_config)?; download_extension(&s3_config, ExtensionType::Shared).await?; Ok(()) } @@ -107,23 +103,33 @@ pub async fn download_extension( Ok(()) } -// TODO: add support for more of these parameters being configurable? -pub fn create_s3_config( - remote_ext_bucket: String, - remote_ext_region: String, - remote_ext_endpoint: String, -) -> RemoteStorageConfig { +pub fn create_s3_config(remote_ext_config: &str) -> anyhow::Result { + let remote_ext_config: serde_json::Value = serde_json::from_str(remote_ext_config)?; + let remote_ext_bucket = match &remote_ext_config["bucket"] { + Value::String(x) => x, + _ => panic!("oops"), + }; + let remote_ext_region = match &remote_ext_config["region"] { + Value::String(x) => x, + _ => panic!("oops"), + }; + let remote_ext_endpoint = match &remote_ext_config["endpoint"] { + Value::String(x) => Some(x.clone()), + _ => None, + }; + + // TODO: add support for more of these parameters being configurable? let config = S3Config { - bucket_name: remote_ext_bucket, - bucket_region: remote_ext_region, + bucket_name: remote_ext_bucket.clone(), + bucket_region: remote_ext_region.clone(), prefix_in_bucket: None, - endpoint: Some(remote_ext_endpoint), + endpoint: remote_ext_endpoint, concurrency_limit: NonZeroUsize::new(100).expect("100 != 0"), max_keys_per_list_response: None, }; - RemoteStorageConfig { + Ok(RemoteStorageConfig { max_concurrent_syncs: NonZeroUsize::new(100).expect("100 != 0"), max_sync_errors: NonZeroU32::new(100).expect("100 != 0"), storage: RemoteStorageKind::AwsS3(config), - } + }) } diff --git a/compute_tools/src/http/api.rs b/compute_tools/src/http/api.rs index cf18c84b5f..f936e28e17 100644 --- a/compute_tools/src/http/api.rs +++ b/compute_tools/src/http/api.rs @@ -134,15 +134,7 @@ async fn routes(req: Request, compute: &Arc) -> Response Response::new(Body::from("OK")), Err(e) => { error!("download_file failed: {}", e); diff --git a/test_runner/regress/test_download_extensions.py b/test_runner/regress/test_download_extensions.py index 84ae31e292..7b63783a07 100644 --- a/test_runner/regress/test_download_extensions.py +++ b/test_runner/regress/test_download_extensions.py @@ -1,11 +1,8 @@ from contextlib import closing -from typing import List from fixtures.log_helper import log from fixtures.neon_fixtures import ( - NeonEnv, NeonEnvBuilder, RemoteStorageKind, - available_remote_storages, ) import json