From 685cabdafa0722395f419cea248f44bbb6d4be93 Mon Sep 17 00:00:00 2001 From: Zhichao Cao Date: Thu, 1 Oct 2020 15:57:28 -0700 Subject: [PATCH] Add trace_analyzer_test to ASSERT_STATUS_CHECKED list (#7480) Summary: Add trace_analyzer_test to ASSERT_STATUS_CHECKED list Pull Request resolved: https://github.com/facebook/rocksdb/pull/7480 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 trace_analyzer_test Reviewed By: riversand963 Differential Revision: D24033768 Pulled By: zhichao-cao fbshipit-source-id: b415045e6fab01d6193448650772368c21c6dba6 --- Makefile | 1 + TARGETS | 1 + db/db_impl/db_impl.cc | 3 +- db/db_impl/db_impl_write.cc | 3 +- db/write_batch.cc | 3 +- tools/trace_analyzer_test.cc | 4 +-- tools/trace_analyzer_tool.cc | 64 +++++++++++++++++++----------------- tools/trace_analyzer_tool.h | 2 +- 8 files changed, 45 insertions(+), 36 deletions(-) diff --git a/Makefile b/Makefile index 798580df8..b65fc5d87 100644 --- a/Makefile +++ b/Makefile @@ -625,6 +625,7 @@ ifdef ASSERT_STATUS_CHECKED sst_dump_test \ statistics_test \ thread_local_test \ + trace_analyzer_test \ env_timed_test \ filelock_test \ timer_queue_test \ diff --git a/TARGETS b/TARGETS index ee66ee94c..c7ef952a4 100644 --- a/TARGETS +++ b/TARGETS @@ -471,6 +471,7 @@ cpp_library( "db/memtable_list.cc", "db/merge_helper.cc", "db/merge_operator.cc", + "db/output_validator.cc", "db/range_del_aggregator.cc", "db/range_tombstone_fragmenter.cc", "db/repair.cc", diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 1426341f2..496118403 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -1600,7 +1600,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key, // tracing is enabled. InstrumentedMutexLock lock(&trace_mutex_); if (tracer_) { - tracer_->Get(get_impl_options.column_family, key); + // TODO: maybe handle the tracing status? + tracer_->Get(get_impl_options.column_family, key).PermitUncheckedError(); } } diff --git a/db/db_impl/db_impl_write.cc b/db/db_impl/db_impl_write.cc index 735b9975a..0e50b3dba 100644 --- a/db/db_impl/db_impl_write.cc +++ b/db/db_impl/db_impl_write.cc @@ -77,7 +77,8 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options, if (tracer_) { InstrumentedMutexLock lock(&trace_mutex_); if (tracer_) { - tracer_->Write(my_batch); + // TODO: maybe handle the tracing status? + tracer_->Write(my_batch).PermitUncheckedError(); } } if (write_options.sync && write_options.disableWAL) { diff --git a/db/write_batch.cc b/db/write_batch.cc index fc23bdccb..f9b134068 100644 --- a/db/write_batch.cc +++ b/db/write_batch.cc @@ -340,7 +340,8 @@ uint32_t WriteBatch::ComputeContentFlags() const { auto rv = content_flags_.load(std::memory_order_relaxed); if ((rv & ContentFlags::DEFERRED) != 0) { BatchContentClassifier classifier; - Iterate(&classifier); + // Should we handle status here? + Iterate(&classifier).PermitUncheckedError(); rv = classifier.content_flags; // this method is conceptually const, because it is performing a lazy diff --git a/tools/trace_analyzer_test.cc b/tools/trace_analyzer_test.cc index 8a2aae6d2..b089e3d33 100644 --- a/tools/trace_analyzer_test.cc +++ b/tools/trace_analyzer_test.cc @@ -47,7 +47,7 @@ class TraceAnalyzerTest : public testing::Test { // test_path_ = test::TmpDir() + "trace_analyzer_test"; test_path_ = test::PerThreadDBPath("trace_analyzer_test"); env_ = ROCKSDB_NAMESPACE::Env::Default(); - env_->CreateDir(test_path_); + env_->CreateDir(test_path_).PermitUncheckedError(); dbname_ = test_path_ + "/db"; } @@ -173,7 +173,7 @@ class TraceAnalyzerTest : public testing::Test { if (!s.ok()) { GenerateTrace(trace_path); } - env_->CreateDir(output_path); + ASSERT_OK(env_->CreateDir(output_path)); RunTraceAnalyzer(paras); } diff --git a/tools/trace_analyzer_tool.cc b/tools/trace_analyzer_tool.cc index 7d240f6b9..2951af13f 100644 --- a/tools/trace_analyzer_tool.cc +++ b/tools/trace_analyzer_tool.cc @@ -1162,15 +1162,18 @@ Status TraceAnalyzer::ReProcessing() { // End the processing, print the requested results Status TraceAnalyzer::EndProcessing() { + Status s; if (trace_sequence_f_) { - trace_sequence_f_->Close(); + s = trace_sequence_f_->Close(); } if (FLAGS_no_print) { - return Status::OK(); + return s; } PrintStatistics(); - CloseOutputFiles(); - return Status::OK(); + if (s.ok()) { + s = CloseOutputFiles(); + } + return s; } // Insert the corresponding key statistics to the correct type @@ -1324,7 +1327,7 @@ Status TraceAnalyzer::KeyStatsInsertion(const uint32_t& type, ta_[type].stats[cf_id].time_series.push_back(trace_u); } - return Status::OK(); + return s; } // Update the correlation unit of each key if enabled @@ -1428,7 +1431,7 @@ Status TraceAnalyzer::OpenStatsOutputFiles(const std::string& type, &new_stats.a_qps_f); } - return Status::OK(); + return s; } // create the output path of the files to be opened @@ -1449,57 +1452,58 @@ Status TraceAnalyzer::CreateOutputFile( } // Close the output files in the TraceStats if they are opened -void TraceAnalyzer::CloseOutputFiles() { +Status TraceAnalyzer::CloseOutputFiles() { + Status s; for (int type = 0; type < kTaTypeNum; type++) { if (!ta_[type].enabled) { continue; } for (auto& stat : ta_[type].stats) { - if (stat.second.time_series_f) { - stat.second.time_series_f->Close(); + if (s.ok() && stat.second.time_series_f) { + s = stat.second.time_series_f->Close(); } - if (stat.second.a_key_f) { - stat.second.a_key_f->Close(); + if (s.ok() && stat.second.a_key_f) { + s = stat.second.a_key_f->Close(); } - if (stat.second.a_key_num_f) { - stat.second.a_key_num_f->Close(); + if (s.ok() && stat.second.a_key_num_f) { + s = stat.second.a_key_num_f->Close(); } - if (stat.second.a_count_dist_f) { - stat.second.a_count_dist_f->Close(); + if (s.ok() && stat.second.a_count_dist_f) { + s = stat.second.a_count_dist_f->Close(); } - if (stat.second.a_prefix_cut_f) { - stat.second.a_prefix_cut_f->Close(); + if (s.ok() && stat.second.a_prefix_cut_f) { + s = stat.second.a_prefix_cut_f->Close(); } - if (stat.second.a_value_size_f) { - stat.second.a_value_size_f->Close(); + if (s.ok() && stat.second.a_value_size_f) { + s = stat.second.a_value_size_f->Close(); } - if (stat.second.a_key_size_f) { - stat.second.a_key_size_f->Close(); + if (s.ok() && stat.second.a_key_size_f) { + s = stat.second.a_key_size_f->Close(); } - if (stat.second.a_qps_f) { - stat.second.a_qps_f->Close(); + if (s.ok() && stat.second.a_qps_f) { + s = stat.second.a_qps_f->Close(); } - if (stat.second.a_top_qps_prefix_f) { - stat.second.a_top_qps_prefix_f->Close(); + if (s.ok() && stat.second.a_top_qps_prefix_f) { + s = stat.second.a_top_qps_prefix_f->Close(); } - if (stat.second.w_key_f) { - stat.second.w_key_f->Close(); + if (s.ok() && stat.second.w_key_f) { + s = stat.second.w_key_f->Close(); } - if (stat.second.w_prefix_cut_f) { - stat.second.w_prefix_cut_f->Close(); + if (s.ok() && stat.second.w_prefix_cut_f) { + s = stat.second.w_prefix_cut_f->Close(); } } } - return; + return s; } // Handle the Get request in the trace diff --git a/tools/trace_analyzer_tool.h b/tools/trace_analyzer_tool.h index d2df2c824..2ca877df4 100644 --- a/tools/trace_analyzer_tool.h +++ b/tools/trace_analyzer_tool.h @@ -238,7 +238,7 @@ class TraceAnalyzer { const std::string& type, const std::string& cf_name, const std::string& ending, std::unique_ptr* f_ptr); - void CloseOutputFiles(); + Status CloseOutputFiles(); void PrintStatistics(); Status TraceUnitWriter(