From db10b2d80506f5f8a64e37974847d857f2c57ad3 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Mon, 30 May 2022 13:38:03 +0800 Subject: [PATCH 1/2] feat(query): Fail fast if the underlying storage is not available Signed-off-by: Xuanwo --- common/exception/src/exception_code.rs | 3 ++- query/src/sessions/session_mgr.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/common/exception/src/exception_code.rs b/common/exception/src/exception_code.rs index d550a4cb444f..e3179fc68e2c 100644 --- a/common/exception/src/exception_code.rs +++ b/common/exception/src/exception_code.rs @@ -228,7 +228,8 @@ build_exceptions! { build_exceptions! { StorageNotFound(3001), StoragePermissionDenied(3002), - StorageOther(4000) + StorageUnavailable(3901), + StorageOther(4000), } // Cache errors [4001, 5000]. diff --git a/query/src/sessions/session_mgr.rs b/query/src/sessions/session_mgr.rs index c42d7fc6abdc..d9605f8365af 100644 --- a/query/src/sessions/session_mgr.rs +++ b/query/src/sessions/session_mgr.rs @@ -363,9 +363,17 @@ impl SessionManager { // Init the storage operator by config. async fn init_storage_operator(conf: &Config) -> Result { let op = init_operator(&conf.storage).await?; - // Enable exponential backoff by default - Ok(op.with_backoff(backon::ExponentialBackoff::default())) + let op = op.with_backoff(backon::ExponentialBackoff::default()); + // OpenDAL will send a real request to underlying storage to check whether it works or not. + // If this check failed, it's highly possible that the users have configured it wrongly. + op.check().await.map_err(|e| { + ErrorCode::StorageUnavailable(format!( + "current configured storage is not available: {e}" + )) + })?; + + Ok(op) } pub async fn reload_config(&self) -> Result<()> { From 50843d543443f25c395b59881486bdca862e6d11 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Mon, 30 May 2022 15:44:05 +0800 Subject: [PATCH 2/2] Introduce mock for unittests Signed-off-by: Xuanwo --- query/tests/it/sessions/query_ctx.rs | 15 ++++++++++-- .../tests/it/storages/system/configs_table.rs | 24 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/query/tests/it/sessions/query_ctx.rs b/query/tests/it/sessions/query_ctx.rs index 79efe82a29de..9e317fa33dac 100644 --- a/query/tests/it/sessions/query_ctx.rs +++ b/query/tests/it/sessions/query_ctx.rs @@ -17,15 +17,26 @@ use common_exception::Result; use common_io::prelude::StorageFsConfig; use common_io::prelude::StorageParams; use common_io::prelude::StorageS3Config; +use wiremock::matchers::method; +use wiremock::matchers::path; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] -// This test need network async fn test_get_storage_accessor_s3() -> Result<()> { + let mock_server = MockServer::start().await; + Mock::given(method("HEAD")) + .and(path("/bucket/.opendal")) + .respond_with(ResponseTemplate::new(404)) + .mount(&mock_server) + .await; + let mut conf = crate::tests::ConfigBuilder::create().config(); conf.storage.params = StorageParams::S3(StorageS3Config { region: "us-east-2".to_string(), - endpoint_url: "http://s3.amazonaws.com".to_string(), + endpoint_url: mock_server.uri(), access_key_id: "".to_string(), secret_access_key: "".to_string(), bucket: "bucket".to_string(), diff --git a/query/tests/it/storages/system/configs_table.rs b/query/tests/it/storages/system/configs_table.rs index d968deaaacab..e5a21b84dd98 100644 --- a/query/tests/it/storages/system/configs_table.rs +++ b/query/tests/it/storages/system/configs_table.rs @@ -20,6 +20,11 @@ use databend_query::storages::system::ConfigsTable; use databend_query::storages::ToReadDataSourcePlan; use futures::TryStreamExt; use pretty_assertions::assert_eq; +use wiremock::matchers::method; +use wiremock::matchers::path; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_configs_table() -> Result<()> { @@ -112,8 +117,17 @@ async fn test_configs_table() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_configs_table_redact() -> Result<()> { + let mock_server = MockServer::start().await; + Mock::given(method("HEAD")) + .and(path("/test/.opendal")) + .respond_with(ResponseTemplate::new(404)) + .mount(&mock_server) + .await; + let mut conf = crate::tests::ConfigBuilder::create().config(); conf.storage.params = StorageParams::S3(StorageS3Config { + region: "us-east-2".to_string(), + endpoint_url: mock_server.uri(), bucket: "test".to_string(), access_key_id: "access_key_id".to_string(), secret_access_key: "secret_access_key".to_string(), @@ -130,6 +144,11 @@ async fn test_configs_table_redact() -> Result<()> { let block = &result[0]; assert_eq!(block.num_columns(), 4); + let endpoint_url_link = format!( + "| storage | s3.endpoint_url | {:<24} | |", + mock_server.uri() + ); + let expected = vec![ "+---------+--------------------------------------+---------------------------+-------------+", "| group | name | value | description |", @@ -193,14 +212,15 @@ async fn test_configs_table_redact() -> Result<()> { "| storage | num_cpus | 0 | |", "| storage | s3.access_key_id | ******_id | |", "| storage | s3.bucket | test | |", - "| storage | s3.endpoint_url | https://s3.amazonaws.com | |", + &endpoint_url_link, "| storage | s3.master_key | | |", - "| storage | s3.region | | |", + "| storage | s3.region | us-east-2 | |", "| storage | s3.root | | |", "| storage | s3.secret_access_key | ******key | |", "| storage | type | s3 | |", "+---------+--------------------------------------+---------------------------+-------------+", ]; + common_datablocks::assert_blocks_sorted_eq(expected, result.as_slice()); Ok(()) }