leveldb: Moved to LevelDBEnv.IOError.BFE histogram entries.

third_party/leveldatabase/env_chromium.cc

 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
 #include "third_party/leveldatabase/env_chromium.h"
 
 #if defined(OS_POSIX)
 #include <dirent.h>
 #include <sys/types.h>
 #endif
 
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/memory/shared_memory.h"
 #include "base/metrics/histogram.h"
 #include "base/process/process_metrics.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "third_party/leveldatabase/chromium_logger.h"
 #include "third_party/re2/re2/re2.h"
 
 using base::FilePath;
 using leveldb::FileLock;
 using leveldb::Slice;
 using leveldb::Status;
 
@@ skipping 291 matching lines @@
   if (make_backup_ && file_type_ == kTable)
     uma_logger_->RecordBackupResult(ChromiumEnv::MakeBackup(filename_));
 
   return Status::OK();
 }
 
 class IDBEnv : public ChromiumEnv {
  public:
   IDBEnv() : ChromiumEnv() {
     name_ = "LevelDBEnv.IDB";
+    uma_ioerror_base_name_ = name_ + ".IOError.BFE";
     make_backup_ = true;
   }
 };
 
 base::LazyInstance<IDBEnv>::Leaky idb_env = LAZY_INSTANCE_INITIALIZER;
 
 base::LazyInstance<ChromiumEnv>::Leaky default_env = LAZY_INSTANCE_INITIALIZER;
 
 }  // unnamed namespace
 
@@ skipping 50 matching lines @@
   NOTREACHED();
   return "Unknown";
 }
 
 Status MakeIOError(Slice filename,
                    const std::string& message,
                    MethodID method,
                    base::File::Error error) {
   DCHECK_LT(error, 0);
   char buf[512];
-  snprintf(buf, sizeof(buf), "%s (ChromeMethodPFE: %d::%s::%d)",
+  snprintf(buf, sizeof(buf), "%s (ChromeMethodBFE: %d::%s::%d)",
            message.c_str(), method, MethodIDToString(method), -error);
   return Status::IOError(filename, buf);
 }
 
 Status MakeIOError(Slice filename,
                    const std::string& message,
                    MethodID method) {
   char buf[512];
   snprintf(buf, sizeof(buf), "%s (ChromeMethodOnly: %d::%s)", message.c_str(),
            method, MethodIDToString(method));
   return Status::IOError(filename, buf);
 }
 
 ErrorParsingResult ParseMethodAndError(const leveldb::Status& status,
                                        MethodID* method_param,
                                        base::File::Error* error) {
   const std::string status_string = status.ToString();
   int method;
   if (RE2::PartialMatch(status_string.c_str(), "ChromeMethodOnly: (\\d+)",
                         &method)) {
     *method_param = static_cast<MethodID>(method);
     return METHOD_ONLY;
   }
   int parsed_error;
   if (RE2::PartialMatch(status_string.c_str(),
-                        "ChromeMethodPFE: (\\d+)::.*::(\\d+)", &method,
+                        "ChromeMethodBFE: (\\d+)::.*::(\\d+)", &method,
                         &parsed_error)) {
     *method_param = static_cast<MethodID>(method);
     *error = static_cast<base::File::Error>(-parsed_error);
     DCHECK_LT(*error, base::File::FILE_OK);
     DCHECK_GT(*error, base::File::FILE_ERROR_MAX);
-    return METHOD_AND_PFE;
+    return METHOD_AND_BFE;
   }
   return NONE;
 }
 
 // Keep in sync with LevelDBCorruptionTypes in histograms.xml. Also, don't
 // change the order because indices into this array have been recorded in uma
 // histograms.
 const char* patterns[] = {
   "missing files",
   "log record too small",
@@ skipping 58 matching lines @@
   return patterns[code - 1];
 }
 
 bool IndicatesDiskFull(const leveldb::Status& status) {
   if (status.ok())
     return false;
   leveldb_env::MethodID method;
   base::File::Error error = base::File::FILE_OK;
   leveldb_env::ErrorParsingResult result =
       leveldb_env::ParseMethodAndError(status, &method, &error);
-  return (result == leveldb_env::METHOD_AND_PFE &&
+  return (result == leveldb_env::METHOD_AND_BFE &&
           static_cast<base::File::Error>(error) ==
               base::File::FILE_ERROR_NO_SPACE);
 }
 
 bool ChromiumEnv::MakeBackup(const std::string& fname) {
   FilePath original_table_name = FilePath::FromUTF8Unsafe(fname);
   FilePath backup_table_name =
       original_table_name.ReplaceExtension(backup_table_extension);
   return base::CopyFile(original_table_name, backup_table_name);
 }
 
 ChromiumEnv::ChromiumEnv()
     : name_("LevelDBEnv"),
       make_backup_(false),
       bgsignal_(&mu_),
       started_bgthread_(false),
       kMaxRetryTimeMillis(1000) {
+  uma_ioerror_base_name_ = name_ + ".IOError.BFE";
 }
 
 ChromiumEnv::~ChromiumEnv() {
   // In chromium, ChromiumEnv is leaked. It'd be nice to add NOTREACHED here to
   // ensure that behavior isn't accidentally changed, but there's an instance in
   // a unit test that is deleted.
 }
 
 bool ChromiumEnv::FileExists(const std::string& fname) {
   return base::PathExists(FilePath::FromUTF8Unsafe(fname));
@@ skipping 406 matching lines @@
 
 void ChromiumEnv::RecordBackupResult(bool result) const {
   std::string uma_name(name_);
   uma_name.append(".TableBackup");
   base::BooleanHistogram::FactoryGet(
       uma_name, base::Histogram::kUmaTargetedHistogramFlag)->AddBoolean(result);
 }
 
 base::HistogramBase* ChromiumEnv::GetOSErrorHistogram(MethodID method,
                                                       int limit) const {
-  std::string uma_name(name_);
-  // TODO(dgrogan): This is probably not the best way to concatenate strings.
-  uma_name.append(".IOError.").append(MethodIDToString(method));
+  std::string uma_name;
+  base::StringAppendF(&uma_name, "%s.%s", uma_ioerror_base_name_.c_str(),
+                      MethodIDToString(method));
   return base::LinearHistogram::FactoryGet(uma_name, 1, limit, limit + 1,
       base::Histogram::kUmaTargetedHistogramFlag);
 }
 
 base::HistogramBase* ChromiumEnv::GetMethodIOErrorHistogram() const {

[dgrogan, 2015/03/14 05:57:37]

It looks like this one didn't need to change. It just records the Env methods
where IOErrors were encountered, it doesn't record the IOErrors themselves.

Edit (after looking at xml):

Changing this like it is here brings us into a world of histograms.xml pain. I
think you should revert the change in this method and leave its histograms named
"LevelDBEnv.IOError" and "LevelDBEnv.IDB.IOError".

As it stands now the data this CL logs to the new
"LevelDBEnv.IDB.IOError.BFE.<methodName>" histograms would show up in the UMA UI
with LevelDBIOErrorMethods labels, not PlatformFileError labels.

[cmumford, 2015/03/17 21:25:52]

Yes, I see your point - and agree.

-  std::string uma_name(name_);
-  uma_name.append(".IOError");
-  return base::LinearHistogram::FactoryGet(uma_name, 1, kNumEntries,
-      kNumEntries + 1, base::Histogram::kUmaTargetedHistogramFlag);
+  return base::LinearHistogram::FactoryGet(
+      uma_ioerror_base_name_, 1, kNumEntries, kNumEntries + 1,
+      base::Histogram::kUmaTargetedHistogramFlag);
 }
 
 base::HistogramBase* ChromiumEnv::GetMaxFDHistogram(
     const std::string& type) const {
   std::string uma_name(name_);
   uma_name.append(".MaxFDs.").append(type);
   // These numbers make each bucket twice as large as the previous bucket.
   const int kFirstEntry = 1;
   const int kLastEntry = 65536;
   const int kNumBuckets = 18;
@@ skipping 128 matching lines @@
 
 Env* IDBEnv() {
   return leveldb_env::idb_env.Pointer();
 }
 
 Env* Env::Default() {
   return leveldb_env::default_env.Pointer();
 }
 
 }  // namespace leveldb