From bb34765faf72183faaf563499c325c377ae53190 Mon Sep 17 00:00:00 2001 From: Ji Bin Date: Fri, 10 Sep 2021 18:15:22 +0800 Subject: [PATCH] Introducing s3 region parameter for aws s3 service. (#7419) - bugfix for #7325. Signed-off-by: Ji Bin --- core/conf/demo/server_config.yaml | 5 +++++ core/src/config/Config.cpp | 20 ++++++++++++++++++++ core/src/config/Config.h | 7 +++++++ core/src/storage/s3/S3ClientWrapper.cpp | 12 ++++++++++++ core/src/storage/s3/S3ClientWrapper.h | 1 + shards/all_in_one/ro_server.yml | 5 +++++ shards/all_in_one/wr_server.yml | 5 +++++ shards/all_in_one_with_mysql/ro_server.yml | 6 ++++++ shards/all_in_one_with_mysql/wr_server.yml | 6 ++++++ 9 files changed, 67 insertions(+) diff --git a/core/conf/demo/server_config.yaml b/core/conf/demo/server_config.yaml index a2643a92dc..c2d146d235 100644 --- a/core/conf/demo/server_config.yaml +++ b/core/conf/demo/server_config.yaml @@ -78,6 +78,11 @@ network: # | Note: please using differnet bucket for different milvus | | | # | cluster. | | | #----------------------+------------------------------------------------------------+------------+-----------------+ +# s3_region | The aws region for where bucket created. | String | "" | +# | This parameter may not needed for some self-managed s3, | | | +# | like ceph-radosgw, minio, when deploy on aws s3 service, | | | +# | this should be set correctly, e.g "ap-east-1", "us-east-1" | | | +#----------------------+------------------------------------------------------------+------------+-----------------+ storage: path: /var/lib/milvus auto_flush_interval: 1 diff --git a/core/src/config/Config.cpp b/core/src/config/Config.cpp index f39b768bf1..16af03c621 100644 --- a/core/src/config/Config.cpp +++ b/core/src/config/Config.cpp @@ -98,6 +98,8 @@ const char* CONFIG_STORAGE_S3_SECRET_KEY = "s3_secret_key"; const char* CONFIG_STORAGE_S3_SECRET_KEY_DEFAULT = ""; const char* CONFIG_STORAGE_S3_BUCKET = "s3_bucket"; const char* CONFIG_STORAGE_S3_BUCKET_DEFAULT = ""; +const char* CONFIG_STORAGE_S3_REGION = "s3_region"; +const char* CONFIG_STORAGE_S3_REGION_DEFAULT = ""; #endif const int64_t CONFIG_STORAGE_FILE_CLEANUP_TIMEOUT_MIN = 0; @@ -379,6 +381,9 @@ Config::ValidateConfig() { std::string storage_s3_bucket; STATUS_CHECK(GetStorageConfigS3Bucket(storage_s3_bucket)); + + std::string storage_s3_region; + STATUS_CHECK(GetStorageConfigS3Region(storage_s3_region)); #endif /* metric config */ @@ -1376,6 +1381,10 @@ Config::CheckStorageConfigS3Bucket(const std::string& value) { return Status::OK(); } +Status +Config::CheckStorageConfigS3Region(const std::string& /* unused */) { + return Status::OK(); +} #endif /* metric config */ @@ -2431,6 +2440,12 @@ Config::GetStorageConfigS3Bucket(std::string& value) { value = GetConfigStr(CONFIG_STORAGE, CONFIG_STORAGE_S3_BUCKET, CONFIG_STORAGE_S3_BUCKET_DEFAULT); return Status::OK(); } + +Status +Config::GetStorageConfigS3Region(std::string& value) { + value = GetConfigStr(CONFIG_STORAGE, CONFIG_STORAGE_S3_REGION, CONFIG_STORAGE_S3_REGION_DEFAULT); + return Status::OK(); +} #endif /* metric config */ @@ -2965,6 +2980,11 @@ Config::SetStorageConfigS3Bucket(const std::string& value) { return SetConfigValueInMem(CONFIG_STORAGE, CONFIG_STORAGE_S3_BUCKET, value); } +Status +Config::SetStorageConfigS3Region(const std::string& value) { + STATUS_CHECK(CheckStorageConfigS3Region(value)); + return SetConfigValueInMem(CONFIG_STORAGE, CONFIG_STORAGE_S3_REGION, value); +} #endif /* metric config */ diff --git a/core/src/config/Config.h b/core/src/config/Config.h index d8dfd23b18..03b6dd185c 100644 --- a/core/src/config/Config.h +++ b/core/src/config/Config.h @@ -281,6 +281,8 @@ class Config { CheckStorageConfigS3SecretKey(const std::string& value); Status CheckStorageConfigS3Bucket(const std::string& value); + Status + CheckStorageConfigS3Region(const std::string& value); #endif /* metric config */ @@ -446,6 +448,9 @@ class Config { Status GetStorageConfigS3Bucket(std::string& value); + + Status + GetStorageConfigS3Region(std::string& value); #endif /* metric config */ @@ -599,6 +604,8 @@ class Config { SetStorageConfigS3SecretKey(const std::string& value); Status SetStorageConfigS3Bucket(const std::string& value); + Status + SetStorageConfigS3Region(const std::string& value); #endif /* metric config */ diff --git a/core/src/storage/s3/S3ClientWrapper.cpp b/core/src/storage/s3/S3ClientWrapper.cpp index 53cfcb2f57..4f303bca3b 100644 --- a/core/src/storage/s3/S3ClientWrapper.cpp +++ b/core/src/storage/s3/S3ClientWrapper.cpp @@ -12,6 +12,7 @@ #include "storage/s3/S3ClientWrapper.h" #include +#include #include #include #include @@ -49,6 +50,7 @@ S3ClientWrapper::StartService() { config.GetStorageConfigS3AccessKey(s3_access_key_); config.GetStorageConfigS3SecretKey(s3_secret_key_); config.GetStorageConfigS3Bucket(s3_bucket_); + config.GetStorageConfigS3Region(s3_region_); Aws::InitAPI(options_); @@ -56,6 +58,9 @@ S3ClientWrapper::StartService() { cfg.endpointOverride = s3_address_ + ":" + s3_port_; cfg.scheme = Aws::Http::Scheme::HTTP; cfg.verifySSL = false; + if (!s3_region_.empty()) { + cfg.region = s3_region_.c_str(); + } client_ptr_ = std::make_shared(Aws::Auth::AWSCredentials(s3_access_key_, s3_secret_key_), cfg, Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Always, false); @@ -81,7 +86,14 @@ S3ClientWrapper::StopService() { Status S3ClientWrapper::CreateBucket() { Aws::S3::Model::CreateBucketRequest request; + auto constraint = Aws::S3::Model::BucketLocationConstraintMapper::GetBucketLocationConstraintForName(s3_region_); + Aws::S3::Model::CreateBucketConfiguration createBucketConfig; + createBucketConfig.WithLocationConstraint(constraint); + request.WithBucket(s3_bucket_); + if (!s3_region_.empty()) { + request.WithCreateBucketConfiguration(createBucketConfig); + } auto outcome = client_ptr_->CreateBucket(request); diff --git a/core/src/storage/s3/S3ClientWrapper.h b/core/src/storage/s3/S3ClientWrapper.h index ae3c98f0da..89d1a9a227 100644 --- a/core/src/storage/s3/S3ClientWrapper.h +++ b/core/src/storage/s3/S3ClientWrapper.h @@ -71,6 +71,7 @@ class S3ClientWrapper { std::string s3_access_key_; std::string s3_secret_key_; std::string s3_bucket_; + std::string s3_region_; }; } // namespace storage diff --git a/shards/all_in_one/ro_server.yml b/shards/all_in_one/ro_server.yml index 8d32011f9c..aa807dc761 100644 --- a/shards/all_in_one/ro_server.yml +++ b/shards/all_in_one/ro_server.yml @@ -78,6 +78,11 @@ network: # | Note: please using differnet bucket for different milvus | | | # | cluster. | | | #----------------------+------------------------------------------------------------+------------+-----------------+ +# s3_region | The aws region for where bucket created. | String | "" | +# | This parameter may not needed for some self-managed s3, | | | +# | like ceph-radosgw, minio, when deploy on aws s3 service, | | | +# | this should be set correctly, e.g "ap-east-1", "us-east-1" | | | +#----------------------+------------------------------------------------------------+------------+-----------------+ storage: path: /var/lib/milvus auto_flush_interval: 1 diff --git a/shards/all_in_one/wr_server.yml b/shards/all_in_one/wr_server.yml index e8ba840299..e83cf69be3 100644 --- a/shards/all_in_one/wr_server.yml +++ b/shards/all_in_one/wr_server.yml @@ -78,6 +78,11 @@ network: # | Note: please using differnet bucket for different milvus | | | # | cluster. | | | #----------------------+------------------------------------------------------------+------------+-----------------+ +# s3_region | The aws region for where bucket created. | String | "" | +# | This parameter may not needed for some self-managed s3, | | | +# | like ceph-radosgw, minio, when deploy on aws s3 service, | | | +# | this should be set correctly, e.g "ap-east-1", "us-east-1" | | | +#----------------------+------------------------------------------------------------+------------+-----------------+ storage: path: /var/lib/milvus auto_flush_interval: 1 diff --git a/shards/all_in_one_with_mysql/ro_server.yml b/shards/all_in_one_with_mysql/ro_server.yml index da02ca641e..d309a2dbcd 100644 --- a/shards/all_in_one_with_mysql/ro_server.yml +++ b/shards/all_in_one_with_mysql/ro_server.yml @@ -76,6 +76,12 @@ network: #----------------------+------------------------------------------------------------+------------+-----------------+ # s3_bucket | The s3 bucket name for store milvus's data. | String | s3_bucket | # | Note: please using differnet bucket for different milvus | | | +# | cluster. | | | +#----------------------+------------------------------------------------------------+------------+-----------------+ +# s3_region | The aws region for where bucket created. | String | "" | +# | This parameter may not needed for some self-managed s3, | | | +# | like ceph-radosgw, minio, when deploy on aws s3 service, | | | +# | this should be set correctly, e.g "ap-east-1", "us-east-1" | | | #----------------------+------------------------------------------------------------+------------+-----------------+ storage: path: /var/lib/milvus diff --git a/shards/all_in_one_with_mysql/wr_server.yml b/shards/all_in_one_with_mysql/wr_server.yml index 50cb10f8c8..6ffab74628 100644 --- a/shards/all_in_one_with_mysql/wr_server.yml +++ b/shards/all_in_one_with_mysql/wr_server.yml @@ -80,6 +80,12 @@ network: #----------------------+------------------------------------------------------------+------------+-----------------+ # s3_bucket | The s3 bucket name for store milvus's data. | String | s3_bucket | # | Note: please using differnet bucket for different milvus | | | +# | cluster. | | | +#----------------------+------------------------------------------------------------+------------+-----------------+ +# s3_region | The aws region for where bucket created. | String | "" | +# | This parameter may not needed for some self-managed s3, | | | +# | like ceph-radosgw, minio, when deploy on aws s3 service, | | | +# | this should be set correctly, e.g "ap-east-1", "us-east-1" | | | #----------------------+------------------------------------------------------------+------------+-----------------+ storage: path: /var/lib/milvus