From ee234e83e33e05bd58ce45a093f6d67f0b9c9c8a Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 23 Aug 2018 18:22:55 -0700 Subject: [PATCH] Invoke OnTableFileCreated for empty SSTs (#4307) Summary: The API comment on `OnTableFileCreationStarted` (https://github.com/facebook/rocksdb/blob/b6280d01f9f9c4305c536dfb804775fce3956280/include/rocksdb/listener.h#L331-L333) led users to believe a call to `OnTableFileCreationStarted` will always be matched with a call to `OnTableFileCreated`. However, we were skipping the `OnTableFileCreated` call in one case: no error happens but also no file is generated since there's no data. This PR adds the call to `OnTableFileCreated` for that case. The filename will be "(nil)" and the size will be zero. Pull Request resolved: https://github.com/facebook/rocksdb/pull/4307 Differential Revision: D9485201 Pulled By: ajkr fbshipit-source-id: 2f077ec7913f128487aae2624c69a50762394df6 --- HISTORY.md | 2 ++ db/compaction_job.cc | 4 +--- tools/db_stress.cc | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index fd18c3f88..27688d6ac 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,8 @@ # Rocksdb Change Log ## Unreleased ### Public API Change +* `OnTableFileCreated` will now be called for empty files generated during compaction. In that case, `TableFileCreationInfo::file_path` will be "(nil)" and `TableFileCreationInfo::file_size` will be zero. + ### New Features ### Bug Fixes diff --git a/db/compaction_job.cc b/db/compaction_job.cc index f17f2e34a..3c6f1eefd 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -1308,9 +1308,7 @@ Status CompactionJob::FinishCompactionOutputFile( // VersionEdit. assert(!sub_compact->outputs.empty()); sub_compact->outputs.pop_back(); - sub_compact->builder.reset(); - sub_compact->current_output_file_size = 0; - return s; + meta = nullptr; } if (s.ok() && (current_entries > 0 || tp.num_range_deletions > 0)) { diff --git a/tools/db_stress.cc b/tools/db_stress.cc index 337ddab2a..258527df2 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -1221,7 +1221,9 @@ class DbStressListener : public EventListener { const std::vector& column_families) : db_name_(db_name), db_paths_(db_paths), column_families_(column_families) {} - virtual ~DbStressListener() {} + virtual ~DbStressListener() { + assert(num_pending_file_creations_ == 0); + } #ifndef ROCKSDB_LITE virtual void OnFlushCompleted(DB* /*db*/, const FlushJobInfo& info) override { assert(IsValidColumnFamilyName(info.cf_name)); @@ -1246,16 +1248,23 @@ class DbStressListener : public EventListener { std::chrono::microseconds(Random::GetTLSInstance()->Uniform(5000))); } + virtual void OnTableFileCreationStarted( + const TableFileCreationBriefInfo& /*info*/) { + ++num_pending_file_creations_; + } virtual void OnTableFileCreated(const TableFileCreationInfo& info) override { assert(info.db_name == db_name_); assert(IsValidColumnFamilyName(info.cf_name)); - VerifyFilePath(info.file_path); + if (info.file_size) { + VerifyFilePath(info.file_path); + } assert(info.job_id > 0 || FLAGS_compact_files_one_in > 0); if (info.status.ok() && info.file_size > 0) { assert(info.table_properties.data_size > 0); assert(info.table_properties.raw_key_size > 0); assert(info.table_properties.num_entries > 0); } + --num_pending_file_creations_; } protected: @@ -1328,6 +1337,7 @@ class DbStressListener : public EventListener { std::string db_name_; std::vector db_paths_; std::vector column_families_; + std::atomic num_pending_file_creations_; }; } // namespace