From 6bab278291a243d0f772c050e51e404d599ec88d Mon Sep 17 00:00:00 2001 From: Jay Zhuang Date: Mon, 10 Jan 2022 22:02:22 -0800 Subject: [PATCH] Fix flaky SimCacheTest.SimCacheLogging (#9373) Summary: The random string may contain the string we're checking, e.g.: ``` ADD - 206FBC78E96BC4C6A2DDDDC0AD5D1ADD - 111 ``` Only check the line starts-with "ADD -". Pull Request resolved: https://github.com/facebook/rocksdb/pull/9373 Test Plan: `gtest-parallel ./sim_cache_test --gtest_filter=SimCacheTest.SimCacheLogging -r 1000` Reviewed By: riversand963 Differential Revision: D33519574 Pulled By: jay-zhuang fbshipit-source-id: d0c1c9b0b489246d292e7da4133030edaa748099 --- utilities/simulator_cache/sim_cache.cc | 47 +++++++++------------ utilities/simulator_cache/sim_cache_test.cc | 24 +++++------ 2 files changed, 32 insertions(+), 39 deletions(-) diff --git a/utilities/simulator_cache/sim_cache.cc b/utilities/simulator_cache/sim_cache.cc index 2ef0fe9b3..46dbb6bf3 100644 --- a/utilities/simulator_cache/sim_cache.cc +++ b/utilities/simulator_cache/sim_cache.cc @@ -6,6 +6,7 @@ #include "rocksdb/utilities/sim_cache.h" #include +#include #include "file/writable_file_writer.h" #include "monitoring/statistics.h" @@ -13,7 +14,6 @@ #include "rocksdb/env.h" #include "rocksdb/file_system.h" #include "util/mutexlock.h" -#include "util/string_util.h" namespace ROCKSDB_NAMESPACE { @@ -68,11 +68,12 @@ class CacheActivityLogger { return; } - std::string log_line = "LOOKUP - " + key.ToString(true) + "\n"; - + std::ostringstream oss; // line format: "LOOKUP - " + oss << "LOOKUP - " << key.ToString(true) << std::endl; + MutexLock l(&mutex_); - Status s = file_writer_->Append(log_line); + Status s = file_writer_->Append(oss.str()); if (!s.ok() && bg_status_.ok()) { bg_status_ = s; } @@ -88,15 +89,11 @@ class CacheActivityLogger { return; } - std::string log_line = "ADD - "; - log_line += key.ToString(true); - log_line += " - "; - AppendNumberTo(&log_line, size); - log_line += "\n"; - + std::ostringstream oss; // line format: "ADD - - " + oss << "ADD - " << key.ToString(true) << " - " << size << std::endl; MutexLock l(&mutex_); - Status s = file_writer_->Append(log_line); + Status s = file_writer_->Append(oss.str()); if (!s.ok() && bg_status_.ok()) { bg_status_ = s; } @@ -298,25 +295,23 @@ class SimCacheImpl : public SimCache { } std::string ToString() const override { - std::string res; - res.append("SimCache MISSes: " + std::to_string(get_miss_counter()) + "\n"); - res.append("SimCache HITs: " + std::to_string(get_hit_counter()) + "\n"); - char buff[350]; + std::ostringstream oss; + oss << "SimCache MISSes: " << get_miss_counter() << std::endl; + oss << "SimCache HITs: " << get_hit_counter() << std::endl; auto lookups = get_miss_counter() + get_hit_counter(); - snprintf(buff, sizeof(buff), "SimCache HITRATE: %.2f%%\n", - (lookups == 0 ? 0 : get_hit_counter() * 100.0f / lookups)); - res.append(buff); - return res; + oss << "SimCache HITRATE: " << std::fixed << std::setprecision(2) + << (lookups == 0 ? 0 : get_hit_counter() * 100.0f / lookups) + << std::endl; + return oss.str(); } std::string GetPrintableOptions() const override { - std::string ret; - ret.reserve(20000); - ret.append(" cache_options:\n"); - ret.append(cache_->GetPrintableOptions()); - ret.append(" sim_cache_options:\n"); - ret.append(key_only_cache_->GetPrintableOptions()); - return ret; + std::ostringstream oss; + oss << " cache_options:" << std::endl; + oss << cache_->GetPrintableOptions(); + oss << " sim_cache_options:" << std::endl; + oss << key_only_cache_->GetPrintableOptions(); + return oss.str(); } Status StartActivityLogging(const std::string& activity_log_file, Env* env, diff --git a/utilities/simulator_cache/sim_cache_test.cc b/utilities/simulator_cache/sim_cache_test.cc index ee01faf64..83d4c6fa9 100644 --- a/utilities/simulator_cache/sim_cache_test.cc +++ b/utilities/simulator_cache/sim_cache_test.cc @@ -177,23 +177,21 @@ TEST_F(SimCacheTest, SimCacheLogging) { std::string file_contents = ""; ASSERT_OK(ReadFileToString(env_, log_file, &file_contents)); + std::istringstream contents(file_contents); int lookup_num = 0; int add_num = 0; - std::string::size_type pos; - // count number of lookups - pos = 0; - while ((pos = file_contents.find("LOOKUP -", pos)) != std::string::npos) { - ++lookup_num; - pos += 1; - } - - // count number of additions - pos = 0; - while ((pos = file_contents.find("ADD -", pos)) != std::string::npos) { - ++add_num; - pos += 1; + std::string line; + // count number of lookups and additions + while (std::getline(contents, line)) { + // check if the line starts with LOOKUP or ADD + if (line.rfind("LOOKUP -", 0) == 0) { + ++lookup_num; + } + if (line.rfind("ADD -", 0) == 0) { + ++add_num; + } } // We asked for every block twice