Skip to content

Commit 6924869

Browse files
mrambacherfacebook-github-bot
authored andcommitted
Make SystemClock into a Customizable Class (facebook#8636)
Summary: Made SystemClock into a Customizable class, complete with CreateFromString. Cleaned up some of the existing SystemClock implementations that were redundant (NoSleep was the same as the internal one for MockEnv). Changed MockEnv construction to allow Clock to be passed to the Memory/MockFileSystem. Pull Request resolved: facebook#8636 Reviewed By: zhichao-cao Differential Revision: D30483360 Pulled By: mrambacher fbshipit-source-id: cd0e3a876c39f8c98fe13374c06e8edbd5b9f2a1
1 parent d497cdf commit 6924869

31 files changed

+560
-236
lines changed

HISTORY.md

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
### Bug Fixes
44
### New Features
55
### Public API change
6+
* Made SystemClock extend the Customizable class and added a CreateFromString method. Implementations need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method.
67

78
## 6.25.0 (2021-09-20)
89
### Bug Fixes

db/blob/blob_file_builder_test.cc

+29-23
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ class TestFileNumberGenerator {
3939

4040
class BlobFileBuilderTest : public testing::Test {
4141
protected:
42-
BlobFileBuilderTest() : mock_env_(Env::Default()) {
43-
fs_ = mock_env_.GetFileSystem().get();
44-
clock_ = mock_env_.GetSystemClock().get();
42+
BlobFileBuilderTest() {
43+
mock_env_.reset(MockEnv::Create(Env::Default()));
44+
fs_ = mock_env_->GetFileSystem().get();
45+
clock_ = mock_env_->GetSystemClock().get();
4546
}
4647

4748
void VerifyBlobFile(uint64_t blob_file_number,
@@ -108,7 +109,7 @@ class BlobFileBuilderTest : public testing::Test {
108109
ASSERT_EQ(footer.expiration_range, ExpirationRange());
109110
}
110111

111-
MockEnv mock_env_;
112+
std::unique_ptr<Env> mock_env_;
112113
FileSystem* fs_;
113114
SystemClock* clock_;
114115
FileOptions file_options_;
@@ -123,11 +124,11 @@ TEST_F(BlobFileBuilderTest, BuildAndCheckOneFile) {
123124

124125
Options options;
125126
options.cf_paths.emplace_back(
126-
test::PerThreadDBPath(&mock_env_,
127+
test::PerThreadDBPath(mock_env_.get(),
127128
"BlobFileBuilderTest_BuildAndCheckOneFile"),
128129
0);
129130
options.enable_blob_files = true;
130-
options.env = &mock_env_;
131+
options.env = mock_env_.get();
131132

132133
ImmutableOptions immutable_options(options);
133134
MutableCFOptions mutable_cf_options(options);
@@ -206,12 +207,12 @@ TEST_F(BlobFileBuilderTest, BuildAndCheckMultipleFiles) {
206207

207208
Options options;
208209
options.cf_paths.emplace_back(
209-
test::PerThreadDBPath(&mock_env_,
210+
test::PerThreadDBPath(mock_env_.get(),
210211
"BlobFileBuilderTest_BuildAndCheckMultipleFiles"),
211212
0);
212213
options.enable_blob_files = true;
213214
options.blob_file_size = value_size;
214-
options.env = &mock_env_;
215+
options.env = mock_env_.get();
215216

216217
ImmutableOptions immutable_options(options);
217218
MutableCFOptions mutable_cf_options(options);
@@ -293,11 +294,12 @@ TEST_F(BlobFileBuilderTest, InlinedValues) {
293294

294295
Options options;
295296
options.cf_paths.emplace_back(
296-
test::PerThreadDBPath(&mock_env_, "BlobFileBuilderTest_InlinedValues"),
297+
test::PerThreadDBPath(mock_env_.get(),
298+
"BlobFileBuilderTest_InlinedValues"),
297299
0);
298300
options.enable_blob_files = true;
299301
options.min_blob_size = 1024;
300-
options.env = &mock_env_;
302+
options.env = mock_env_.get();
301303

302304
ImmutableOptions immutable_options(options);
303305
MutableCFOptions mutable_cf_options(options);
@@ -347,10 +349,11 @@ TEST_F(BlobFileBuilderTest, Compression) {
347349

348350
Options options;
349351
options.cf_paths.emplace_back(
350-
test::PerThreadDBPath(&mock_env_, "BlobFileBuilderTest_Compression"), 0);
352+
test::PerThreadDBPath(mock_env_.get(), "BlobFileBuilderTest_Compression"),
353+
0);
351354
options.enable_blob_files = true;
352355
options.blob_compression_type = kSnappyCompression;
353-
options.env = &mock_env_;
356+
options.env = mock_env_.get();
354357

355358
ImmutableOptions immutable_options(options);
356359
MutableCFOptions mutable_cf_options(options);
@@ -429,11 +432,12 @@ TEST_F(BlobFileBuilderTest, CompressionError) {
429432

430433
Options options;
431434
options.cf_paths.emplace_back(
432-
test::PerThreadDBPath(&mock_env_, "BlobFileBuilderTest_CompressionError"),
435+
test::PerThreadDBPath(mock_env_.get(),
436+
"BlobFileBuilderTest_CompressionError"),
433437
0);
434438
options.enable_blob_files = true;
435439
options.blob_compression_type = kSnappyCompression;
436-
options.env = &mock_env_;
440+
options.env = mock_env_.get();
437441
ImmutableOptions immutable_options(options);
438442
MutableCFOptions mutable_cf_options(options);
439443

@@ -506,11 +510,12 @@ TEST_F(BlobFileBuilderTest, Checksum) {
506510

507511
Options options;
508512
options.cf_paths.emplace_back(
509-
test::PerThreadDBPath(&mock_env_, "BlobFileBuilderTest_Checksum"), 0);
513+
test::PerThreadDBPath(mock_env_.get(), "BlobFileBuilderTest_Checksum"),
514+
0);
510515
options.enable_blob_files = true;
511516
options.file_checksum_gen_factory =
512517
std::make_shared<DummyFileChecksumGenFactory>();
513-
options.env = &mock_env_;
518+
options.env = mock_env_.get();
514519

515520
ImmutableOptions immutable_options(options);
516521
MutableCFOptions mutable_cf_options(options);
@@ -575,12 +580,12 @@ class BlobFileBuilderIOErrorTest
575580
: public testing::Test,
576581
public testing::WithParamInterface<std::string> {
577582
protected:
578-
BlobFileBuilderIOErrorTest()
579-
: mock_env_(Env::Default()),
580-
fs_(mock_env_.GetFileSystem().get()),
581-
sync_point_(GetParam()) {}
583+
BlobFileBuilderIOErrorTest() : sync_point_(GetParam()) {
584+
mock_env_.reset(MockEnv::Create(Env::Default()));
585+
fs_ = mock_env_->GetFileSystem().get();
586+
}
582587

583-
MockEnv mock_env_;
588+
std::unique_ptr<Env> mock_env_;
584589
FileSystem* fs_;
585590
FileOptions file_options_;
586591
std::string sync_point_;
@@ -602,11 +607,12 @@ TEST_P(BlobFileBuilderIOErrorTest, IOError) {
602607

603608
Options options;
604609
options.cf_paths.emplace_back(
605-
test::PerThreadDBPath(&mock_env_, "BlobFileBuilderIOErrorTest_IOError"),
610+
test::PerThreadDBPath(mock_env_.get(),
611+
"BlobFileBuilderIOErrorTest_IOError"),
606612
0);
607613
options.enable_blob_files = true;
608614
options.blob_file_size = value_size;
609-
options.env = &mock_env_;
615+
options.env = mock_env_.get();
610616

611617
ImmutableOptions immutable_options(options);
612618
MutableCFOptions mutable_cf_options(options);

db/blob/blob_file_cache_test.cc

+11-10
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,18 @@ void WriteBlobFile(uint32_t column_family_id,
8484

8585
class BlobFileCacheTest : public testing::Test {
8686
protected:
87-
BlobFileCacheTest() : mock_env_(Env::Default()) {}
87+
BlobFileCacheTest() { mock_env_.reset(MockEnv::Create(Env::Default())); }
8888

89-
MockEnv mock_env_;
89+
std::unique_ptr<Env> mock_env_;
9090
};
9191

9292
TEST_F(BlobFileCacheTest, GetBlobFileReader) {
9393
Options options;
94-
options.env = &mock_env_;
94+
options.env = mock_env_.get();
9595
options.statistics = CreateDBStatistics();
9696
options.cf_paths.emplace_back(
97-
test::PerThreadDBPath(&mock_env_, "BlobFileCacheTest_GetBlobFileReader"),
97+
test::PerThreadDBPath(mock_env_.get(),
98+
"BlobFileCacheTest_GetBlobFileReader"),
9899
0);
99100
options.enable_blob_files = true;
100101

@@ -135,10 +136,10 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader) {
135136

136137
TEST_F(BlobFileCacheTest, GetBlobFileReader_Race) {
137138
Options options;
138-
options.env = &mock_env_;
139+
options.env = mock_env_.get();
139140
options.statistics = CreateDBStatistics();
140141
options.cf_paths.emplace_back(
141-
test::PerThreadDBPath(&mock_env_,
142+
test::PerThreadDBPath(mock_env_.get(),
142143
"BlobFileCacheTest_GetBlobFileReader_Race"),
143144
0);
144145
options.enable_blob_files = true;
@@ -187,10 +188,10 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader_Race) {
187188

188189
TEST_F(BlobFileCacheTest, GetBlobFileReader_IOError) {
189190
Options options;
190-
options.env = &mock_env_;
191+
options.env = mock_env_.get();
191192
options.statistics = CreateDBStatistics();
192193
options.cf_paths.emplace_back(
193-
test::PerThreadDBPath(&mock_env_,
194+
test::PerThreadDBPath(mock_env_.get(),
194195
"BlobFileCacheTest_GetBlobFileReader_IOError"),
195196
0);
196197
options.enable_blob_files = true;
@@ -221,10 +222,10 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader_IOError) {
221222

222223
TEST_F(BlobFileCacheTest, GetBlobFileReader_CacheFull) {
223224
Options options;
224-
options.env = &mock_env_;
225+
options.env = mock_env_.get();
225226
options.statistics = CreateDBStatistics();
226227
options.cf_paths.emplace_back(
227-
test::PerThreadDBPath(&mock_env_,
228+
test::PerThreadDBPath(mock_env_.get(),
228229
"BlobFileCacheTest_GetBlobFileReader_CacheFull"),
229230
0);
230231
options.enable_blob_files = true;

0 commit comments

Comments
 (0)