Repository: kudu Updated Branches: refs/heads/master 1bd98c7d4 -> c73f023da
KUDU-2356. Idle WALs should not consume significant memory Prior to this patch, each WAL writer would keep its temporary compress_buf_ sized based on the largest data that was ever written to it, even if the tablet was idle. Keeping the buffer around for a hot WAL probably has some small performance benefit, but when the WAL is idle enough to shut down its thread it should also clear its buffer to avoid occupying memory. Change-Id: I977976ef9505401b1bd596046915473a91153cd9 Reviewed-on: http://gerrit.cloudera.org:8080/9747 Tested-by: Todd Lipcon <t...@apache.org> Reviewed-by: Adar Dembo <a...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/e03375eb Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/e03375eb Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/e03375eb Branch: refs/heads/master Commit: e03375eb04945f172692ca6c4677e3442ed0b295 Parents: 1bd98c7 Author: Todd Lipcon <t...@apache.org> Authored: Wed Mar 21 16:20:37 2018 -0700 Committer: Todd Lipcon <t...@apache.org> Committed: Thu Mar 22 16:40:07 2018 +0000 ---------------------------------------------------------------------- src/kudu/consensus/log-test.cc | 10 ++++++++++ src/kudu/consensus/log.cc | 1 + src/kudu/consensus/log.h | 1 + src/kudu/consensus/log_util.cc | 4 ++++ src/kudu/consensus/log_util.h | 7 +++++++ 5 files changed, 23 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/e03375eb/src/kudu/consensus/log-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log-test.cc b/src/kudu/consensus/log-test.cc index 3d56b96..c1f4c92 100644 --- a/src/kudu/consensus/log-test.cc +++ b/src/kudu/consensus/log-test.cc @@ -52,7 +52,9 @@ #include "kudu/gutil/strings/substitute.h" #include "kudu/util/async_util.h" #include "kudu/util/compression/compression.pb.h" +#include "kudu/util/debug/sanitizer_scopes.h" #include "kudu/util/env.h" +#include "kudu/util/faststring.h" #include "kudu/util/metrics.h" #include "kudu/util/random.h" #include "kudu/util/status.h" @@ -1139,11 +1141,19 @@ TEST_F(LogTest, TestAutoStopIdleAppendThread) { ASSERT_EVENTUALLY([&]() { AppendNoOpsToLogSync(clock_, log_.get(), &opid, 2); ASSERT_TRUE(log_->append_thread_active_for_tests()); + debug::ScopedTSANIgnoreReadsAndWrites ignore_tsan; + ASSERT_GT(log_->active_segment_->compress_buf_.capacity(), faststring::kInitialCapacity); }); // After some time, the append thread should shut itself down. ASSERT_EVENTUALLY([&]() { ASSERT_FALSE(log_->append_thread_active_for_tests()); }); + + // The log should free its buffer once it is idle. + { + debug::ScopedTSANIgnoreReadsAndWrites ignore_tsan; + ASSERT_EQ(faststring::kInitialCapacity, log_->active_segment_->compress_buf_.capacity()); + } } // Test that Log::TotalSize() captures creation, addition, and deletion of log segments. http://git-wip-us.apache.org/repos/asf/kudu/blob/e03375eb/src/kudu/consensus/log.cc ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc index 5d82ebc..c27e425 100644 --- a/src/kudu/consensus/log.cc +++ b/src/kudu/consensus/log.cc @@ -341,6 +341,7 @@ void Log::AppendThread::DoWork() { } HandleGroup(std::move(entry_batches)); } + log_->active_segment_->GoIdle(); VLOG_WITH_PREFIX(2) << "WAL Appender going idle"; } http://git-wip-us.apache.org/repos/asf/kudu/blob/e03375eb/src/kudu/consensus/log.h ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log.h b/src/kudu/consensus/log.h index a398151..0ae30f0 100644 --- a/src/kudu/consensus/log.h +++ b/src/kudu/consensus/log.h @@ -241,6 +241,7 @@ class Log : public RefCountedThreadSafe<Log> { FRIEND_TEST(LogTestOptionalCompression, TestMultipleEntriesInABatch); FRIEND_TEST(LogTestOptionalCompression, TestReadLogWithReplacedReplicates); FRIEND_TEST(LogTest, TestWriteAndReadToAndFromInProgressSegment); + FRIEND_TEST(LogTest, TestAutoStopIdleAppendThread); class AppendThread; http://git-wip-us.apache.org/repos/asf/kudu/blob/e03375eb/src/kudu/consensus/log_util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log_util.cc b/src/kudu/consensus/log_util.cc index 07fba47..33c8188 100644 --- a/src/kudu/consensus/log_util.cc +++ b/src/kudu/consensus/log_util.cc @@ -866,6 +866,10 @@ Status WritableLogSegment::WriteEntryBatch(const Slice& data, return Status::OK(); } +void WritableLogSegment::GoIdle() { + compress_buf_.clear(); + compress_buf_.shrink_to_fit(); +} unique_ptr<LogEntryBatchPB> CreateBatchFromAllocatedOperations( const vector<consensus::ReplicateRefPtr>& msgs) { http://git-wip-us.apache.org/repos/asf/kudu/blob/e03375eb/src/kudu/consensus/log_util.h ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log_util.h b/src/kudu/consensus/log_util.h index af81c82..7f21fd3 100644 --- a/src/kudu/consensus/log_util.h +++ b/src/kudu/consensus/log_util.h @@ -402,6 +402,8 @@ class ReadableLogSegment : public RefCountedThreadSafe<ReadableLogSegment> { }; // A writable log segment where state data is stored. +// +// This class is not thread-safe. class WritableLogSegment { public: WritableLogSegment(std::string path, @@ -433,6 +435,10 @@ class WritableLogSegment { return writable_file_->Sync(); } + // Indicate that the segment has not been written for some period of time. + // In this case, temporary buffers should be freed up. + void GoIdle(); + // Returns true if the segment header has already been written to disk. bool IsHeaderWritten() const { return is_header_written_; @@ -466,6 +472,7 @@ class WritableLogSegment { } private: + FRIEND_TEST(LogTest, TestAutoStopIdleAppendThread); const std::shared_ptr<WritableFile>& writable_file() const { return writable_file_;
