diff --git a/db/compaction_job.cc b/db/compaction_job.cc index 1d023ca4563a3caad0ae0d112b7c75fd62d42a8e..13a43a00a392bd262db60683256d9f868495afc0 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -1067,16 +1067,18 @@ Status CompactionJob::FinishCompactionOutputFile( range_del_agg->AddToBuilder(sub_compact->builder.get(), lower_bound, upper_bound, meta, range_del_out_stats, bottommost_level_); + meta->marked_for_compaction = sub_compact->builder->NeedCompact(); } const uint64_t current_entries = sub_compact->builder->NumEntries(); - meta->marked_for_compaction = sub_compact->builder->NeedCompact(); if (s.ok()) { s = sub_compact->builder->Finish(); } else { sub_compact->builder->Abandon(); } const uint64_t current_bytes = sub_compact->builder->FileSize(); - meta->fd.file_size = current_bytes; + if (s.ok()) { + meta->fd.file_size = current_bytes; + } sub_compact->current_output()->finished = true; sub_compact->total_bytes += current_bytes; @@ -1144,17 +1146,24 @@ Status CompactionJob::FinishCompactionOutputFile( meta->marked_for_compaction ? " (need compaction)" : ""); } } - std::string fname = TableFileName(db_options_.db_paths, meta->fd.GetNumber(), - meta->fd.GetPathId()); + std::string fname; + FileDescriptor output_fd; + if (meta != nullptr) { + fname = TableFileName(db_options_.db_paths, meta->fd.GetNumber(), + meta->fd.GetPathId()); + output_fd = meta->fd; + } else { + fname = "(nil)"; + } EventHelpers::LogAndNotifyTableFileCreationFinished( event_logger_, cfd->ioptions()->listeners, dbname_, cfd->GetName(), fname, - job_id_, meta->fd, tp, TableFileCreationReason::kCompaction, s); + job_id_, output_fd, tp, TableFileCreationReason::kCompaction, s); #ifndef ROCKSDB_LITE // Report new file to SstFileManagerImpl auto sfm = static_cast(db_options_.sst_file_manager.get()); - if (sfm && meta->fd.GetPathId() == 0) { + if (sfm && meta != nullptr && meta->fd.GetPathId() == 0) { auto fn = TableFileName(cfd->ioptions()->db_paths, meta->fd.GetNumber(), meta->fd.GetPathId()); sfm->OnAddFile(fn); diff --git a/db/db_impl.cc b/db/db_impl.cc index 0652ce0c09c7bcd6a4ee96cae5c85799222798a2..0e1a821b0e0ba8a490d37b979110b5a71bd7e53f 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -777,9 +777,7 @@ void DBImpl::BackgroundCallPurge() { purge_queue_.pop_front(); mutex_.Unlock(); - Status file_deletion_status; - DeleteObsoleteFileImpl(file_deletion_status, job_id, fname, type, number, - path_id); + DeleteObsoleteFileImpl(job_id, fname, type, number, path_id); mutex_.Lock(); } else { assert(!logs_to_free_queue_.empty()); diff --git a/db/db_impl.h b/db/db_impl.h index e44b8126098f12dbaa26b2eb09a32cd806170ccf..5115ac6f12fd336576bd943578a3f27ed53b44af 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -685,9 +685,8 @@ class DBImpl : public DB { // Delete any unneeded files and stale in-memory entries. void DeleteObsoleteFiles(); // Delete obsolete files and log status and information of file deletion - void DeleteObsoleteFileImpl(Status file_deletion_status, int job_id, - const std::string& fname, FileType type, - uint64_t number, uint32_t path_id); + void DeleteObsoleteFileImpl(int job_id, const std::string& fname, + FileType type, uint64_t number, uint32_t path_id); // Background process needs to call // auto x = CaptureCurrentFileNumberInPendingOutputs() diff --git a/db/db_impl_files.cc b/db/db_impl_files.cc index e44e42318950fe377ec75056c8ee5e01e7ee3e37..4add98a81172fdcd929316e66552a367b1fef688 100644 --- a/db/db_impl_files.cc +++ b/db/db_impl_files.cc @@ -300,9 +300,10 @@ bool CompareCandidateFile(const JobContext::CandidateFileInfo& first, }; // namespace // Delete obsolete files and log status and information of file deletion -void DBImpl::DeleteObsoleteFileImpl(Status file_deletion_status, int job_id, - const std::string& fname, FileType type, - uint64_t number, uint32_t path_id) { +void DBImpl::DeleteObsoleteFileImpl(int job_id, const std::string& fname, + FileType type, uint64_t number, + uint32_t path_id) { + Status file_deletion_status; if (type == kTableFile) { file_deletion_status = DeleteSSTFile(&immutable_db_options_, fname, path_id); @@ -488,8 +489,7 @@ void DBImpl::PurgeObsoleteFiles(const JobContext& state, bool schedule_only) { InstrumentedMutexLock guard_lock(&mutex_); SchedulePendingPurge(fname, type, number, path_id, state.job_id); } else { - DeleteObsoleteFileImpl(file_deletion_status, state.job_id, fname, type, - number, path_id); + DeleteObsoleteFileImpl(state.job_id, fname, type, number, path_id); } } diff --git a/memtable/inlineskiplist.h b/memtable/inlineskiplist.h index 5cf6c57d573ad02ffea67462e1ff55fc19cedb4f..505b73d28a7c6a70ba37409103bd1b8027c4ea67 100644 --- a/memtable/inlineskiplist.h +++ b/memtable/inlineskiplist.h @@ -483,11 +483,13 @@ InlineSkipList::FindLessThan(const char* key, Node** prev, // KeyIsAfter(key, last_not_after) is definitely false Node* last_not_after = nullptr; while (true) { + assert(x != nullptr); Node* next = x->Next(level); assert(x == head_ || next == nullptr || KeyIsAfterNode(next->Key(), x)); assert(x == head_ || KeyIsAfterNode(key, x)); if (next != last_not_after && KeyIsAfterNode(key, next)) { // Keep searching in this list + assert(next != nullptr); x = next; } else { if (prev != nullptr) { @@ -863,6 +865,7 @@ void InlineSkipList::TEST_Validate() const { // levels. Node* nodes[kMaxPossibleHeight]; int max_height = GetMaxHeight(); + assert(max_height > 0); for (int i = 0; i < max_height; i++) { nodes[i] = head_; } @@ -892,7 +895,7 @@ void InlineSkipList::TEST_Validate() const { } } for (int i = 1; i < max_height; i++) { - assert(nodes[i]->Next(i) == nullptr); + assert(nodes[i] != nullptr && nodes[i]->Next(i) == nullptr); } } diff --git a/memtable/skiplist.h b/memtable/skiplist.h index 0162dccb78a80e53c3acb27e43f0f065ed9fd28b..58996be3e1c81f81bf8d4a55ba040e14a25a0a67 100644 --- a/memtable/skiplist.h +++ b/memtable/skiplist.h @@ -310,6 +310,7 @@ typename SkipList::Node* SkipList:: int level = GetMaxHeight() - 1; Node* last_bigger = nullptr; while (true) { + assert(x != nullptr); Node* next = x->Next(level); // Make sure the lists are sorted assert(x == head_ || next == nullptr || KeyIsAfterNode(next->key, x)); @@ -338,6 +339,7 @@ SkipList::FindLessThan(const Key& key, Node** prev) const { // KeyIsAfter(key, last_not_after) is definitely false Node* last_not_after = nullptr; while (true) { + assert(x != nullptr); Node* next = x->Next(level); assert(x == head_ || next == nullptr || KeyIsAfterNode(next->key, x)); assert(x == head_ || KeyIsAfterNode(key, x)); diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc index bd1e9fd95549e1efeae9e85b9d96bc5cbfd039b3..01d7667a34bfc633eeb77c7fa102ce2f39cd4279 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backupable/backupable_db_test.cc @@ -843,7 +843,7 @@ TEST_F(BackupableDBTest, NoDoubleCopy) { ASSERT_OK(test_backup_env_->FileExists(backupdir_ + "/shared/00015.sst")); // MANIFEST file size should be only 100 - uint64_t size; + uint64_t size = 0; test_backup_env_->GetFileSize(backupdir_ + "/private/2/MANIFEST-01", &size); ASSERT_EQ(100UL, size); test_backup_env_->GetFileSize(backupdir_ + "/shared/00015.sst", &size);