KUDU-2279 (part 1). rolling_log: respect logfile retention flag Previously we never deleted old metrics logs, making it somewhat dangerous to enable this in production. Now we respect the same retention gflag that we apply to our other glogs.
Change-Id: If3cc9b085c9827fd544b0bf7d7ae868e3692296b Reviewed-on: http://gerrit.cloudera.org:8080/9175 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley <wdberke...@gmail.com> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3e49fe2f Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3e49fe2f Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3e49fe2f Branch: refs/heads/master Commit: 3e49fe2f7ca9afa603da600872cf3649346a0c22 Parents: 6d8694c Author: Todd Lipcon <t...@apache.org> Authored: Wed Jan 31 16:11:44 2018 -0800 Committer: Todd Lipcon <t...@apache.org> Committed: Sat Feb 3 00:57:34 2018 +0000 ---------------------------------------------------------------------- src/kudu/util/rolling_log-test.cc | 15 ++++++ src/kudu/util/rolling_log.cc | 87 +++++++++++++++++++++------------- src/kudu/util/rolling_log.h | 16 +++++-- 3 files changed, 81 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/3e49fe2f/src/kudu/util/rolling_log-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/rolling_log-test.cc b/src/kudu/util/rolling_log-test.cc index 626c0de..5c4c13b 100644 --- a/src/kudu/util/rolling_log-test.cc +++ b/src/kudu/util/rolling_log-test.cc @@ -125,4 +125,19 @@ TEST_F(RollingLogTest, TestCompression) { ASSERT_GT(size, 0); } +TEST_F(RollingLogTest, TestFileCountLimit) { + RollingLog log(env_, log_dir_, "mylog"); + ASSERT_OK(log.Open()); + log.SetSizeLimitBytes(100); + log.SetMaxNumSegments(3); + + for (int i = 0; i < 100; i++) { + ASSERT_OK(log.Append("hello world\n")); + } + ASSERT_OK(log.Close()); + + vector<string> children; + NO_FATALS(AssertLogCount(3, &children)); +} + } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/3e49fe2f/src/kudu/util/rolling_log.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/rolling_log.cc b/src/kudu/util/rolling_log.cc index 9f8d10f..a508e91 100644 --- a/src/kudu/util/rolling_log.cc +++ b/src/kudu/util/rolling_log.cc @@ -27,6 +27,7 @@ #include <utility> #include <gflags/gflags.h> +#include <gflags/gflags_declare.h> #include <glog/logging.h> #include <zlib.h> @@ -34,6 +35,7 @@ #include "kudu/gutil/strings/substitute.h" #include "kudu/gutil/walltime.h" #include "kudu/util/env.h" +#include "kudu/util/env_util.h" #include "kudu/util/net/net_util.h" #include "kudu/util/path_util.h" #include "kudu/util/slice.h" @@ -47,6 +49,8 @@ using strings::Substitute; static const int kDefaultSizeLimitBytes = 64 * 1024 * 1024; // 64MB +DECLARE_int32(max_log_files); + namespace kudu { RollingLog::RollingLog(Env* env, string log_dir, string log_name) @@ -54,6 +58,7 @@ RollingLog::RollingLog(Env* env, string log_dir, string log_name) log_dir_(std::move(log_dir)), log_name_(std::move(log_name)), size_limit_bytes_(kDefaultSizeLimitBytes), + max_num_segments_(FLAGS_max_log_files), compress_after_close_(true) {} RollingLog::~RollingLog() { @@ -65,42 +70,42 @@ void RollingLog::SetSizeLimitBytes(int64_t size) { size_limit_bytes_ = size; } +void RollingLog::SetMaxNumSegments(int num_segments) { + CHECK_GT(num_segments, 0); + max_num_segments_ = num_segments; +} + void RollingLog::SetCompressionEnabled(bool compress) { compress_after_close_ = compress; } -string RollingLog::GetLogFileName(int sequence) const { - ostringstream str; - - // 1. Program name. - str << google::ProgramInvocationShortName(); +namespace { - // 2. Host name. +string HostnameOrUnknown() { string hostname; Status s = GetHostname(&hostname); if (!s.ok()) { - hostname = "unknown_host"; + return "unknown_host"; } - str << "." << hostname; + return hostname; +} - // 3. User name. +string UsernameOrUnknown() { string user_name; - s = GetLoggedInUser(&user_name); + Status s = GetLoggedInUser(&user_name); if (!s.ok()) { - user_name = "unknown_user"; + return "unknown_user"; } - str << "." << user_name; - - // 4. Log name. - str << "." << log_name_; + return user_name; +} - // 5. Timestamp. +string FormattedTimestamp() { // Implementation cribbed from glog/logging.cc time_t time = static_cast<time_t>(WallTime_Now()); struct ::tm tm_time; localtime_r(&time, &tm_time); - str << "."; + ostringstream str; str.fill('0'); str << 1900+tm_time.tm_year << setw(2) << 1+tm_time.tm_mon @@ -109,15 +114,31 @@ string RollingLog::GetLogFileName(int sequence) const { << setw(2) << tm_time.tm_hour << setw(2) << tm_time.tm_min << setw(2) << tm_time.tm_sec; - str.clear(); // resets formatting flags + return str.str(); +} - // 6. Sequence number. - str << "." << sequence; +} // anonymous namespace - // 7. Pid. - str << "." << getpid(); +string RollingLog::GetLogFileName(int sequence) const { + return Substitute("$0.$1.$2.$3.$4.$5.$6", + google::ProgramInvocationShortName(), + HostnameOrUnknown(), + UsernameOrUnknown(), + log_name_, + FormattedTimestamp(), + sequence, + getpid()); +} - return str.str(); +string RollingLog::GetLogFilePattern() const { + return Substitute("$0.$1.$2.$3.$4.$5.$6", + google::ProgramInvocationShortName(), + HostnameOrUnknown(), + UsernameOrUnknown(), + log_name_, + /* any timestamp */'*', + /* any sequence number */'*', + /* any pid */'*'); } Status RollingLog::Open() { @@ -125,21 +146,20 @@ Status RollingLog::Open() { for (int sequence = 0; ; sequence++) { - string path = JoinPathSegments(log_dir_, - GetLogFileName(sequence)); + string path = JoinPathSegments(log_dir_, GetLogFileName(sequence)); + // Don't reuse an existing path if there is already a log + // or a compressed log with the same name. + if (env_->FileExists(path) || + env_->FileExists(path + ".gz")) { + continue; + } WritableFileOptions opts; // Logs aren't worth the performance cost of durability. opts.sync_on_close = false; opts.mode = Env::CREATE_NON_EXISTING; - Status s = env_->NewWritableFile(opts, path, &file_); - if (s.IsAlreadyPresent()) { - // We already rolled once at this same timestamp. - // Try again with a new sequence number. - continue; - } - RETURN_NOT_OK(s); + RETURN_NOT_OK(env_->NewWritableFile(opts, path, &file_)); VLOG(1) << "Rolled " << log_name_ << " log to new file: " << path; break; @@ -158,6 +178,9 @@ Status RollingLog::Close() { if (compress_after_close_) { WARN_NOT_OK(CompressFile(path), "Unable to compress old log file"); } + auto glob = JoinPathSegments(log_dir_, GetLogFilePattern()); + WARN_NOT_OK(env_util::DeleteExcessFilesByPattern(env_, glob, max_num_segments_), + Substitute("failed to delete old $0 log files", log_name_)); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/3e49fe2f/src/kudu/util/rolling_log.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/rolling_log.h b/src/kudu/util/rolling_log.h index 01829f0..897572a 100644 --- a/src/kudu/util/rolling_log.h +++ b/src/kudu/util/rolling_log.h @@ -63,11 +63,13 @@ class RollingLog { // the log as necessary if it is not open. Status Open(); - // Set the size limit for the current and any future log files. - // - // There is no limit on the total number of previous log segments. We rely - // on system utilities to clean up old logs to maintain some size limit. - void SetSizeLimitBytes(int64_t bytes); + // Set the pre-compression size limit for the current and any future log files. + // Note that this is the limit on a single segment of the log, not the total. + void SetSizeLimitBytes(int64_t size); + + // Set the total number of log segments to be retained. When the log is rolled, + // old segments are removed to achieve the targeted number of segments. + void SetMaxNumSegments(int num_segments); // If compression is enabled, log files are compressed. // NOTE: this requires that the passed-in Env instance is the local file system. @@ -89,6 +91,9 @@ class RollingLog { private: std::string GetLogFileName(int sequence) const; + // Get a glob pattern matching all log files written by this instance. + std::string GetLogFilePattern() const; + // Compress the given path, writing a new file '<path>.gz'. Status CompressFile(const std::string& path) const; @@ -97,6 +102,7 @@ class RollingLog { const std::string log_name_; int64_t size_limit_bytes_; + int max_num_segments_; std::unique_ptr<WritableFile> file_; bool compress_after_close_;
