Skip to content

Commit

Permalink
[Backport to the 2.0.5 branch] [#3665] Evict log cache at follower af…
Browse files Browse the repository at this point in the history
…ter operation was appended to log

Summary:
After 38ef51a, we ended up not calling ResponseFromPeer
for followers. At least one of the side effects of this was that the LogCache would be filled up
when getting requests, but no longer cleared after writing them to the Log. This quickly lead to
memory issues on lower memory machines, as we assign up to 1GB of memory across all LogCache
instances.

This diff addresses the issue, by cleaning LogCache after the operation is appended to the Log.

Test Plan:
ybd --gtest_filter QLStressTest.LongRemoteBootstrap
Jenkins: auto rebase: no

Reviewers: timur, mikhail, bogdan

Reviewed By: bogdan

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D8312
  • Loading branch information
spolitov committed Apr 17, 2020
1 parent 4bbf2e9 commit c9307b6
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
15 changes: 15 additions & 0 deletions src/yb/client/ql-stress-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,21 @@ TEST_F_EX(QLStressTest, LongRemoteBootstrap, QLStressTestLongRemoteBootstrap) {
ASSERT_OK(WaitAllReplicasHaveIndex(cluster_.get(), key.load(std::memory_order_acquire), 40s));
LOG(INFO) << "All replicas ready";

ASSERT_OK(WaitFor([this] {
bool result = true;
auto followers = ListTabletPeers(cluster_.get(), ListPeersFilter::kNonLeaders);
LOG(INFO) << "Num followers: " << followers.size();
for (const auto& peer : followers) {
auto log_cache_size = peer->raft_consensus()->LogCacheSize();
LOG(INFO) << "T " << peer->tablet_id() << " P " << peer->permanent_uuid()
<< ", log cache size: " << log_cache_size;
if (log_cache_size != 0) {
result = false;
}
}
return result;
}, 5s, "All followers cleanup cache"));

// Write some more values and check that replica still in touch.
std::this_thread::sleep_for(5s);
ASSERT_OK(WaitAllReplicasHaveIndex(cluster_.get(), key.load(std::memory_order_acquire), 1s));
Expand Down
3 changes: 3 additions & 0 deletions src/yb/consensus/consensus_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ void PeerMessageQueue::LocalPeerAppendFinished(const OpId& id,
fake_response.mutable_status()->set_last_committed_idx(queue_state_.committed_index.index());

if (queue_state_.mode != Mode::LEADER) {
log_cache_.EvictThroughOp(id.index());

UpdateMetrics();
return;
}
}
Expand Down

0 comments on commit c9307b6

Please sign in to comment.