Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions mooncake-common/include/environ.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,26 +68,6 @@ class Environ {
int GetEfaCqThreads() const { return efa_cq_threads_; }
bool GetStoreChecksumEnabled() const { return store_checksum_enabled_; }

// AWS / S3 client configuration
std::string GetAwsRegion() const { return aws_region_; }
std::string GetAwsS3Endpoint() const { return aws_s3_endpoint_; }
std::string GetAwsBucketName() const { return aws_bucket_name_; }
std::string GetAwsAccessKeyId() const { return aws_access_key_id_; }
std::string GetAwsSecretAccessKey() const { return aws_secret_access_key_; }
bool GetAwsUseVirtualAddressing() const {
return aws_use_virtual_addressing_;
}
bool GetAwsUseHttps() const { return aws_use_https_; }
// Empty string means "unset" — s3_helper keeps the AWS SDK default in
// that case. Parsing to AWS enums is done by the consumer.
std::string GetAwsRequestChecksumCalculation() const {
return aws_request_checksum_calculation_;
}
std::string GetAwsResponseChecksumValidation() const {
return aws_response_checksum_validation_;
}
int64_t GetAwsConnectTimeoutMs() const { return aws_connect_timeout_ms_; }
int64_t GetAwsRequestTimeoutMs() const { return aws_request_timeout_ms_; }
uint32_t GetRpcClientIoThreads() const { return rpc_client_io_threads_; }
uint32_t GetStoreRpcClientIoThreads() const {
return store_rpc_client_io_threads_;
Expand Down Expand Up @@ -183,19 +163,6 @@ class Environ {
uint32_t rpc_client_io_threads_;
uint32_t store_rpc_client_io_threads_;
uint32_t transfer_engine_rpc_client_io_threads_;

// AWS / S3 client configuration
std::string aws_region_;
std::string aws_s3_endpoint_;
std::string aws_bucket_name_;
std::string aws_access_key_id_;
std::string aws_secret_access_key_;
bool aws_use_virtual_addressing_;
bool aws_use_https_;
std::string aws_request_checksum_calculation_;
std::string aws_response_checksum_validation_;
int64_t aws_connect_timeout_ms_;
int64_t aws_request_timeout_ms_;
};

} // namespace mooncake
14 changes: 14 additions & 0 deletions mooncake-common/include/environment_variables.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,20 @@ struct NvmeKvIoConcurrencyEnvironmentVariables {
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_NVME_KV_PREPARE_CONCURRENCY);
};

struct S3ClientEnvironmentVariables {
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_REGION);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_S3_ENDPOINT);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_BUCKET_NAME);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_ACCESS_KEY_ID);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_SECRET_ACCESS_KEY);
MC_DEFINE_ENV_VAR(bool, MOONCAKE_AWS_USE_VIRTUAL_ADDRESSING);
MC_DEFINE_ENV_VAR(bool, MOONCAKE_AWS_USE_HTTPS);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_REQUEST_CHECKSUM_CALCULATION);
MC_DEFINE_ENV_VAR(std::string, MOONCAKE_AWS_RESPONSE_CHECKSUM_VALIDATION);
MC_DEFINE_ENV_VAR(int64_t, MOONCAKE_AWS_CONNECT_TIMEOUT_MS);
MC_DEFINE_ENV_VAR(int64_t, MOONCAKE_AWS_REQUEST_TIMEOUT_MS);
};

#undef MC_DEFINE_ENV_VAR

} // namespace mooncake
21 changes: 0 additions & 21 deletions mooncake-common/src/environ.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,27 +202,6 @@ Environ::Environ(const EnvironSource& source) {
efa_cq_threads_ = ReadInt(source, "MC_EFA_CQ_THREADS", 1);
store_checksum_enabled_ =
ReadBool(source, "MOONCAKE_STORE_CHECKSUM", false);

// AWS / S3 client configuration (consumed by s3_helper.cpp)
aws_region_ = ReadString(source, "MOONCAKE_AWS_REGION", "");
aws_s3_endpoint_ = ReadString(source, "MOONCAKE_AWS_S3_ENDPOINT", "");
aws_bucket_name_ = ReadString(source, "MOONCAKE_AWS_BUCKET_NAME", "");
aws_access_key_id_ = ReadString(source, "MOONCAKE_AWS_ACCESS_KEY_ID", "");
aws_secret_access_key_ =
ReadString(source, "MOONCAKE_AWS_SECRET_ACCESS_KEY", "");
aws_use_virtual_addressing_ =
ReadBool(source, "MOONCAKE_AWS_USE_VIRTUAL_ADDRESSING", true);
aws_use_https_ = ReadBool(source, "MOONCAKE_AWS_USE_HTTPS", true);
// Empty string preserves "unset" semantics — s3_helper keeps the AWS SDK
// default in that case rather than forcing a value.
aws_request_checksum_calculation_ =
ReadString(source, "MOONCAKE_AWS_REQUEST_CHECKSUM_CALCULATION", "");
aws_response_checksum_validation_ =
ReadString(source, "MOONCAKE_AWS_RESPONSE_CHECKSUM_VALIDATION", "");
aws_connect_timeout_ms_ =
ReadInt64(source, "MOONCAKE_AWS_CONNECT_TIMEOUT_MS", 10000);
aws_request_timeout_ms_ =
ReadInt64(source, "MOONCAKE_AWS_REQUEST_TIMEOUT_MS", 30000);
}

} // namespace mooncake
47 changes: 1 addition & 46 deletions mooncake-common/tests/environ_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,6 @@ class EnvironTest : public ::testing::Test {
unsetenv("MC_TEST_DOUBLE");
unsetenv("MC_TEST_BOOL");
unsetenv("MC_TEST_STRING");
// Make sure AWS vars don't leak in from the test runner's env.
unsetenv("MOONCAKE_AWS_REGION");
unsetenv("MOONCAKE_AWS_S3_ENDPOINT");
unsetenv("MOONCAKE_AWS_BUCKET_NAME");
unsetenv("MOONCAKE_AWS_ACCESS_KEY_ID");
unsetenv("MOONCAKE_AWS_SECRET_ACCESS_KEY");
unsetenv("MOONCAKE_AWS_USE_VIRTUAL_ADDRESSING");
unsetenv("MOONCAKE_AWS_USE_HTTPS");
unsetenv("MOONCAKE_AWS_REQUEST_CHECKSUM_CALCULATION");
unsetenv("MOONCAKE_AWS_RESPONSE_CHECKSUM_VALIDATION");
unsetenv("MOONCAKE_AWS_CONNECT_TIMEOUT_MS");
unsetenv("MOONCAKE_AWS_REQUEST_TIMEOUT_MS");
unsetenv("MOONCAKE_STORE_CHECKSUM");
}
};
Expand Down Expand Up @@ -157,43 +145,10 @@ TEST_F(EnvironTest, GetDoubleMissingOrInvalidUsesRequestedDefault) {
EXPECT_DOUBLE_EQ(Environ::GetDouble("MC_TEST_DOUBLE", 0.5), 0.5);
}

// --- AWS / S3 fields ---
//
// NOTE: Environ is a singleton whose constructor caches every value the
// first time Get() is called. So all AWS env vars must be set BEFORE the
// first Environ::Get() in this process. We therefore cover the populate
// path in a single test that takes the singleton's "first call" for
// itself; the default-path behavior is implicitly covered by Environ's
// constructor defaults (any earlier test would lock the cache to defaults
// and prevent us from observing populated values here).

TEST_F(EnvironTest, AwsFieldsPopulateFromEnv) {
setenv("MOONCAKE_AWS_REGION", "us-east-1", 1);
setenv("MOONCAKE_AWS_S3_ENDPOINT", "https://s3.example.com", 1);
setenv("MOONCAKE_AWS_BUCKET_NAME", "my-bucket", 1);
setenv("MOONCAKE_AWS_ACCESS_KEY_ID", "AKIA-test", 1);
setenv("MOONCAKE_AWS_SECRET_ACCESS_KEY", "secret", 1);
setenv("MOONCAKE_AWS_USE_VIRTUAL_ADDRESSING", "0", 1);
setenv("MOONCAKE_AWS_USE_HTTPS", "0", 1);
setenv("MOONCAKE_AWS_REQUEST_CHECKSUM_CALCULATION", "when_required", 1);
setenv("MOONCAKE_AWS_RESPONSE_CHECKSUM_VALIDATION", "when_supported", 1);
setenv("MOONCAKE_AWS_CONNECT_TIMEOUT_MS", "5000", 1);
// Bogus request timeout should fall back to the registered default.
setenv("MOONCAKE_AWS_REQUEST_TIMEOUT_MS", "bogus", 1);
TEST_F(EnvironTest, StoreChecksumPopulatesFromEnv) {
setenv("MOONCAKE_STORE_CHECKSUM", "1", 1);

const auto& e = Environ::Get();
EXPECT_EQ(e.GetAwsRegion(), "us-east-1");
EXPECT_EQ(e.GetAwsS3Endpoint(), "https://s3.example.com");
EXPECT_EQ(e.GetAwsBucketName(), "my-bucket");
EXPECT_EQ(e.GetAwsAccessKeyId(), "AKIA-test");
EXPECT_EQ(e.GetAwsSecretAccessKey(), "secret");
EXPECT_FALSE(e.GetAwsUseVirtualAddressing());
EXPECT_FALSE(e.GetAwsUseHttps());
EXPECT_EQ(e.GetAwsRequestChecksumCalculation(), "when_required");
EXPECT_EQ(e.GetAwsResponseChecksumValidation(), "when_supported");
EXPECT_EQ(e.GetAwsConnectTimeoutMs(), 5000);
EXPECT_EQ(e.GetAwsRequestTimeoutMs(), 30000);
EXPECT_TRUE(e.GetStoreChecksumEnabled());
}

Expand Down
1 change: 1 addition & 0 deletions mooncake-store/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ set(MOONCAKE_STORE_MASTER_SOURCES
allocator.cpp
config/local_file_snapshot_config.cpp
config/redis_connection_config.cpp
config/s3_client_config.cpp
segment_allocator_registration.cpp
client_offboarding.cpp
master_service.cpp
Expand Down
45 changes: 24 additions & 21 deletions mooncake-store/src/common/s3_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,19 @@
#include <aws/s3/model/UploadPartRequest.h>
#include <aws/core/client/ClientConfiguration.h>
#include "crc32c.h"
#include "environ.h"
#include "fmt/format.h"
#include "common/base64.h"
#include "config/s3_client_config.h"

namespace mooncake {

namespace {

const S3ClientConfig &GetS3ClientConfig() {
static const S3ClientConfig config = S3ClientConfig::FromEnvironment();
return config;
}

// Parse a checksum-mode string ("when_supported" | "when_required") into the
// AWS SDK enum. Returns std::nullopt when the value is unset or invalid — the
// caller should then leave the AWS SDK's default in place. This is purely
Expand Down Expand Up @@ -76,10 +81,8 @@ void S3Helper::InitAPI() {
Aws::InitAPI(options_);
aws_initialized = true;

// Force Environ initialization so all MOONCAKE_AWS_* env vars are read
// once during startup, matching the previous "read once at InitAPI"
// behavior.
(void)Environ::Get();
// Cache all MOONCAKE_AWS_* settings once during startup.
(void)GetS3ClientConfig();
}

void S3Helper::ShutdownAPI() {
Expand All @@ -91,43 +94,43 @@ void S3Helper::ShutdownAPI() {

S3Helper::S3Helper(const std::string &endpoint, const std::string &bucket,
const std::string &region) {
const auto &env = Environ::Get();
const auto &env = GetS3ClientConfig();
Aws::Client::ClientConfiguration config(true);

config.connectTimeoutMs = env.GetAwsConnectTimeoutMs();
config.requestTimeoutMs = env.GetAwsRequestTimeoutMs();
config.scheme = env.GetAwsUseHttps() ? Aws::Http::Scheme::HTTPS
: Aws::Http::Scheme::HTTP;
config.connectTimeoutMs = env.connect_timeout_ms;
config.requestTimeoutMs = env.request_timeout_ms;
config.scheme =
env.use_https ? Aws::Http::Scheme::HTTPS : Aws::Http::Scheme::HTTP;
if (auto request_checksum =
ParseChecksumMode<Aws::Client::RequestChecksumCalculation>(
env.GetAwsRequestChecksumCalculation())) {
env.request_checksum_calculation)) {
config.checksumConfig.requestChecksumCalculation = *request_checksum;
}
if (auto response_checksum =
ParseChecksumMode<Aws::Client::ResponseChecksumValidation>(
env.GetAwsResponseChecksumValidation())) {
env.response_checksum_validation)) {
config.checksumConfig.responseChecksumValidation = *response_checksum;
}

if (!region.empty()) {
config.region = region;
} else {
config.region = env.GetAwsRegion();
config.region = env.region;
}

if (!endpoint.empty()) {
config.endpointOverride = endpoint;
} else {
config.endpointOverride = env.GetAwsS3Endpoint();
config.endpointOverride = env.s3_endpoint;
}

bucket_ = env.GetAwsBucketName();
bucket_ = env.bucket_name;
if (!bucket.empty()) {
bucket_ = bucket;
}

Aws::Auth::AWSCredentials credentials(env.GetAwsAccessKeyId(),
env.GetAwsSecretAccessKey());
Aws::Auth::AWSCredentials credentials(env.access_key_id,
env.secret_access_key);

// Concatenate log information into member variable connection_info_
connection_info_ = fmt::format(
Expand All @@ -145,14 +148,14 @@ S3Helper::S3Helper(const std::string &endpoint, const std::string &bucket,
bucket_.empty() ? "unset" : bucket_, config.connectTimeoutMs,
config.requestTimeoutMs,
config.scheme == Aws::Http::Scheme::HTTPS ? "HTTPS" : "HTTP",
!env.GetAwsAccessKeyId().empty() ? "set" : "unset",
!env.GetAwsSecretAccessKey().empty() ? "set" : "unset",
env.GetAwsUseVirtualAddressing() ? "true" : "false");
!env.access_key_id.empty() ? "set" : "unset",
!env.secret_access_key.empty() ? "set" : "unset",
env.use_virtual_addressing ? "true" : "false");

s3_client_ = Aws::S3::S3Client(
credentials, config,
Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
env.GetAwsUseVirtualAddressing());
env.use_virtual_addressing);
}

S3Helper::~S3Helper() = default;
Expand Down
44 changes: 44 additions & 0 deletions mooncake-store/src/config/s3_client_config.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#include "s3_client_config.h"

#include "environ.h"
#include "environment_variables.h"

namespace mooncake {

S3ClientConfig S3ClientConfig::FromEnvironment() {
S3ClientConfig config;
config.region = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_REGION, config.region);
config.s3_endpoint =
Environ::ReadOr(S3ClientEnvironmentVariables::MOONCAKE_AWS_S3_ENDPOINT,
config.s3_endpoint);
config.bucket_name =
Environ::ReadOr(S3ClientEnvironmentVariables::MOONCAKE_AWS_BUCKET_NAME,
config.bucket_name);
config.access_key_id = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_ACCESS_KEY_ID,
config.access_key_id);
config.secret_access_key = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_SECRET_ACCESS_KEY,
config.secret_access_key);
config.use_virtual_addressing = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_USE_VIRTUAL_ADDRESSING,
config.use_virtual_addressing);
config.use_https = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_USE_HTTPS, config.use_https);
config.request_checksum_calculation = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_REQUEST_CHECKSUM_CALCULATION,
config.request_checksum_calculation);
config.response_checksum_validation = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_RESPONSE_CHECKSUM_VALIDATION,
config.response_checksum_validation);
config.connect_timeout_ms = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_CONNECT_TIMEOUT_MS,
config.connect_timeout_ms);
config.request_timeout_ms = Environ::ReadOr(
S3ClientEnvironmentVariables::MOONCAKE_AWS_REQUEST_TIMEOUT_MS,
config.request_timeout_ms);
return config;
}

} // namespace mooncake
24 changes: 24 additions & 0 deletions mooncake-store/src/config/s3_client_config.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#pragma once

#include <cstdint>
#include <string>

namespace mooncake {

struct S3ClientConfig {
std::string region;
std::string s3_endpoint;
std::string bucket_name;
std::string access_key_id;
std::string secret_access_key;
bool use_virtual_addressing = true;
bool use_https = true;
std::string request_checksum_calculation;
std::string response_checksum_validation;
int64_t connect_timeout_ms = 10000;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use std::chrono::milliseconds instead of int64_t here?

int64_t request_timeout_ms = 30000;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


static S3ClientConfig FromEnvironment();
};

} // namespace mooncake
1 change: 1 addition & 0 deletions mooncake-store/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ add_store_test(
add_store_test(zstd_util_test zstd_util_test.cpp)
add_store_test(local_file_snapshot_config_test
local_file_snapshot_config_test.cpp)
add_store_test(s3_client_config_test s3_client_config_test.cpp)
add_store_test(
local_file_snapshot_object_store_test
ha/snapshot/object/backends/local/local_file_snapshot_object_store_test.cpp)
Expand Down
Loading
Loading