Skip to content

Commit 059e584

Browse files
committed
[unit test] CompactRange should fail if we don't have space
Summary: See t5106397. Also, few more changes: 1. in unit tests, the assumption is that writes will be dropped when there is no space left on device. I changed the wording around it. 2. InvalidArgument() errors are only when user-provided arguments are invalid. When the file is corrupted, we need to return Status::Corruption Test Plan: make check Reviewers: sdong, ljin Reviewed By: ljin Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D23145
1 parent dd641b2 commit 059e584

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

db/db_test.cc

+37-7
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ class SpecialEnv : public EnvWrapper {
123123
// sstable Sync() calls are blocked while this pointer is non-nullptr.
124124
port::AtomicPointer delay_sstable_sync_;
125125

126+
// Drop writes on the floor while this pointer is non-nullptr.
127+
port::AtomicPointer drop_writes_;
128+
126129
// Simulate no-space errors while this pointer is non-nullptr.
127130
port::AtomicPointer no_space_;
128131

@@ -150,6 +153,7 @@ class SpecialEnv : public EnvWrapper {
150153

151154
explicit SpecialEnv(Env* base) : EnvWrapper(base) {
152155
delay_sstable_sync_.Release_Store(nullptr);
156+
drop_writes_.Release_Store(nullptr);
153157
no_space_.Release_Store(nullptr);
154158
non_writable_.Release_Store(nullptr);
155159
count_random_reads_ = false;
@@ -173,9 +177,11 @@ class SpecialEnv : public EnvWrapper {
173177
base_(std::move(base)) {
174178
}
175179
Status Append(const Slice& data) {
176-
if (env_->no_space_.Acquire_Load() != nullptr) {
180+
if (env_->drop_writes_.Acquire_Load() != nullptr) {
177181
// Drop writes on the floor
178182
return Status::OK();
183+
} else if (env_->no_space_.Acquire_Load() != nullptr) {
184+
return Status::IOError("No space left on device");
179185
} else {
180186
env_->bytes_written_ += data.size();
181187
return base_->Append(data);
@@ -5573,8 +5579,8 @@ TEST(DBTest, DestroyDBMetaDatabase) {
55735579
ASSERT_TRUE(!(DB::Open(opts, metametadbname, &db)).ok());
55745580
}
55755581

5576-
// Check that number of files does not grow when we are out of space
5577-
TEST(DBTest, NoSpace) {
5582+
// Check that number of files does not grow when writes are dropped
5583+
TEST(DBTest, DropWrites) {
55785584
do {
55795585
Options options = CurrentOptions();
55805586
options.env = env_;
@@ -5585,7 +5591,7 @@ TEST(DBTest, NoSpace) {
55855591
ASSERT_EQ("v1", Get("foo"));
55865592
Compact("a", "z");
55875593
const int num_files = CountFiles();
5588-
env_->no_space_.Release_Store(env_); // Force out-of-space errors
5594+
env_->drop_writes_.Release_Store(env_); // Force out-of-space errors
55895595
env_->sleep_counter_.Reset();
55905596
for (int i = 0; i < 5; i++) {
55915597
for (int level = 0; level < dbfull()->NumberLevels()-1; level++) {
@@ -5597,7 +5603,7 @@ TEST(DBTest, NoSpace) {
55975603
ASSERT_TRUE(db_->GetProperty("rocksdb.background-errors", &property_value));
55985604
ASSERT_EQ("5", property_value);
55995605

5600-
env_->no_space_.Release_Store(nullptr);
5606+
env_->drop_writes_.Release_Store(nullptr);
56015607
ASSERT_LT(CountFiles(), num_files + 3);
56025608

56035609
// Check that compaction attempts slept after errors
@@ -5606,15 +5612,15 @@ TEST(DBTest, NoSpace) {
56065612
}
56075613

56085614
// Check background error counter bumped on flush failures.
5609-
TEST(DBTest, NoSpaceFlush) {
5615+
TEST(DBTest, DropWritesFlush) {
56105616
do {
56115617
Options options = CurrentOptions();
56125618
options.env = env_;
56135619
options.max_background_flushes = 1;
56145620
Reopen(&options);
56155621

56165622
ASSERT_OK(Put("foo", "v1"));
5617-
env_->no_space_.Release_Store(env_); // Force out-of-space errors
5623+
env_->drop_writes_.Release_Store(env_); // Force out-of-space errors
56185624

56195625
std::string property_value;
56205626
// Background error count is 0 now.
@@ -5638,6 +5644,30 @@ TEST(DBTest, NoSpaceFlush) {
56385644
}
56395645
ASSERT_EQ("1", property_value);
56405646

5647+
env_->drop_writes_.Release_Store(nullptr);
5648+
} while (ChangeCompactOptions());
5649+
}
5650+
5651+
// Check that CompactRange() returns failure if there is not enough space left
5652+
// on device
5653+
TEST(DBTest, NoSpaceCompactRange) {
5654+
do {
5655+
Options options = CurrentOptions();
5656+
options.env = env_;
5657+
options.disable_auto_compactions = true;
5658+
Reopen(&options);
5659+
5660+
// generate 5 tables
5661+
for (int i = 0; i < 5; ++i) {
5662+
ASSERT_OK(Put(Key(i), Key(i) + "v"));
5663+
ASSERT_OK(Flush());
5664+
}
5665+
5666+
env_->no_space_.Release_Store(env_); // Force out-of-space errors
5667+
5668+
Status s = db_->CompactRange(nullptr, nullptr);
5669+
ASSERT_TRUE(s.IsIOError());
5670+
56415671
env_->no_space_.Release_Store(nullptr);
56425672
} while (ChangeCompactOptions());
56435673
}

table/format.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ Status Footer::DecodeFrom(Slice* input) {
135135
snprintf(buffer, sizeof(buffer) - 1,
136136
"not an sstable (bad magic number --- %lx)",
137137
(long)magic);
138-
return Status::InvalidArgument(buffer);
138+
return Status::Corruption(buffer);
139139
}
140140
} else {
141141
set_table_magic_number(magic);
@@ -156,7 +156,7 @@ Status Footer::DecodeFrom(Slice* input) {
156156
// It consists of the checksum type, two block handles, padding,
157157
// a version number, and a magic number
158158
if (input->size() < kVersion1EncodedLength) {
159-
return Status::InvalidArgument("input is too short to be an sstable");
159+
return Status::Corruption("input is too short to be an sstable");
160160
} else {
161161
input->remove_prefix(input->size() - kVersion1EncodedLength);
162162
}
@@ -183,7 +183,7 @@ Status ReadFooterFromFile(RandomAccessFile* file,
183183
uint64_t file_size,
184184
Footer* footer) {
185185
if (file_size < Footer::kMinEncodedLength) {
186-
return Status::InvalidArgument("file is too short to be an sstable");
186+
return Status::Corruption("file is too short to be an sstable");
187187
}
188188

189189
char footer_space[Footer::kMaxEncodedLength];
@@ -198,7 +198,7 @@ Status ReadFooterFromFile(RandomAccessFile* file,
198198
// Check that we actually read the whole footer from the file. It may be
199199
// that size isn't correct.
200200
if (footer_input.size() < Footer::kMinEncodedLength) {
201-
return Status::InvalidArgument("file is too short to be an sstable");
201+
return Status::Corruption("file is too short to be an sstable");
202202
}
203203

204204
return footer->DecodeFrom(&footer_input);

0 commit comments

Comments
 (0)