Skip to content

Commit 0242553

Browse files
committed
Changes to support Windows builds
Replaces direct use of /tmp, to fix Windows issues - Add ScratchDir/File methods to CloudEnvImpl - Fix the ObjectPath to always be set to unix-style directory - Use RocksDB DestroyDir rather than custom implementation - Change the CopyToFromS3 test to append smaller blocks. Windows barfs when appending large blocks to files (RocksDB, not Cloud issue?)
1 parent 55842b1 commit 0242553

File tree

10 files changed

+129
-56
lines changed

10 files changed

+129
-56
lines changed

cloud/aws/aws_env.cc

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ AwsAccessType AwsCloudAccessCredentials::GetAccessType() const {
5050
return AwsAccessType::kConfig;
5151
} else if (!access_key_id.empty() || !secret_key.empty()) {
5252
return AwsAccessType::kSimple;
53+
} else if (getenv("AWS_ACCESS_KEY_ID") != nullptr &&
54+
getenv("AWS_SECRET_ACCESS_KEY") != nullptr) {
55+
return AwsAccessType::kEnvironment;
5356
}
5457
return AwsAccessType::kUndefined;
5558
}
@@ -161,6 +164,7 @@ Status AwsCloudAccessCredentials::GetCredentialsProvider(
161164
}
162165

163166
#ifdef USE_AWS
167+
static Aws::SDKOptions sdkOptions;
164168

165169
//
166170
// The AWS credentials are specified to the constructor via
@@ -169,7 +173,8 @@ Status AwsCloudAccessCredentials::GetCredentialsProvider(
169173
AwsEnv::AwsEnv(Env* underlying_env, const CloudEnvOptions& _cloud_env_options,
170174
const std::shared_ptr<Logger>& info_log)
171175
: CloudEnvImpl(_cloud_env_options, underlying_env, info_log) {
172-
Aws::InitAPI(Aws::SDKOptions());
176+
Aws::InitAPI(sdkOptions); //**TODO: Move this into PrepareOptions and do it
177+
// conditionally (first time)
173178
if (cloud_env_options.src_bucket.GetRegion().empty() ||
174179
cloud_env_options.dest_bucket.GetRegion().empty()) {
175180
std::string region;
@@ -187,7 +192,11 @@ AwsEnv::AwsEnv(Env* underlying_env, const CloudEnvOptions& _cloud_env_options,
187192
base_env_ = underlying_env;
188193
}
189194

190-
void AwsEnv::Shutdown() { Aws::ShutdownAPI(Aws::SDKOptions()); }
195+
AwsEnv::~AwsEnv() {
196+
//**TODO: Conditionally call shutdown (or make shutdown conditional on last...
197+
}
198+
199+
void AwsEnv::Shutdown() { Aws::ShutdownAPI(sdkOptions); }
191200

192201

193202
// The factory method for creating an S3 Env

cloud/aws/aws_env.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class AwsEnv : public CloudEnvImpl {
4949
const std::shared_ptr<Logger>& info_log,
5050
CloudEnv** cenv);
5151

52-
virtual ~AwsEnv() {}
52+
virtual ~AwsEnv();
5353

5454
const char* Name() const override { return "aws"; }
5555

cloud/aws/aws_retry.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ Status AwsCloudOptions::GetClientConfiguration(
125125
// Setup how retries need to be done
126126
config->retryStrategy = std::make_shared<AwsRetryStrategy>(env);
127127
if (cloud_env_options.request_timeout_ms != 0) {
128-
config->requestTimeoutMs = cloud_env_options.request_timeout_ms;
128+
config->requestTimeoutMs = static_cast<long>(cloud_env_options.request_timeout_ms);
129129
}
130130

131131
config->region = ToAwsString(region);

cloud/cloud_env.cc

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2017 Rockset.
22
#ifndef ROCKSDB_LITE
33

4-
#ifndef _WIN32_WINNT
4+
#ifndef _WIN32
55
#include <unistd.h>
66
#else
77
#include <windows.h>
@@ -12,6 +12,7 @@
1212
#include "cloud/cloud_env_wrapper.h"
1313
#include "cloud/db_cloud_impl.h"
1414
#include "cloud/filename.h"
15+
#include "file/filename.h"
1516
#include "port/likely.h"
1617
#include "rocksdb/cloud/cloud_log_controller.h"
1718
#include "rocksdb/db.h"
@@ -59,6 +60,23 @@ void BucketOptions::SetBucketName(const std::string& bucket,
5960
}
6061
}
6162

63+
void BucketOptions::SetObjectPath(const std::string& object) {
64+
// Remove the drive if there is one...
65+
auto colon = object.find(':');
66+
if (colon != std::string::npos) {
67+
object_ = object.substr(colon + 1);
68+
} else {
69+
object_ = object;
70+
}
71+
// Replace any "\" with "/"
72+
for (auto pos = object_.find('\\'); pos != std::string::npos;
73+
pos = object_.find('\\', pos)) {
74+
object_[pos] = '/';
75+
}
76+
// Remove any duplicate markers
77+
object_ = NormalizePath(object_);
78+
}
79+
6280
// Initializes the bucket properties
6381

6482
void BucketOptions::TEST_Initialize(const std::string& bucket,
@@ -70,15 +88,13 @@ void BucketOptions::TEST_Initialize(const std::string& bucket,
7088
if (!CloudEnvOptions::GetNameFromEnvironment("ROCKSDB_CLOUD_TEST_BUCKET_NAME",
7189
"ROCKSDB_CLOUD_BUCKET_NAME",
7290
&bucket_)) {
73-
#ifdef _WIN32_WINNT
91+
#ifdef _WIN32
7492
char user_name[257]; // UNLEN + 1
7593
DWORD dwsize = sizeof(user_name);
7694
if (!::GetUserName(user_name, &dwsize)) {
7795
bucket_ = bucket_ + "unknown";
7896
} else {
79-
bucket_ =
80-
bucket_ +
81-
std::string(user_name, static_cast<std::string::size_type>(dwsize));
97+
bucket_ = bucket_ + user_name;
8298
}
8399
#else
84100
bucket_ = bucket + std::to_string(geteuid());
@@ -90,10 +106,13 @@ void BucketOptions::TEST_Initialize(const std::string& bucket,
90106
prefix_ = prefix;
91107
}
92108
name_ = prefix_ + bucket_;
93-
if (!CloudEnvOptions::GetNameFromEnvironment("ROCKSDB_CLOUD_TEST_OBECT_PATH",
94-
"ROCKSDB_CLOUD_OBJECT_PATH",
95-
&object_)) {
96-
object_ = object;
109+
std::string value;
110+
if (CloudEnvOptions::GetNameFromEnvironment("ROCKSDB_CLOUD_TEST_OBECT_PATH",
111+
"ROCKSDB_CLOUD_OBJECT_PATH",
112+
&value)) {
113+
SetObjectPath(value);
114+
} else {
115+
SetObjectPath(object);
97116
}
98117
if (!CloudEnvOptions::GetNameFromEnvironment(
99118
"ROCKSDB_CLOUD_TEST_REGION", "ROCKSDB_CLOUD_REGION", &region_)) {

cloud/cloud_env_impl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ std::string CloudEnvImpl::RemapFilename(const std::string& logical_path) const {
848848
return logical_path;
849849
}
850850
auto file_name = basename(logical_path);
851-
uint64_t fileNumber;
851+
uint64_t fileNumber = 0;
852852
FileType type;
853853
WalFileType walType;
854854
if (file_name == "MANIFEST") {

cloud/cloud_env_impl.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111
#include "rocksdb/env.h"
1212
#include "rocksdb/status.h"
1313

14+
#ifdef _WIN32
15+
// Windows API macro interference
16+
#undef DeleteFile
17+
#undef GetCurrentTime
18+
#undef GetFreeSpace
19+
#endif
20+
1421
namespace ROCKSDB_NAMESPACE {
1522
class CloudScheduler;
1623
class CloudStorageReadableFile;
@@ -336,7 +343,9 @@ class CloudEnvImpl : public CloudEnv {
336343
bool test_disable_cloud_manifest_{false};
337344

338345
// scratch space in local dir
339-
static constexpr const char* SCRATCH_LOCAL_DIR = "/tmp";
346+
std::string GetScratchDirectory() const;
347+
std::string GetScratchFile() const;
348+
340349
std::mutex files_to_delete_mutex_;
341350
std::chrono::seconds file_deletion_delay_ = std::chrono::hours(1);
342351
std::unordered_map<std::string, int> files_to_delete_;

cloud/cloud_scheduler_test.cc

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,22 @@
1313
#include <thread>
1414
#include <unordered_set>
1515

16+
#include "rocksdb/env.h"
1617
#include "test_util/testharness.h"
1718

1819
namespace ROCKSDB_NAMESPACE {
1920

2021
class CloudSchedulerTest : public testing::Test {
2122
public:
22-
CloudSchedulerTest() { scheduler_ = CloudScheduler::Get(); }
23+
CloudSchedulerTest() {
24+
scheduler_ = CloudScheduler::Get();
25+
env_ = Env::Default();
26+
}
2327
~CloudSchedulerTest() {}
2428

2529
std::shared_ptr<CloudScheduler> scheduler_;
30+
Env *env_;
31+
2632
void WaitForJobs(const std::vector<long> &jobs, uint32_t delay) {
2733
bool running = true;
2834
while (running) {
@@ -34,7 +40,7 @@ class CloudSchedulerTest : public testing::Test {
3440
}
3541
}
3642
if (running) {
37-
usleep(delay);
43+
env_->SleepForMicroseconds(delay);
3844
}
3945
}
4046
}
@@ -89,14 +95,14 @@ TEST_F(CloudSchedulerTest, TestRecurring) {
8995
std::chrono::microseconds(100), doJob2,
9096
nullptr);
9197
while (job2 <= 4) {
92-
usleep(100);
98+
env_->SleepForMicroseconds(100);
9399
}
94100
ASSERT_GE(job2.load(), 4);
95101
ASSERT_GT(job1.load(), job2);
96102
ASSERT_TRUE(scheduler_->CancelJob(handle1));
97103
auto old1 = job1.load();
98104
auto old2 = job2.load();
99-
usleep(200);
105+
env_->SleepForMicroseconds(200);
100106
ASSERT_EQ(job1.load(), old1);
101107
ASSERT_GT(job2.load(), old2);
102108
}
@@ -117,7 +123,7 @@ TEST_F(CloudSchedulerTest, TestMultipleSchedulers) {
117123
ASSERT_FALSE(scheduler2->CancelJob(handle1));
118124
ASSERT_TRUE(scheduler2->CancelJob(handle2));
119125
ASSERT_FALSE(scheduler2->CancelJob(handle2));
120-
usleep(200);
126+
env_->SleepForMicroseconds(200);
121127
ASSERT_EQ(job1, 2);
122128
ASSERT_EQ(job2, 1);
123129

@@ -130,7 +136,7 @@ TEST_F(CloudSchedulerTest, TestMultipleSchedulers) {
130136
scheduler2.reset();
131137
auto old1 = job1.load();
132138
auto old2 = job2.load();
133-
usleep(200);
139+
env_->SleepForMicroseconds(200);
134140
ASSERT_EQ(job2, old2);
135141
ASSERT_GT(job1, old1);
136142
}

0 commit comments

Comments
 (0)