Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

32 general bug fixes #33

Merged
merged 7 commits into from
Jul 18, 2022
Merged

32 general bug fixes #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
49bc525
SPDB-180: general: avoid comparing Status using == as it only compare…
isaac-io Jul 8, 2021
516aa3b
SPDB-180: tests: avoid comparing Status using == as it only compares …
isaac-io Jul 13, 2022
09774cb
SPDB-156: db_test: add test for - forward the incomplete status on no_io
Yuval-Ariel May 17, 2022
f996fd4
SPDB-341: optimistic_transaction_example: fix snapshots leak
isaac-io Nov 16, 2021
ea4c187
SPDB-96: ldb: fix get to print the entire value
isaac-io Apr 27, 2021
97e925a
SPDB-534: db_bench: fix rocksdb bug of last_ref assertion
Yuval-Ariel Feb 23, 2022
4e5dae3
SPDB 521: db_bench: fix SeekRandom and ReadRandomWriteRandom
Yuval-Ariel Jun 20, 2022
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
2 changes: 1 addition & 1 deletion cache/cache_reservation_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Status CacheReservationManagerImpl<R>::IncreaseCacheReservation(
return_status = cache_->Insert(GetNextCacheKey(), nullptr, kSizeDummyEntry,
GetNoopDeleterForRole<R>(), &handle);

if (return_status != Status::OK()) {
if (!return_status.ok()) {
return return_status;
}

Expand Down
44 changes: 20 additions & 24 deletions cache/cache_reservation_manager_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class CacheReservationManagerTest : public ::testing::Test {
TEST_F(CacheReservationManagerTest, GenerateCacheKey) {
std::size_t new_mem_used = 1 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_GE(cache->GetPinnedUsage(), 1 * kSizeDummyEntry);
ASSERT_LT(cache->GetPinnedUsage(),
1 * kSizeDummyEntry + kMetaDataChargeOverhead);
Expand All @@ -66,7 +66,7 @@ TEST_F(CacheReservationManagerTest, GenerateCacheKey) {
TEST_F(CacheReservationManagerTest, KeepCacheReservationTheSame) {
std::size_t new_mem_used = 1 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
1 * kSizeDummyEntry);
ASSERT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), new_mem_used);
Expand All @@ -76,7 +76,7 @@ TEST_F(CacheReservationManagerTest, KeepCacheReservationTheSame) {
1 * kSizeDummyEntry + kMetaDataChargeOverhead);

s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
EXPECT_OK(s)
<< "Failed to keep cache reservation the same when new_mem_used equals "
"to current cache reservation";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand All @@ -95,8 +95,7 @@ TEST_F(CacheReservationManagerTest,
IncreaseCacheReservationByMultiplesOfDummyEntrySize) {
std::size_t new_mem_used = 2 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
<< "Failed to increase cache reservation correctly";
EXPECT_OK(s) << "Failed to increase cache reservation correctly";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
2 * kSizeDummyEntry)
<< "Failed to bookkeep cache reservation increase correctly";
Expand All @@ -113,8 +112,7 @@ TEST_F(CacheReservationManagerTest,
IncreaseCacheReservationNotByMultiplesOfDummyEntrySize) {
std::size_t new_mem_used = 2 * kSizeDummyEntry + kSizeDummyEntry / 2;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
<< "Failed to increase cache reservation correctly";
EXPECT_OK(s) << "Failed to increase cache reservation correctly";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
3 * kSizeDummyEntry)
<< "Failed to bookkeep cache reservation increase correctly";
Expand Down Expand Up @@ -147,7 +145,7 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,

std::size_t new_mem_used = kSmallCacheCapacity + 1;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::Incomplete())
EXPECT_TRUE(s.IsIncomplete())
<< "Failed to return status to indicate failure of dummy entry insertion "
"during cache reservation on full cache";
EXPECT_GE(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand All @@ -170,7 +168,7 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,

new_mem_used = kSmallCacheCapacity / 2; // 2 dummy entries
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
EXPECT_OK(s)
<< "Failed to decrease cache reservation after encountering cache "
"reservation failure due to full cache";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand All @@ -192,7 +190,7 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
// Create cache full again for subsequent tests
new_mem_used = kSmallCacheCapacity + 1;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::Incomplete())
EXPECT_TRUE(s.IsIncomplete())
<< "Failed to return status to indicate failure of dummy entry insertion "
"during cache reservation on full cache";
EXPECT_GE(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand All @@ -218,7 +216,7 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
cache->SetCapacity(kBigCacheCapacity);
new_mem_used = kSmallCacheCapacity + 1;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
EXPECT_OK(s)
<< "Failed to increase cache reservation after increasing cache capacity "
"and mitigating cache full error";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand All @@ -240,7 +238,7 @@ TEST_F(CacheReservationManagerTest,
DecreaseCacheReservationByMultiplesOfDummyEntrySize) {
std::size_t new_mem_used = 2 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
2 * kSizeDummyEntry);
ASSERT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), new_mem_used);
Expand All @@ -250,8 +248,7 @@ TEST_F(CacheReservationManagerTest,

new_mem_used = 1 * kSizeDummyEntry;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
<< "Failed to decrease cache reservation correctly";
EXPECT_OK(s) << "Failed to decrease cache reservation correctly";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
1 * kSizeDummyEntry)
<< "Failed to bookkeep cache reservation decrease correctly";
Expand All @@ -268,7 +265,7 @@ TEST_F(CacheReservationManagerTest,
DecreaseCacheReservationNotByMultiplesOfDummyEntrySize) {
std::size_t new_mem_used = 2 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
2 * kSizeDummyEntry);
ASSERT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), new_mem_used);
Expand All @@ -278,8 +275,7 @@ TEST_F(CacheReservationManagerTest,

new_mem_used = kSizeDummyEntry / 2;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
<< "Failed to decrease cache reservation correctly";
EXPECT_OK(s) << "Failed to decrease cache reservation correctly";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
1 * kSizeDummyEntry)
<< "Failed to bookkeep cache reservation decrease correctly";
Expand Down Expand Up @@ -309,7 +305,7 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,

std::size_t new_mem_used = 8 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
8 * kSizeDummyEntry);
ASSERT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), new_mem_used);
Expand All @@ -320,7 +316,7 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,

new_mem_used = 6 * kSizeDummyEntry;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK()) << "Failed to delay decreasing cache reservation";
EXPECT_OK(s) << "Failed to delay decreasing cache reservation";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
8 * kSizeDummyEntry)
<< "Failed to bookkeep correctly when delaying cache reservation "
Expand All @@ -332,7 +328,7 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,

new_mem_used = 7 * kSizeDummyEntry;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK()) << "Failed to delay decreasing cache reservation";
EXPECT_OK(s) << "Failed to delay decreasing cache reservation";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
8 * kSizeDummyEntry)
<< "Failed to bookkeep correctly when delaying cache reservation "
Expand All @@ -344,7 +340,7 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,

new_mem_used = 6 * kSizeDummyEntry - 1;
s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
EXPECT_EQ(s, Status::OK())
EXPECT_OK(s)
<< "Failed to decrease cache reservation correctly when new_mem_used < "
"GetTotalReservedCacheSize() * 3 / 4 on delayed decrease mode";
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
Expand Down Expand Up @@ -381,7 +377,7 @@ TEST(CacheReservationManagerDestructorTest,
cache);
std::size_t new_mem_used = 1 * kSizeDummyEntry;
Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
ASSERT_GE(cache->GetPinnedUsage(), 1 * kSizeDummyEntry);
ASSERT_LT(cache->GetPinnedUsage(),
1 * kSizeDummyEntry + kMetaDataChargeOverhead);
Expand Down Expand Up @@ -417,7 +413,7 @@ TEST(CacheReservationHandleTest, HandleTest) {
Status s = test_cache_rev_mng->MakeCacheReservation(
incremental_mem_used_handle_1, &handle_1);
mem_used = mem_used + incremental_mem_used_handle_1;
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
EXPECT_TRUE(handle_1 != nullptr);
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(), mem_used);
EXPECT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), mem_used);
Expand All @@ -427,7 +423,7 @@ TEST(CacheReservationHandleTest, HandleTest) {
s = test_cache_rev_mng->MakeCacheReservation(incremental_mem_used_handle_2,
&handle_2);
mem_used = mem_used + incremental_mem_used_handle_2;
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
EXPECT_TRUE(handle_2 != nullptr);
EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(), mem_used);
EXPECT_EQ(test_cache_rev_mng->GetTotalMemoryUsed(), mem_used);
Expand Down
3 changes: 2 additions & 1 deletion db/blob/db_blob_index_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,8 @@ TEST_F(DBBlobIndexTest, IntegratedBlobIterate) {

auto check_iterator = [&](Iterator* iterator, Status expected_status,
const Slice& expected_value) {
ASSERT_EQ(expected_status, iterator->status());
ASSERT_EQ(expected_status.code(), iterator->status().code());
ASSERT_EQ(expected_status.subcode(), iterator->status().subcode());
if (expected_status.ok()) {
ASSERT_TRUE(iterator->Valid());
ASSERT_EQ(expected_value, iterator->value());
Expand Down
2 changes: 1 addition & 1 deletion db/compact_files_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ TEST_F(CompactFilesTest, ObsoleteFiles) {

// verify all compaction input files are deleted
for (auto fname : l0_files) {
ASSERT_EQ(Status::NotFound(), env_->FileExists(fname));
ASSERT_TRUE(env_->FileExists(fname).IsNotFound());
}
delete db;
}
Expand Down
43 changes: 23 additions & 20 deletions db/db_basic_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ TEST_F(DBBasicTest, DBClose) {

s = db->Close();
ASSERT_EQ(env->GetCloseCount(), 1);
ASSERT_EQ(s, Status::IOError());
ASSERT_TRUE(s.IsIOError());

delete db;
ASSERT_EQ(env->GetCloseCount(), 1);
Expand All @@ -1145,7 +1145,7 @@ TEST_F(DBBasicTest, DBClose) {
ASSERT_TRUE(db != nullptr);

s = db->Close();
ASSERT_EQ(s, Status::OK());
ASSERT_OK(s);
delete db;
ASSERT_EQ(env->GetCloseCount(), 2);
options.info_log.reset();
Expand All @@ -1168,15 +1168,15 @@ TEST_F(DBBasicTest, DBCloseFlushError) {
ASSERT_OK(Put("key3", "value3"));
fault_injection_env->SetFilesystemActive(false);
Status s = dbfull()->Close();
ASSERT_NE(s, Status::OK());
ASSERT_NOK(s);
// retry should return the same error
s = dbfull()->Close();
ASSERT_NE(s, Status::OK());
ASSERT_NOK(s);
fault_injection_env->SetFilesystemActive(true);
// retry close() is no-op even the system is back. Could be improved if
// Close() is retry-able: #9029
s = dbfull()->Close();
ASSERT_NE(s, Status::OK());
ASSERT_NOK(s);
Destroy(options);
}

Expand Down Expand Up @@ -2247,7 +2247,7 @@ TEST_F(DBBasicTest, MultiGetIOBufferOverrun) {
// Make the value compressible. A purely random string doesn't compress
// and the resultant data block will not be compressed
std::string value(rnd.RandomString(128) + zero_str);
assert(Put(Key(i), value) == Status::OK());
ASSERT_OK(Put(Key(i), value));
}
ASSERT_OK(Flush());

Expand Down Expand Up @@ -2789,8 +2789,11 @@ class DBBasicTestMultiGet : public DBTestBase {
// Make the value compressible. A purely random string doesn't compress
// and the resultant data block will not be compressed
values_.emplace_back(rnd.RandomString(128) + zero_str);
assert(((num_cfs == 1) ? Put(Key(i), values_[i])
: Put(cf, Key(i), values_[i])) == Status::OK());
if (num_cfs == 1) {
assert(Put(Key(i), values_[i]).ok());
} else {
assert(Put(cf, Key(i), values_[i]).ok());
}
}
if (num_cfs == 1) {
EXPECT_OK(Flush());
Expand All @@ -2802,9 +2805,11 @@ class DBBasicTestMultiGet : public DBTestBase {
// block cannot gain space by compression
uncompressable_values_.emplace_back(rnd.RandomString(256) + '\0');
std::string tmp_key = "a" + Key(i);
assert(((num_cfs == 1) ? Put(tmp_key, uncompressable_values_[i])
: Put(cf, tmp_key, uncompressable_values_[i])) ==
Status::OK());
if (num_cfs == 1) {
assert(Put(tmp_key, uncompressable_values_[i]).ok());
} else {
assert(Put(cf, tmp_key, uncompressable_values_[i]).ok());
}
}
if (num_cfs == 1) {
EXPECT_OK(Flush());
Expand Down Expand Up @@ -3229,8 +3234,8 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithChecksumMismatch) {
keys.data(), values.data(), statuses.data(), true);
ASSERT_TRUE(CheckValue(0, values[0].ToString()));
// ASSERT_TRUE(CheckValue(50, values[1].ToString()));
ASSERT_EQ(statuses[0], Status::OK());
ASSERT_EQ(statuses[1], Status::Corruption());
ASSERT_OK(statuses[0]);
ASSERT_TRUE(statuses[1].IsCorruption());

SyncPoint::GetInstance()->DisableProcessing();
}
Expand Down Expand Up @@ -3275,8 +3280,8 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithMissingFile) {

dbfull()->MultiGet(ro, dbfull()->DefaultColumnFamily(), keys.size(),
keys.data(), values.data(), statuses.data(), true);
ASSERT_EQ(statuses[0], Status::IOError());
ASSERT_EQ(statuses[1], Status::IOError());
ASSERT_TRUE(statuses[0].IsIOError());
ASSERT_TRUE(statuses[1].IsIOError());

SyncPoint::GetInstance()->DisableProcessing();
}
Expand Down Expand Up @@ -3484,9 +3489,7 @@ class DBBasicTestMultiGetDeadline : public DBBasicTestMultiGet {
if (i < num_ok) {
EXPECT_OK(statuses[i]);
} else {
if (statuses[i] != Status::TimedOut()) {
EXPECT_EQ(statuses[i], Status::TimedOut());
}
EXPECT_TRUE(statuses[i].IsTimedOut());
}
}
}
Expand Down Expand Up @@ -3811,7 +3814,7 @@ TEST_P(DBBasicTestDeadline, PointLookupDeadline) {
std::string value;
Status s = dbfull()->Get(ro, "k50", &value);
if (fs->TimedOut()) {
ASSERT_EQ(s, Status::TimedOut());
ASSERT_TRUE(s.IsTimedOut());
} else {
timedout = false;
ASSERT_OK(s);
Expand Down Expand Up @@ -3898,7 +3901,7 @@ TEST_P(DBBasicTestDeadline, IteratorDeadline) {
}
if (fs->TimedOut()) {
ASSERT_FALSE(iter->Valid());
ASSERT_EQ(iter->status(), Status::TimedOut());
ASSERT_TRUE(iter->status().IsTimedOut());
} else {
timedout = false;
ASSERT_OK(iter->status());
Expand Down
Loading