From 72202962f9c388436edcad8602232a3c81974d0b Mon Sep 17 00:00:00 2001 From: Leonidas Galanis Date: Tue, 7 Mar 2017 11:06:53 -0800 Subject: [PATCH] fix db_sst_test flakiness Summary: db_sst_test had been flaky occasionally in the following way: reached_max_space_on_compaction can in very rare cases be 0. This happens when the limit on maximum allowable space set using SetMaxAllowedSpaceUsage is hit during flush for all test db sizes (1,2,4,8 and 10MB).The fix clears the error returned when the the space limit is reached during flush. This ensures that the compaction call back will always be called. The runtime is increased slightly because the 1MB loop writes more data and hits the limit during multiple flushes until compaction is scheduled. Closes https://github.com/facebook/rocksdb/pull/1861 Differential Revision: D4557396 Pulled By: lgalanis fbshipit-source-id: ff778d1 --- db/db_impl.cc | 5 +++-- db/db_sst_test.cc | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 6a203fa2c..9e8582fa8 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1971,8 +1971,9 @@ Status DBImpl::FlushMemTableToOutputFile( sfm->OnAddFile(file_path); if (sfm->IsMaxAllowedSpaceReached() && bg_error_.ok()) { bg_error_ = Status::IOError("Max allowed space was reached"); - TEST_SYNC_POINT( - "DBImpl::FlushMemTableToOutputFile:MaxAllowedSpaceReached"); + TEST_SYNC_POINT_CALLBACK( + "DBImpl::FlushMemTableToOutputFile:MaxAllowedSpaceReached", + &bg_error_); } } #endif // ROCKSDB_LITE diff --git a/db/db_sst_test.cc b/db/db_sst_test.cc index fb0c39077..903fca762 100644 --- a/db/db_sst_test.cc +++ b/db/db_sst_test.cc @@ -516,18 +516,25 @@ TEST_F(DBSSTTest, DBWithMaxSpaceAllowedRandomized) { // than the limit. std::vector max_space_limits_mbs = {1, 2, 4, 8, 10}; - + decltype(max_space_limits_mbs)::value_type limit_mb_cb; bool bg_error_set = false; uint64_t total_sst_files_size = 0; + std::atomic estimate_multiplier(1); int reached_max_space_on_flush = 0; int reached_max_space_on_compaction = 0; rocksdb::SyncPoint::GetInstance()->SetCallBack( "DBImpl::FlushMemTableToOutputFile:MaxAllowedSpaceReached", [&](void* arg) { + Status* bg_error = static_cast(arg); bg_error_set = true; GetAllSSTFiles(&total_sst_files_size); reached_max_space_on_flush++; + // low limit for size calculated using sst files + ASSERT_GE(total_sst_files_size, limit_mb_cb * 1024 * 1024); + // clear error to ensure compaction callback is called + *bg_error = Status::OK(); + estimate_multiplier++; // used in the main loop assert }); rocksdb::SyncPoint::GetInstance()->SetCallBack( @@ -541,6 +548,8 @@ TEST_F(DBSSTTest, DBWithMaxSpaceAllowedRandomized) { for (auto limit_mb : max_space_limits_mbs) { bg_error_set = false; total_sst_files_size = 0; + estimate_multiplier = 1; + limit_mb_cb = limit_mb; rocksdb::SyncPoint::GetInstance()->ClearTrace(); rocksdb::SyncPoint::GetInstance()->EnableProcessing(); std::shared_ptr sst_file_manager(NewSstFileManager(env_)); @@ -565,7 +574,8 @@ TEST_F(DBSSTTest, DBWithMaxSpaceAllowedRandomized) { // Check the estimated db size vs the db limit just to make sure we // dont run into an infinite loop estimated_db_size = keys_written * 60; // ~60 bytes per key - ASSERT_LT(estimated_db_size, limit_mb * 1024 * 1024 * 2); + ASSERT_LT(estimated_db_size, + estimate_multiplier * limit_mb * 1024 * 1024 * 2); } ASSERT_TRUE(bg_error_set); ASSERT_GE(total_sst_files_size, limit_mb * 1024 * 1024);