Add trace_analyzer_test to ASSERT_STATUS_CHECKED list (facebook#7480)

Summary: Add trace_analyzer_test to ASSERT_STATUS_CHECKED list Pull Request resolved: facebook#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
akankshamahajan15 · Oct 1, 2020 · 685cabd · 685cabd
1 parent f5e22ce
commit 685cabd
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -238,7 +238,7 @@ class TraceAnalyzer {
       const std::string& type, const std::string& cf_name,
       const std::string& ending,
       std::unique_ptr<ROCKSDB_NAMESPACE::WritableFile>* f_ptr);
-  void CloseOutputFiles();
+  Status CloseOutputFiles();
 
   void PrintStatistics();
   Status TraceUnitWriter(