andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Require only one Logger::Logv() implementation (#7605)

Browse Source 
Summary:
A user who extended `Logger` recently pointed out it is unusual to
require they implement the two-argument `Logv()` overload when they've
already implemented the three-argument `Logv()` overload. I agree with
that and think we can fix it by only calling the two-argument overload
from the default implementation of the three-argument overload. Then
when the three-argument overload is overridden, RocksDB would not
rely on the two-argument overload. Only `Logger::LogHeader()` needed
adjustment to achieve this.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7605

Reviewed By: riversand963

Differential Revision: D24584749

Pulled By: ajkr

fbshipit-source-id: 9aabe040ac761c4c0dbebc4be046967403ecaf21
This commit is contained in:
Andrew Kryczka 2020-10-28 09:59:12 -07:00 committed by Facebook GitHub Bot
parent 0e2e67562f
commit 6c2c0635c9
2 changed files with 28 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
env/env_test.cc vendored
View File

@ -2051,6 +2051,26 @@ TEST_F(EnvTest, Close) {
delete env;
}
class LogvWithInfoLogLevelLogger : public Logger {
public:
using Logger::Logv;
void Logv(const InfoLogLevel /* log_level */, const char* /* format */,
va_list /* ap */) override {}
};
TEST_F(EnvTest, LogvWithInfoLogLevel) {
// Verifies the log functions work on a `Logger` that only overrides the
// `Logv()` overload including `InfoLogLevel`.
const std::string kSampleMessage("sample log message");
LogvWithInfoLogLevelLogger logger;
ROCKS_LOG_HEADER(&logger, "%s", kSampleMessage.c_str());
ROCKS_LOG_DEBUG(&logger, "%s", kSampleMessage.c_str());
ROCKS_LOG_INFO(&logger, "%s", kSampleMessage.c_str());
ROCKS_LOG_WARN(&logger, "%s", kSampleMessage.c_str());
ROCKS_LOG_ERROR(&logger, "%s", kSampleMessage.c_str());
ROCKS_LOG_FATAL(&logger, "%s", kSampleMessage.c_str());
}
INSTANTIATE_TEST_CASE_P(DefaultEnvWithoutDirectIO, EnvPosixTestWithParam,
::testing::Values(std::pair<Env*, bool>(Env::Default(),
false)));

10
include/rocksdb/env.h
View File

@ -1051,11 +1051,17 @@ class Logger {
virtual void LogHeader(const char* format, va_list ap) {
// Default implementation does a simple INFO level log write.
// Please override as per the logger class requirement.
Logv(format, ap);
Logv(InfoLogLevel::INFO_LEVEL, format, ap);
}
// Write an entry to the log file with the specified format.
virtual void Logv(const char* format, va_list ap) = 0;
//
// Users who override the `Logv()` overload taking `InfoLogLevel` do not need
// to implement this, unless they explicitly invoke it in
// `Logv(InfoLogLevel, ...)`.
virtual void Logv(const char* /* format */, va_list /* ap */) {
assert(false);
}
// Write an entry to the log file with the specified log level
// and format. Any log with level under the internal log level