Skip to content

Commit

Permalink
Fix a test failure when built with ASSERT_STATUS_CHECKED=1 (facebook#…
Browse files Browse the repository at this point in the history
…8075)

Summary:
As title.
Test plan
ASSERT_STATUS_CHECKED=1 make -j20 backupable_db_test error_handler_fs_test
./backupable_db_test
./error_handler_fs_test

Pull Request resolved: facebook#8075

Reviewed By: zhichao-cao

Differential Revision: D27173832

Pulled By: riversand963

fbshipit-source-id: 37dac50f7c89127804ff2572abddd4174642de30
  • Loading branch information
riversand963 authored and facebook-github-bot committed Mar 19, 2021
1 parent 576cff1 commit 7ee41a5
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
6 changes: 3 additions & 3 deletions db/db_impl/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,14 @@ Status DBImpl::FlushMemTableToOutputFile(
// other column families are missing.
// SyncClosedLogs() may unlock and re-lock the db_mutex.
log_io_s = SyncClosedLogs(job_context);
s = log_io_s;
if (!log_io_s.ok() && !log_io_s.IsShutdownInProgress() &&
!log_io_s.IsColumnFamilyDropped()) {
error_handler_.SetBGError(log_io_s, BackgroundErrorReason::kFlush);
}
} else {
TEST_SYNC_POINT("DBImpl::SyncClosedLogs:Skip");
}
s = log_io_s;

// If the log sync failed, we do not need to pick memtable. Otherwise,
// num_flush_not_started_ needs to be rollback.
Expand Down Expand Up @@ -431,7 +431,6 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
// TODO (yanqin) investigate whether we should sync the closed logs for
// single column family case.
log_io_s = SyncClosedLogs(job_context);
s = log_io_s;
if (!log_io_s.ok() && !log_io_s.IsShutdownInProgress() &&
!log_io_s.IsColumnFamilyDropped()) {
if (total_log_size_ > 0) {
Expand All @@ -442,6 +441,7 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
}
}
}
s = log_io_s;

// exec_status stores the execution status of flush_jobs as
// <bool /* executed */, Status /* status code */>
Expand Down Expand Up @@ -541,7 +541,7 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
}
}
for (int i = 0; i != num_cfs; ++i) {
if (exec_status[i].first && exec_status[i].second.ok()) {
if (exec_status[i].second.ok() && exec_status[i].first) {
auto& mems = jobs[i]->GetMemTables();
cfds[i]->imm()->RollbackMemtableFlush(mems,
file_meta[i].fd.GetNumber());
Expand Down
4 changes: 2 additions & 2 deletions utilities/backupable/backupable_db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ TEST_F(BackupableDBTest, FileCollision) {

// If the db directory has been cleaned up, it is sensitive to file
// collision.
DestroyDB(dbname_, options_);
ASSERT_OK(DestroyDB(dbname_, options_));

// open with old backup
OpenDBAndBackupEngine(false /* destroy_old_data */, false /* dummy */,
Expand All @@ -995,7 +995,7 @@ TEST_F(BackupableDBTest, FileCollision) {
CloseDBAndBackupEngine();

// delete old data
DestroyDB(dbname_, options_);
ASSERT_OK(DestroyDB(dbname_, options_));
}
}

Expand Down

0 comments on commit 7ee41a5

Please sign in to comment.